Skip to content

Commit

Permalink
Addressing comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Kumar, Aashish <[email protected]>
  • Loading branch information
@aashishkuma
aashishkuma committed Jan 9, 2024
1 parent 5daee31 commit 4270406
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 66 deletions.
108 changes: 51 additions & 57 deletions switchapi/es2k/switch_lag.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,8 @@ switch_status_t switch_api_lag_create(switch_device_t device,
if (!SWITCH_RMAC_HANDLE(api_lag_info->rmac_handle)) {
status = SWITCH_STATUS_INVALID_HANDLE;
krnlmon_log_error(
"LAG create: Invalid rmac handle on device %d: "
"error: %s\n",
device, switch_error_to_string(status));
"LAG create: Invalid rmac handle on device:%d error: %s\n", device,
switch_error_to_string(status));
return status;
}

Expand All @@ -115,18 +114,16 @@ switch_status_t switch_api_lag_create(switch_device_t device,

status = switch_lag_get(device, *lag_h, &lag_info);
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"LAG create: Failed to get LAG on device %d: "
"error: %s\n",
device, switch_error_to_string(status));
krnlmon_log_error("LAG create: Failed to get LAG on device:%d error: %s\n",
device, switch_error_to_string(status));
status = switch_lag_handle_delete(device, *lag_h);
CHECK_RET(status != SWITCH_STATUS_SUCCESS, status);
return status;
}

lag_info->lag_handle = *lag_h;

status = SWITCH_LIST_INIT(&(lag_info->lag_members));
status = SWITCH_LIST_INIT(&lag_info->lag_members);
SWITCH_ASSERT(status == SWITCH_STATUS_SUCCESS);

SWITCH_MEMCPY(&lag_info->api_lag_info, api_lag_info,
Expand Down Expand Up @@ -159,19 +156,17 @@ switch_status_t switch_api_lag_delete(switch_device_t device,
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));
"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));
"Failed to delete rx_lag_table entry on device:%d error: %s\n", device,
switch_error_to_string(status));
return status;
}

Expand Down Expand Up @@ -208,8 +203,7 @@ switch_status_t switch_api_lag_member_create(
CHECK_RET(status != SWITCH_STATUS_SUCCESS, status);
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"LAG member create: Failed to get LAG member on device %d: "
"error: %s\n",
"LAG member create: Failed to get LAG member on device:%d error: %s\n",
device, switch_error_to_string(status));
status = switch_lag_member_handle_delete(device, *lag_member_h);
CHECK_RET(status != SWITCH_STATUS_SUCCESS, status);
Expand Down Expand Up @@ -303,29 +297,29 @@ switch_status_t switch_api_lag_update(const switch_device_t device,
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to delete rx_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}

// insert lag member
status = SWITCH_LIST_INSERT(&(lag_info->lag_members),
&(lag_member_info->node), lag_member_info);
status = SWITCH_LIST_INSERT(&lag_info->lag_members,
&lag_member_info->node, lag_member_info);
SWITCH_ASSERT(status == SWITCH_STATUS_SUCCESS);

