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

[nbrmgrd] added function to parse IP address from APP_DB #1672

Merged

Conversation

vadymhlushko-mlnx
Copy link
Contributor

@vadymhlushko-mlnx vadymhlushko-mlnx commented Mar 16, 2021

Signed-off-by: Vadym Hlushko vadymh@nvidia.com

What I did
Added specific function to parse keys from APP_DB which looks like:
"NEIGH_RESOLVE_TABLE:Vlan2:2000:1::1"
"NEIGH_RESOLVE_TABLE:Vlan2:192.168.0.10"

Why I did it
The fix for the issues/7045.

How I verified it
Provided the VS tests.
Also, it was verified manually, error messages and crash from the issues/7045 wasn't observed.

Signed-off-by: Vadym Hlushko <vadymh@nvidia.com>
Copy link
Collaborator

@prsunny prsunny left a comment

Choose a reason for hiding this comment

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

changes lgtm.. as we discussed, we can add a simple VS test to add to APP_DB and verify. Recommend to have the test covered as part of the same PR

Signed-off-by: Vadym Hlushko <vadymh@nvidia.com>
time.sleep(2)

(exitcode, output) = dvs.runcmd(['sh', '-c', "supervisorctl status nbrmgrd | awk '{print $2}'"])
assert output == "RUNNING\n"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it ok if you can also check whether the req has been sent out? Like parsing ip neigh show for 2000:1::1. This way we can verify the functionality. It might be in 'failed' state which is ok.

Copy link
Contributor Author

@vadymhlushko-mlnx vadymhlushko-mlnx Mar 18, 2021

Choose a reason for hiding this comment

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

The ip neigh show command produced the empty output, it seems like because in test_FlushResolveNeighborIpv6 we don't test the whole flow of flushing the neighbor entry, because it is impossible to do on the VS.

When we did testing on the VS we actually saw a crash from the issues/7045 and with this fix the crash wasn't observed.

So, in any case, I believe this verification should be enough to cover the changes, even if we are not able to check ip neigh show.

@vadymhlushko-mlnx
Copy link
Contributor Author

/AzurePipelines run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 1672 in repo Azure/sonic-swss

@liat-grozovik
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny prsunny merged commit 48dc60e into sonic-net:master Mar 18, 2021
daall pushed a commit that referenced this pull request Mar 19, 2021
* [nbrmgrd] added function to parse IP address from APP_DB
Signed-off-by: Vadym Hlushko <vadymh@nvidia.com>
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
)

* [nbrmgrd] added function to parse IP address from APP_DB
Signed-off-by: Vadym Hlushko <vadymh@nvidia.com>
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
…onic-net#1672)

What I did
Pcieutil to load the platform api first instead of using common api
Some platform device with different BIOS version needs more than one pcie configuration to check the pcie devices properly.
Please refer to the platform api support : sonic-net/sonic-platform-common#195

How I did it
Load the platform pcie api first prior to use the common api
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants