ISSUE 90: SONiC-VPP LAG feature support #91
Conversation
Files Modified:
platform/saivpp/vpplib/SwitchStateBase.cpp
platform/saivpp/vpplib/SwitchStateBase.h
platform/saivpp/vpplib/SwitchStateBaseFdb.cpp
platform/saivpp/vpplib/vppxlate/SaiVppXlate.c
platform/saivpp/vpplib/vppxlate/SaiVppXlate.h
Signed-of-by:
bendrapubalareddy
[email protected]
Files Added:
LAG-Dot1q-Bridging-Topo.png
README.LAG.dot1q.bridging.topo.md
Signed-off-by:
bendrapubalareddy
[email protected]
Files Added:
docs/HLD/vpp-lag.md
Singed-off-by:
bendrapubalareddy
[email protected]
Files modified:
modified: docs/HLD/vpp-lag.md
modified: docs/README.LAG.dot1q.bridging.topo.md
modified: platform/saivpp/vpplib/SwitchStateBase.cpp
modified: platform/saivpp/vpplib/SwitchStateBaseFdb.cpp
Signed-Off-By: bendrapubalareddy
email: [email protected]
| //Delete the Bond interface | ||
| delete_bond_interface(lag_ifname); | ||
| remove_lag_to_bond_entry(lag_oid); | ||
|
|
There was a problem hiding this comment.
I think you need to call refresh_interfaces_list() to build new interface id to name mapping once bond interface is deleted.
There was a problem hiding this comment.
Yes, done
|
|
||
| //if LACP is to be enabled in VPP | ||
| //mode = VPP_BOND_API_MODE_LACP; | ||
| lb = VPP_BOND_API_LB_ALGO_L2; |
There was a problem hiding this comment.
I think the hash algorithm should cover L4 and L4 headers for flow consistency to avoid reordering problem.
There was a problem hiding this comment.
Ideally this should be configurable , which SONiC does not support. I tried with different options like round robin etc it did not work.
There was a problem hiding this comment.
LB ALGO is only relevant to LACP or XOR mode. The other modes are static. For example, there is only one interface up in active/standy so no LB is needed. In round-robin, packets are sent to multiple member links in round robin fashion.
VPP LB ALGO supports L2, L23 and L34. If the LAG is used for L3 forwarding, L34 is most likely the choice. But if the LAG is for L2 switching, L23 is probably the better choice. In modern ASICs (8K and BRCM for example), LB algo is automatically adjusted for different scenarios in a sense by excluding terminated headers in hash calculation.
From what I read of SONiC document, it seems LACP is always enabled. Should we do the same in vpp?
There was a problem hiding this comment.
Agree, L23 seems more reasonable choice, given there is configurable option available.
Yes, LACP mode can be enabled and need to be tested once.
There was a problem hiding this comment.
There is no one-size-fit-all solution here. If the LAG is for L2 switching, we should use L23. If it is for L3 routing, L34 is better, which is the typical 5 tuples hashing. But we don't know if a LAG is for L2 or L3 at the time it is created. Maybe we can create lag with a default lb algo like l23 and change algo to l34 if the lag has IP configured? To do that, either we have to delete and recreate lag in vpp or we augment vpp to allow changing lb algo after lag is created.
| @@ -1011,6 +1092,20 @@ static u32 get_swif_idx (vat_main_t *vam, const char *ifname) | |||
| return ((u32) -1); | |||
| } | |||
|
|
|||
| static const char * get_swif_name (vat_main_t *vam, const u32 swif_idx) | |||
| { | |||
| hash_pair_t *p; | |||
There was a problem hiding this comment.
Do you think a VPP_LOCK() is required while iterating over the list ?
No description provided.