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

Deactivate mirror session only when status is true in updateLagMember #1666

Merged
merged 1 commit into from
Mar 22, 2021

Conversation

shi-su
Copy link
Contributor

@shi-su shi-su commented Mar 9, 2021

What I did
Deactivate mirror session only when status is true in updateLagMember

Why I did it
Fix the issue when mirrororch tries to deactivate a session when it is not activated

Mar  7 18:22:03.125662 e07265d2546e NOTICE #orchagent: :- removeLagMember: Remove member Ethernet88 from LAG PortChannel008 lid:20000000005ff lmid:1b000000000600
Mar  7 18:22:03.125691 e07265d2546e ERR #orchagent: :- meta_sai_validate_oid: object key SAI_OBJECT_TYPE_MIRROR_SESSION:oid:0xe000000000603 doesn't exist
Mar  7 18:22:03.125695 e07265d2546e ERR #orchagent: :- deactivateSession: Failed to deactivate mirroring session TEST_SESSION
Mar  7 18:22:04.126757 e07265d2546e NOTICE #orchagent: :- removeLag: Remove LAG PortChannel008 lid:20000000005ff

How I verified it
Verified that the above error message does not show up in dvs test

Details if related

@shi-su shi-su marked this pull request as ready for review March 9, 2021 04:36
@shi-su shi-su requested a review from qiluo-msft March 9, 2021 18:24
@@ -1342,7 +1342,10 @@ void MirrorOrch::updateLagMember(const LagMemberUpdate& update)
// If LAG is empty, deactivate session
if (update.lag.m_members.empty())
{
deactivateSession(name, session);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it requested by end user? If yes, the error message is expected.

Copy link
Contributor Author

@shi-su shi-su Mar 10, 2021

Choose a reason for hiding this comment

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

It seems that the session got deactivated twice, and we should avoid deactivating the same session once it is already deactivated. I think checking the session status should be a valid way to do the job. The function deactivateSession() even has an assertion that session status is true at the beginning of the function.

@lguohan
Copy link
Contributor

lguohan commented Mar 19, 2021

is there an issue associated with this, if yes, can you link the issue to this pr?

@shi-su
Copy link
Contributor Author

shi-su commented Mar 19, 2021

is there an issue associated with this, if yes, can you link the issue to this pr?

I did not create an issue for this one. Noticed it when adding on-par SAI failure handling and created this PR for fix.

@shi-su shi-su merged commit a0a7e9a into sonic-net:master Mar 22, 2021
shi-su added a commit to shi-su/sonic-swss that referenced this pull request Aug 18, 2021
…agMember (sonic-net#1666)

Deactivate mirror session only when the status is true in updateLagMember
shi-su added a commit that referenced this pull request Aug 18, 2021
What I did
Backport SAI failure handling related commits into the 202012 branch. The following is a list of backported commits:

941875a Deactivate mirror session only when session status is true in updateLagMember (#1666)
be12482 Ignore ALREADY_EXIST error in FDB creation (#1815)
c9c1aa2 Add failure handling for SAI get operations (#1768)
47b4276 [BufferOrch] Don't call SAI API for BUFFER_POOL/PROFILE handling in case the op is DEL and the SAI OID is NULL (#1786)
db9238f Add failure notification for orchagent (#1665)
fc8e43f [synchronous mode] Add failure notification for SAI failures in synchronous mode (#1596)

Why I did it
202012 image needs to include failure handling mechanism for enough notification in the presence of SAI failures.
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
…agMember (sonic-net#1666)

Deactivate mirror session only when the status is true in updateLagMember
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
What I did
HLD for Dump Utility: HLD.

Added the Logic for Match Infrastructure along with corresponding unit tests.
Note: Before merging other PR's, please merge this first

For More Info on MatchRequest and MatchEngine, Check this section in the HLD:
MatchInfra
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants