[restore_neigh] Check STATE_DB before sending ARP/ND pkts for neighbors associated with PortChannel #2444
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
dgsudharsan
approved these changes
Sep 9, 2022
prsunny
reviewed
Sep 10, 2022
| return True | ||
| key = db.keys(db.STATE_DB, table_name) | ||
| if key is None: | ||
| log_info ("members for {} are not yet created".format(intf)) |
There was a problem hiding this comment.
Please keep the log for Vlan as before and add a new one for LAG.
There was a problem hiding this comment.
Why, are there any log checkers matching for this string?
There was a problem hiding this comment.
Not sure. but its been a very legacy code. So just thot of being safe. Approving from my side
prsunny
approved these changes
Sep 10, 2022
| return True | ||
| key = db.keys(db.STATE_DB, table_name) | ||
| if key is None: | ||
| log_info ("members for {} are not yet created".format(intf)) |
There was a problem hiding this comment.
Not sure. but its been a very legacy code. So just thot of being safe. Approving from my side
vivekrnv
added a commit
to vivekrnv/sonic-swss
that referenced
this pull request
Sep 14, 2022
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com> Check STATE_DB before sending ARP/ND pkts for neighbors associated with PortChannel. As a part of intf check, wait for the LAG_MEMBER_TABLE to be populated for Portchannels ifaces
7 tasks
dgsudharsan
pushed a commit
that referenced
this pull request
Sep 16, 2022
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com> Check STATE_DB before sending ARP/ND pkts for neighbors associated with PortChannel. As a part of intf check, wait for the LAG_MEMBER_TABLE to be populated for Portchannels ifaces
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Vivek Reddy Karri vkarri@nvidia.com
What I did
As a part of intf check, wait for the LAG_MEMBER_TABLE to be populated for PortChannel ifaces
Why I did it
Currently, restore_neighbors just checks the
/sys/class/net/{0}/carrierto determine of if the status is up, but during WR teamdstarts very early. It creates the netdev from the prev-life state and as i understand sets the carrier state to be based on what was found in the prev-life dump. Ref: https://github.com/sonic-net/sonic-buildimage/blob/master/src/libteam/patch/0008-libteam-Add-warm_reboot-mode.patch#L442
Checking the sysfs attribute is not a reliable option. Thus during WR because of this false-positive, restore_neighbor sends the ARP/NS very early i.e. before anything else is created, resulting in neighbor not being resolved in the end. This results in problems such as this
https://github.com/sonic-net/sonic-buildimage/issues/12029How I verified it
On the live switch:
Verified logs and checked if the ND/ARP msgs are received on the other end:
Details if related