-
Notifications
You must be signed in to change notification settings - Fork 525
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
Reduce severity of log to info in case of flush on non-existing member #1669
Conversation
orchagent/fdborch.cpp
Outdated
if (type == SAI_FDB_EVENT_FLUSHED) | ||
{ | ||
/* in case of flush - can be ignored due to a race */ | ||
/* there are notifications about FDB FLUSH (syncd/sai_redis) on port which was already removed |
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.
Please combine the comments from previous line and have as a single block of comment.
orchagent/fdborch.cpp
Outdated
/* in case of flush - can be ignored due to a race */ | ||
/* there are notifications about FDB FLUSH (syncd/sai_redis) on port which was already removed | ||
* by orchagent as a result of removeVlanMember action (removeBridgePort) */ | ||
SWSS_LOG_INFO("Flush ignored. Received event on bridge port ID 0x%" PRIx64 " which is not exist.", |
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.
Can you please rephrase to "Flush event: Failed to get port by bridge port ID 0x%" PRIx64 "."
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.
I can :) but, IMO its better to avoid to use word "Failed"in INFO notification.
What do you think?
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.
Agree, but this has been the model for some exceptions.. For e.g check this. IMO, even though it is INFO, it is still a failure in fetch the data.
Also can you please respond to sonic-net/sonic-buildimage#6936, before the PR merge? |
@allas-nvidia please align the code based on the above comments. |
315f16a
to
066675a
Compare
Signed-off-by: allas <allas@nvidia.com>
066675a
to
7a1a4d3
Compare
#1669) - What I did Reduced severity of log to info in case of flush on non-existing member. - Why I did it There is a race in the infra which can cause the scenario: FDB FLUSH notification is received on port which was already removed. - How I verified it > sudo config vlan add 3 > sudo config vlan member add 3 Ethernet8 > sudo config vlan member del 3 Ethernet8 Signed-off-by: allas <allas@nvidia.com>
sonic-net#1669) - What I did Reduced severity of log to info in case of flush on non-existing member. - Why I did it There is a race in the infra which can cause the scenario: FDB FLUSH notification is received on port which was already removed. - How I verified it > sudo config vlan add 3 > sudo config vlan member add 3 Ethernet8 > sudo config vlan member del 3 Ethernet8 Signed-off-by: allas <allas@nvidia.com>
What I did HLD for Dump Utility: HLD. For More info on Tech Support Addition: TechSupport How I did it Module Names are retrieved by parsing the output of dump state --show command.
What I did Implemented vlan and vlan_member modules for debug dump utility. How I did it Used infrastructure and followed examples in sonic-net#1666 sonic-net#1667 sonic-net#1668 sonic-net#1669 sonic-net#1670 How to verify it On switch: dump state vlan <vlan_name> dump state vlan_member '<vlan_name|<member_name>' Unit test: pytest-3 dump_tests/module_tests/vlan_test.py (same test file covers both vlan and vlan_member)
Signed-off-by: allas allas@nvidia.com
What I did
Reduced severity of log to info in case of flush on non-existing member.
Why I did it
There is a race in the infra which can cause the scenario: FDB FLUSH notification is received on port which was already removed.
How I verified it
> sudo config vlan add 3
> sudo config vlan member add 3 Ethernet8
> sudo config vlan member del 3 Ethernet8
Details if related