// create rx path
status = switch_pd_rx_lag_table_entry(device, lag_info, true);
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to create rx_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
} else {
// update lag members list with the lag_member_h
status = SWITCH_LIST_INSERT(&(lag_info->lag_members),
&(lag_member_info->node), lag_member_info);
status = SWITCH_LIST_INSERT(&lag_info->lag_members,
&lag_member_info->node, lag_member_info);
SWITCH_ASSERT(status == SWITCH_STATUS_SUCCESS);
}
}
Expand All @@ -335,7 +329,7 @@ switch_status_t switch_api_lag_update(const switch_device_t device,

/**
* Routine Description:
* @brief On change of oper_state of a LAG member,
* @details On change of oper_state of a LAG member,
* update the number of active ports of a LAG and
* re-populate the Tx and Rx LAG table entries with
* active port.
Expand Down Expand Up @@ -386,18 +380,18 @@ switch_status_t switch_api_lag_member_update(const switch_device_t device,

// update the active num ports of LAG
if (oper_state)
lag_info->active_num_ports++;
lag_info->num_active_ports++;
else
lag_info->active_num_ports--;
lag_info->num_active_ports--;

if (lag_info->active_num_ports == 0 &&
if (lag_info->num_active_ports == 0 &&
(SWITCH_LIST_COUNT(&lag_info->lag_members) != 0)) {
// delete the existing tx and rx entries
status = switch_pd_tx_lacp_lag_table_entry(device, lag_info, false);
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to delete tx_lacp_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
Expand All @@ -406,28 +400,28 @@ switch_status_t switch_api_lag_member_update(const switch_device_t device,
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to delete rx_lacp_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}

// delete the lag_member from the LAG
status =
SWITCH_LIST_DELETE(&(lag_info->lag_members), &(lag_member_info->node));
status = SWITCH_LIST_DELETE(&lag_info->lag_members, &lag_member_info->node);
SWITCH_ASSERT(status == SWITCH_STATUS_SUCCESS);

} else if (lag_info->active_num_ports == 1) {
// from 2->1:oper_state is false or 0->1:oper_state is true
} else if (lag_info->num_active_ports == 1) {
/* oper_state false means num_active_ports is decremented from 2 to 1
and oper_state true means it is incremented from 0 to 1 */
if (oper_state) {
// populate the tx and rx entries with single active member
status = SWITCH_LIST_INSERT(&(lag_info->lag_members),
&(lag_member_info->node), lag_member_info);
status = SWITCH_LIST_INSERT(&lag_info->lag_members,
&lag_member_info->node, lag_member_info);
SWITCH_ASSERT(status == SWITCH_STATUS_SUCCESS);
status = switch_pd_tx_lacp_lag_table_entry(device, lag_info, true);
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to create tx_lacp_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
Expand All @@ -436,7 +430,7 @@ switch_status_t switch_api_lag_member_update(const switch_device_t device,
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to create rx_lacp_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
Expand All @@ -447,7 +441,7 @@ switch_status_t switch_api_lag_member_update(const switch_device_t device,
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to delete tx_lacp_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
Expand All @@ -456,20 +450,20 @@ switch_status_t switch_api_lag_member_update(const switch_device_t device,
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to delete rx_lacp_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}

status = SWITCH_LIST_DELETE(&(lag_info->lag_members),
&(lag_member_info->node));
status =
SWITCH_LIST_DELETE(&lag_info->lag_members, &lag_member_info->node);
SWITCH_ASSERT(status == SWITCH_STATUS_SUCCESS);

status = switch_pd_tx_lacp_lag_table_entry(device, lag_info, true);
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to create tx_lacp_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
Expand All @@ -478,19 +472,19 @@ switch_status_t switch_api_lag_member_update(const switch_device_t device,
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to create rx_lacp_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
}
} else if (lag_info->active_num_ports == 2) {
} else if (lag_info->num_active_ports == 2) {
// delete the existing entries and re-distribute
// table entries between both active ports
status = switch_pd_tx_lacp_lag_table_entry(device, lag_info, false);
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to delete tx_lacp_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
Expand All @@ -499,20 +493,20 @@ switch_status_t switch_api_lag_member_update(const switch_device_t device,
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to delete rx_lacp_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}

status = SWITCH_LIST_INSERT(&(lag_info->lag_members),
&(lag_member_info->node), lag_member_info);
status = SWITCH_LIST_INSERT(&lag_info->lag_members, &lag_member_info->node,
lag_member_info);
SWITCH_ASSERT(status == SWITCH_STATUS_SUCCESS);

status = switch_pd_tx_lacp_lag_table_entry(device, lag_info, true);
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to create tx_lacp_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
Expand All @@ -521,7 +515,7 @@ switch_status_t switch_api_lag_member_update(const switch_device_t device,
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to create rx_lacp_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
Expand Down Expand Up @@ -608,7 +602,7 @@ switch_status_t switch_api_program_lag_hw(const switch_device_t device,
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to create tx_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
Expand All @@ -618,7 +612,7 @@ switch_status_t switch_api_program_lag_hw(const switch_device_t device,
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to create rx_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
Expand All @@ -629,7 +623,7 @@ switch_status_t switch_api_program_lag_hw(const switch_device_t device,
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to delete tx_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
Expand All @@ -638,7 +632,7 @@ switch_status_t switch_api_program_lag_hw(const switch_device_t device,
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to delete rx_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
Expand All @@ -650,7 +644,7 @@ switch_status_t switch_api_program_lag_hw(const switch_device_t device,
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to create tx_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
Expand All @@ -659,7 +653,7 @@ switch_status_t switch_api_program_lag_hw(const switch_device_t device,
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to create rx_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
Expand All @@ -669,7 +663,7 @@ switch_status_t switch_api_program_lag_hw(const switch_device_t device,
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to delete tx_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
Expand All @@ -678,7 +672,7 @@ switch_status_t switch_api_program_lag_hw(const switch_device_t device,
if (status != SWITCH_STATUS_SUCCESS) {
krnlmon_log_error(
"Failed to delete rx_lag_table entry on device %d: "
",error: %s\n",
"error: %s\n",
device, switch_error_to_string(status));
return status;
}
Expand Down
4 changes: 2 additions & 2 deletions switchapi/switch_lag.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ extern "C" {
/*** LAG information ***/
typedef struct switch_api_lag_info_s {
uint32_t lag_ifindex;
uint8_t bond_mode;
uint8_t bond_mode; // see switchapi_bond_mode_t for values
uint32_t active_slave;
switch_handle_t rmac_handle;
} switch_api_lag_info_t;
Expand All @@ -70,7 +70,7 @@ typedef struct switch_lag_info_s {
switch_list_t lag_members;
switch_handle_t lag_handle;
switch_handle_t active_lag_member;
uint8_t active_num_ports;
uint8_t num_active_ports;
} switch_lag_info_t;

/*** LAG member information ***/
Expand Down
11 changes: 4 additions & 7 deletions switchlink/sai/switchlink_handle_lag.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ static int set_lag_attribute(const switchlink_db_interface_info_t* lag_info,

/**
* Routine Description:
* SAI call to set lag member attribute
* Calls SAI to set lag member attribute
*
* Arguments:
* [in] lag_member_info - lag member info
Expand All @@ -149,10 +149,7 @@ static int set_lag_member_attribute(
memset(&attr, 0, sizeof(attr));

attr.id = SAI_LAG_MEMBER_ATTR_EGRESS_DISABLE;
if (oper_state == IF_OPER_DOWN)
attr.value.booldata = false;
else
attr.value.booldata = true;
attr.value.booldata = (oper_state == IF_OPER_DOWN) ? false : true;
return sai_lag_api->set_lag_member_attribute(lag_member_info->lag_member_h,
&attr);
}
Expand Down Expand Up @@ -304,9 +301,9 @@ void switchlink_create_lag(switchlink_db_interface_info_t* lag_intf) {
}
}

if (memcmp(&(lag_info.mac_addr), &(lag_intf->mac_addr),
if (memcmp(&lag_info.mac_addr, &lag_intf->mac_addr,
sizeof(switchlink_mac_addr_t))) {
memcpy(&(lag_info.mac_addr), &(lag_intf->mac_addr),
memcpy(&lag_info.mac_addr, &lag_intf->mac_addr,
sizeof(switchlink_mac_addr_t));

// Delete if RMAC is configured previously, and create this new RMAC.
Expand Down

0 comments on commit 4270406

Please sign in to comment.