LAG LACP & L3 Support #146
Conversation
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
|
/azp run |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
|
/azp run |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| Both SONiC and VPP can run LACP. If we choose to run LACP in VPP, the PortChannel interface in SONiC will not be reported up. | ||
| Instead, we need to let SONiC run LACP and instruct VPP to carry SONiC's LACP packets without generating or consuming them. | ||
|
|
||
| To achieve this, we can use the VPP [linux-cp plugin](https://s3-docs.fd.io/vpp/22.06/developer/plugins/lcp.html) to mirror packets between the member tap interfaces (on host) |
There was a problem hiding this comment.
mirror means copy. Do we actually do that?
| #define SAIVPP_WARN clib_warning | ||
| #define SAIVPP_ERROR clib_error | ||
| //#define SAIVPP_DEBUG(format,args...) {} | ||
| #define SAIVPP_DEBUG(format,args...) clib_warning("PID: %d, TID: %ld, " format, getpid(), syscall(SYS_gettid), ##args) |
There was a problem hiding this comment.
Do we want to enable debug log? Could it be too chatty?
| @@ -1159,28 +1164,86 @@ sai_status_t SwitchStateBase:: createLag( | |||
|
|
|||
| } | |||
|
|
|||
| uint32_t SwitchStateBase::find_bond_id() | |||
There was a problem hiding this comment.
probably find_new_bond_id is better.
|
|
||
| if (existing_bond_ids.find(bond_id) == existing_bond_ids.end()) { | ||
| SWSS_LOG_NOTICE("Found new bond id from PortChannel name: %d", bond_id); | ||
| found_new_bond_id = true; |
There was a problem hiding this comment.
what will happen if there are multiple port channel interfaces created? I feel this race condition is real but we don't see any problem with sonic-mgmt test. So probably we just keep this in mind in case weird behavior happens.
| } | ||
|
|
||
| // Set mode and lb. SONiC config does not have provision to pass mode and load balancing algorithm | ||
| mode = VPP_BOND_API_MODE_XOR; | ||
| lb = VPP_BOND_API_LB_ALGO_L23; |
There was a problem hiding this comment.
for l3 port channel interface, LB algo should be L34. Can we change the algo when it is assigned?
| SWSS_LOG_ENTER(); | ||
|
|
||
| uint32_t lag_swif_idx = lag_to_bond_if_idx(lag_oid); | ||
| SWSS_LOG_NOTICE("lag swif idx :%d",lag_swif_idx); | ||
| LAG_MUTEX; |
| bool found = getTapNameFromPortId(obj_id, if_name); | ||
| bool found = false; | ||
| platform_bond_info_t bond_info; | ||
| if (objectTypeQuery(obj_id) == SAI_OBJECT_TYPE_LAG) { |
There was a problem hiding this comment.
is it better to incorporate this logic in getTapNameFromPortid?
| snprintf(hw_bondifname, sizeof(hw_bondifname), "%s%d", BONDETHERNET_PREFIX, bond_info.id); | ||
| hwif_name = hw_bondifname; | ||
| } else { | ||
| hwif_name = tap_to_hwif_name(if_name.c_str()); |
There was a problem hiding this comment.
same as above, is it better to incorporate it in tap_to_hwif_name?
Adding support for LAG LACP & L3. Addresses #140 and #90.
See the corresponding sections in the LAG HLD for details of the changes in this PR.