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.

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

Conversation

n-sandeep
Copy link
Contributor

@n-sandeep n-sandeep commented Jun 20, 2024

Issues fixed are:

  • Populate tx_lag_table, modify exact to ternary match for this table.
  • Populate RIF tables
  • Populate correct dmac_high and dmac_low fields correctly
  • Handle LACP bond interface delete and re-add

Issues fixed are:
Populate tx_lag_table, modify exact t ternary match for this table.
Populate RIF tables
Populate correct dmac_high and dmac_low fields correctly
Handle LACP bond interface delete and re-add

Signed-off-by: Sandeep N <[email protected]>
Copy link
Collaborator

@5abeel 5abeel left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -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.

Copy link
Contributor

@ffoulkes ffoulkes left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -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
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.

@@ -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);
}

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) {

@ffoulkes ffoulkes merged commit 7bb5797 into main Jun 20, 2024
4 checks passed
@ffoulkes ffoulkes deleted the fix_p4cp_lag_nexthop_issues branch June 20, 2024 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants