-
Notifications
You must be signed in to change notification settings - Fork 6
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
LNW nexthop changes for LAG scenario #155
Conversation
Signed-off-by: Sabeel Ansari <[email protected]>
Signed-off-by: Sabeel Ansari <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR needs a good changelist comment.
See Writing good CL descriptions for an excellent description of how to do this.
I'd recommend doing this in the comment box at the top of the PR, and then copy/pasting it into the comment box when merging the PR.
See my second comment for ideas on what information you might provide.
Signed-off-by: Sabeel Ansari <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
status = tdi_data_field_set_value(data_hdl, rif_data_field_id, lag_id); | ||
if (status != TDI_SUCCESS) { | ||
krnlmon_log_error("Unable to set action value for ID: %d, error: %d", | ||
data_field_id, status); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data_field_id, --> change this to rif_data_field_id for correct error message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened PR #156 to address this
&rif_data_field_id); | ||
if (status != TDI_SUCCESS) { | ||
krnlmon_log_error("Unable to get data field id param for: %s, error: %d", | ||
ACTION_SET_EGRESS_PORT_PARAM_EGRESS_PORT, status); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACTION_SET_EGRESS_PORT_PARAM_EGRESS_PORT -> Change this to ACTION_SET_EGRESS_PORT_PARAM_ROUTER_INTF_ID for correct error message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened PR #156 to address this
router_interface_id
argument fromset_nexthop_lag
action innexthop_table
.router_interface_id
withset_egress_port
action fortx_lag_table
to resolve source MAC address population for LAG scenario.This aligns with recent changes in the P4 program to address a LAG bug.