Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues with LACP and nexthop LAG action #146

Merged
merged 1 commit into from
Jun 20, 2024
Merged

Fix issues with LACP and nexthop LAG action #146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions switchapi/es2k/lnw_v3/switch_pd_lag.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ switch_status_t switch_pd_tx_lag_table_entry(switch_device_t device,
}

status = tdi_key_field_set_value_and_mask(key_hdl, field_id_hash,
lag_list + port_count, 0x3);
lag_list + port_count, 0x7);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the 0x7 here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

He's widening a two-bit mask to three bits, which appears to be the width of the field in the P4 program.

if (status != TDI_SUCCESS) {
krnlmon_log_error(
"Unable to set value for key ID: %d for tx_lag_table"
Expand Down Expand Up @@ -604,7 +604,8 @@ switch_status_t switch_pd_tx_lacp_lag_table_entry(
CHECK_RET(status != SWITCH_STATUS_SUCCESS, status);
status = switch_pd_get_physical_port_id(
device, lag_member_info->api_lag_member_info.port_id, &egress_port);
status = tdi_key_field_set_value(key_hdl, field_id_lag_id, lag_id);
status = tdi_key_field_set_value_and_mask(key_hdl, field_id_lag_id,
lag_id, 0xFF);
if (status != TDI_SUCCESS) {
krnlmon_log_error(
"Unable to set value for key ID: %d for tx_lag_table"
Expand All @@ -613,8 +614,8 @@ switch_status_t switch_pd_tx_lacp_lag_table_entry(
goto dealloc_resources;
}

status = tdi_key_field_set_value(key_hdl, field_id_hash,
lag_list + port_count);
status = tdi_key_field_set_value_and_mask(key_hdl, field_id_hash,
lag_list + port_count, 0x7);
if (status != TDI_SUCCESS) {
krnlmon_log_error(
"Unable to set value for key ID: %d for tx_lag_table"
Expand Down
22 changes: 14 additions & 8 deletions switchapi/es2k/lnw_v3/switch_pd_routing.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
#define MAC_BASE 0
#define MAC_LOW_BYTES 4
#define MAC_HIGH_BYTES 2
#define MAC_HIGH_OFFSET MAC_BASE + MAC_HIGH_BYTES
#define MAC_LOW_OFFSET MAC_BASE + MAC_HIGH_BYTES

switch_status_t switch_routing_table_entry(
switch_device_t device, const switch_pd_routing_info_t* api_routing_info,
Expand Down Expand Up @@ -290,7 +290,7 @@ switch_status_t switch_pd_nexthop_table_entry(
status = tdi_data_field_set_value_ptr(
data_hdl, data_field_id,
(const uint8_t*)&api_nexthop_pd_info->dst_mac_addr.mac_addr +
MAC_HIGH_OFFSET,
MAC_LOW_OFFSET,
MAC_LOW_BYTES);
if (status != TDI_SUCCESS) {
krnlmon_log_error("Unable to set action value for ID: %d, error: %d",
Expand Down Expand Up @@ -380,8 +380,7 @@ switch_status_t switch_pd_nexthop_table_entry(

status = tdi_data_field_set_value_ptr(
data_hdl, data_field_id,
(const uint8_t*)&api_nexthop_pd_info->dst_mac_addr.mac_addr +
MAC_HIGH_OFFSET,
(const uint8_t*)&api_nexthop_pd_info->dst_mac_addr.mac_addr + MAC_BASE,
MAC_HIGH_BYTES);
if (status != TDI_SUCCESS) {
krnlmon_log_error("Unable to set action value for ID: %d, error: %d",
Expand All @@ -400,7 +399,8 @@ switch_status_t switch_pd_nexthop_table_entry(

status = tdi_data_field_set_value_ptr(
data_hdl, data_field_id,
(const uint8_t*)&api_nexthop_pd_info->dst_mac_addr.mac_addr + MAC_BASE,
(const uint8_t*)&api_nexthop_pd_info->dst_mac_addr.mac_addr +
MAC_LOW_OFFSET,
MAC_LOW_BYTES);
if (status != TDI_SUCCESS) {
krnlmon_log_error("Unable to set action value for ID: %d, error: %d",
Expand Down Expand Up @@ -473,6 +473,12 @@ switch_status_t switch_pd_ecmp_nexthop_table_entry(
uint16_t network_byte_order_rif_id = 0;

krnlmon_log_debug("%s", __func__);

if (SWITCH_LAG_HANDLE(api_nexthop_pd_info->rif_handle)) {
// ECMP and LAG are mutually exclusive, we don't need to handle LAG case.
return SWITCH_STATUS_SUCCESS;
}

status = tdi_info_get(dev_id, PROGRAM_NAME, &info_hdl);
if (status != TDI_SUCCESS) {
krnlmon_log_error("Failed to get tdi info handle, error: %d", status);
Expand Down Expand Up @@ -612,8 +618,7 @@ switch_status_t switch_pd_ecmp_nexthop_table_entry(
}
status = tdi_data_field_set_value_ptr(
data_hdl, data_field_id,
(const uint8_t*)&api_nexthop_pd_info->dst_mac_addr.mac_addr +
MAC_HIGH_OFFSET,
(const uint8_t*)&api_nexthop_pd_info->dst_mac_addr.mac_addr + MAC_BASE,
MAC_HIGH_BYTES);
if (status != TDI_SUCCESS) {
krnlmon_log_error("Unable to set action value for ID: %d, error: %d",
Expand All @@ -632,7 +637,8 @@ switch_status_t switch_pd_ecmp_nexthop_table_entry(

status = tdi_data_field_set_value_ptr(
data_hdl, data_field_id,
(const uint8_t*)&api_nexthop_pd_info->dst_mac_addr.mac_addr + MAC_BASE,
(const uint8_t*)&api_nexthop_pd_info->dst_mac_addr.mac_addr +
MAC_LOW_OFFSET,
MAC_LOW_BYTES);
if (status != TDI_SUCCESS) {
krnlmon_log_error("Unable to set action value for ID: %d, error: %d",
Expand Down
71 changes: 55 additions & 16 deletions switchapi/es2k/switch_lag.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,24 +152,26 @@ switch_status_t switch_api_lag_delete(switch_device_t device,
status = switch_lag_get(device, lag_h, &lag_info);
CHECK_RET(status != SWITCH_STATUS_SUCCESS, status);

//--------------------- Tx Path : Del Case ----------------------//
status = switch_pd_tx_lag_table_entry(device, lag_info, false);
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to delete tx_lag_table entry on device:%d error: %s\n", device,
switch_error_to_string(status));
return status;
}
/* Delete only when we have an active LAG member*/
if (lag_info->active_lag_member != 0) {
//--------------------- Tx Path : Del Case ----------------------//
status = switch_pd_tx_lag_table_entry(device, lag_info, false);
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to delete tx_lag_table entry on device:%d error: %s\n",
device, switch_error_to_string(status));
return status;
}

//--------------------- Rx Path : Del Case ----------------------//
status = switch_pd_rx_lag_table_entry(device, lag_info, false);
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to delete rx_lag_table entry on device:%d error: %s\n", device,
switch_error_to_string(status));
return status;
//--------------------- Rx Path : Del Case ----------------------//
status = switch_pd_rx_lag_table_entry(device, lag_info, false);
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to delete rx_lag_table entry on device:%d error: %s\n",
device, switch_error_to_string(status));
return status;
}
}

status = switch_lag_handle_delete(device, lag_h);
CHECK_RET(status != SWITCH_STATUS_SUCCESS, status);

Expand Down Expand Up @@ -561,6 +563,43 @@ switch_status_t switch_api_lag_attribute_get(
return status;
}

/**
* Routine Description:
* @brief Update RMAC handle for a LAG interface
*
* Arguments:
* @param[in] device - device
* @param[in] lag_h - LAG handle
* @param[out] rmac_h - RMAC handle
*
* Return Values:
* @return SWITCH_STATUS_SUCCESS on success
* Failure status code on error
*/
switch_status_t switch_api_lag_update_rmac_handle(
const switch_device_t device, const switch_handle_t lag_h,
const switch_handle_t rmac_h) {
switch_lag_info_t* lag_info = NULL;
switch_status_t status = SWITCH_STATUS_SUCCESS;

if (!SWITCH_LAG_HANDLE(lag_h)) {
status = SWITCH_STATUS_INVALID_PARAMETER;
krnlmon_log_error(
"LAG attribute get: Invalid LAG handle on device %d, "
"LAG handle 0x%lx: "
"error: %s\n",
device, lag_h, switch_error_to_string(status));
return status;
}

status = switch_lag_get(device, lag_h, &lag_info);
CHECK_RET(status != SWITCH_STATUS_SUCCESS, status);

lag_info->api_lag_info.rmac_handle = rmac_h;

return status;
}

/**
* Routine Description:
* @brief Program the Tx and Rx LAG tables
Expand Down
5 changes: 5 additions & 0 deletions switchapi/switch_lag.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ switch_status_t switch_api_lag_attribute_get(
const switch_device_t device, const switch_handle_t lag_handle,
switch_api_lag_info_t* api_lag_info);

// Switchapi method to update RMAC handle for a LAG
switch_status_t switch_api_lag_update_rmac_handle(const switch_device_t device,
const switch_handle_t lag_h,
const switch_handle_t rmac_h);

// Switchapi method to program LAG tables in HW
switch_status_t switch_api_program_lag_hw(const switch_device_t device,
switch_handle_t lag_handle,
Expand Down
59 changes: 35 additions & 24 deletions switchlink/sai/switchlink_handle_lag.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,30 @@ void switchlink_create_lag(switchlink_db_interface_info_t* lag_intf) {
return;
}

/**
* Routine Description:
* Wrapper function to delete lag member
*
* Arguments:
* [in] ifindex - lag member ifindex
*
* Return Values:
* void
*/
void switchlink_delete_lag_member(uint32_t ifindex) {
switchlink_db_lag_member_info_t lag_member_info;
memset(&lag_member_info, 0, sizeof(switchlink_db_lag_member_info_t));
lag_member_info.ifindex = ifindex;
if (switchlink_db_get_lag_member_info(&lag_member_info) ==
SWITCHLINK_DB_STATUS_ITEM_NOT_FOUND) {
return;
}

// delete the lag member from backend and DB
delete_lag_member(&lag_member_info, lag_member_info.lag_member_h);
switchlink_db_delete_lag_member(&lag_member_info);
}

/**
* Routine Description:
* Wrapper function to delete lag
Expand All @@ -343,6 +367,17 @@ void switchlink_delete_lag(uint32_t ifindex) {
return;
}

/* Delete LAG members for an LACP */
if (lag_intf.bond_mode == SWITCHLINK_BOND_MODE_LACP) {
uint32_t member_if_index = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loop might be clearer in the following form:

for (;;) {
  uint32_t member_if_index =
    switchlink_db_delete_lacp_member(lag_intf.lag_h);
  if (member_if_index == -1) break;
  switchlink_delete_lag_member(member_if_index);
}

while (member_if_index != -1) {
member_if_index = switchlink_db_delete_lacp_member(lag_intf.lag_h);
if (member_if_index != -1) {
switchlink_delete_lag_member(member_if_index);
}
}
}

// delete the lag from backend and DB
delete_lag(&lag_intf, lag_intf.lag_h);
switchlink_db_delete_interface(lag_intf.ifindex);
Expand Down Expand Up @@ -425,27 +460,3 @@ void switchlink_create_lag_member(

return;
}

/**
* Routine Description:
* Wrapper function to delete lag member
*
* Arguments:
* [in] ifindex - lag member ifindex
*
* Return Values:
* void
*/
void switchlink_delete_lag_member(uint32_t ifindex) {
switchlink_db_lag_member_info_t lag_member_info;
memset(&lag_member_info, 0, sizeof(switchlink_db_lag_member_info_t));
lag_member_info.ifindex = ifindex;
if (switchlink_db_get_lag_member_info(&lag_member_info) ==
SWITCHLINK_DB_STATUS_ITEM_NOT_FOUND) {
return;
}

// delete the lag member from backend and DB
delete_lag_member(&lag_member_info, lag_member_info.lag_member_h);
switchlink_db_delete_lag_member(&lag_member_info);
}
25 changes: 25 additions & 0 deletions switchlink/switchlink_db.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1132,6 +1132,31 @@ switchlink_db_status_t switchlink_db_delete_lag_member(
return SWITCHLINK_DB_STATUS_ITEM_NOT_FOUND;
}

/**
* Routine Description:
* Lookup through LAG members and fetch the lag member which points to valid
* LACP lag_h
*
* Arguments:
* [in] lag_h - LACP LAG handle
*
* Return Values:
* LAG member ifindex on success
* -1 otherwise
*/
uint32_t switchlink_db_delete_lacp_member(switchlink_handle_t lag_h) {
tommy_node* node = tommy_list_head(&switchlink_db_lag_member_obj_list);
while (node) {
switchlink_db_lag_member_obj_t* obj = node->data;
krnlmon_assert(obj != NULL);
node = node->next;
if ((obj->lag_member_info.lag_h == lag_h)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ((obj->lag_member_info.lag_h == lag_h)) {
if (obj->lag_member_info.lag_h == lag_h) {

return obj->lag_member_info.ifindex;
}
}
return -1;
}

/**
* Routine Description:
* Updates oper_state of lag member in switchlink database
Expand Down
2 changes: 2 additions & 0 deletions switchlink/switchlink_db.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,4 +266,6 @@ extern switchlink_db_status_t switchlink_db_update_lag_member_oper_state(
extern switchlink_db_status_t switchlink_db_get_lag_member_info(
switchlink_db_lag_member_info_t* lag_member_info);

extern uint32_t switchlink_db_delete_lacp_member(switchlink_handle_t lag_h);

#endif /* __SWITCHLINK_DB_H__ */
2 changes: 0 additions & 2 deletions switchlink/switchlink_link.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,6 @@ void switchlink_process_link_msg(const struct nlmsghdr* nlmsg, int msgtype) {
if (link_type == SWITCHLINK_LINK_TYPE_TUN ||
link_type == SWITCHLINK_LINK_TYPE_RIF) {
switchlink_delete_interface(ifmsg->ifi_index);
} else {
krnlmon_log_debug("Unhandled link type");
}
#ifdef LAG_OPTION
if (link_type == SWITCHLINK_LINK_TYPE_BOND) {
Expand Down
9 changes: 9 additions & 0 deletions switchsai/sailag.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,15 @@ static sai_status_t sai_create_lag(_Out_ sai_object_id_t* lag_id,
sai_status_to_string(status));
return status;
}
#if defined(ES2K_TARGET)
status = switch_api_lag_update_rmac_handle(switch_id, lag_h, rmac_handle);
if (status != SAI_STATUS_SUCCESS) {
krnlmon_log_error("Failed to update RMAC handle, error: %s",
sai_status_to_string(status));
return status;
}
#endif

} else {
*lag_id = SAI_NULL_OBJECT_ID;

Expand Down