Remove system neigh DEL operation if SET operation succeeds#2853
Remove system neigh DEL operation if SET operation succeeds#2853prsunny merged 1 commit intosonic-net:masterfrom
Conversation
|
Add @arlakshm @kenneth-arista for visibility. |
|
This fix will likely also address sonic-net/sonic-mgmt#8588 |
|
@ysmanman, can you rebase the branch? |
|
/Azpw run Azure.sonic-swss |
|
/AzurePipelines run Azure.sonic-swss |
|
No commit pushedDate could be found for PR 2853 in repo sonic-net/sonic-swss |
the same neighbor succeeds. This is to avoid mistakenly removing the system neighbor. Fix sonic-net/sonic-buildimage#15266.
|
@prsunny could we have an ADO to track this? |
@StormLiangMS @prsunny this is the ADO# 24596307 |
…sonic-net#2853) *Remove system neighbor DEL operation in m_toSync if SET operation for the same neighbor succeeds. This is to avoid mistakenly removing the system neighbor. Fix sonic-net/sonic-buildimage#15266.
| */ | ||
| auto rit = make_reverse_iterator(it); | ||
| while (rit != consumer.m_toSync.rend() && rit->first == key && kfvOp(rit->second) == DEL_COMMAND) | ||
| { |
There was a problem hiding this comment.
i thought m_toSync will only have one entry for each neighbor, i didn't know if we have multiple entry for the same neighbor, just curious.
There was a problem hiding this comment.
why this problem is voq neighbor specificially? for normal neighbor, why we do not have such problem?
There was a problem hiding this comment.
i thought m_toSync will only have one entry for each neighbor, i didn't know if we have multiple entry for the same neighbor, just curious.
m_toSync is type of std::multimap, so it allows multiple actions (like DEL and SET) for the same neighbor.
There was a problem hiding this comment.
why this problem is voq neighbor specificially? for normal neighbor, why we do not have such problem?
This problem is not voq specific. The same change/fix exists for local neighbor too. The PR is to apply the same fix for voq.
| * Since DEL operation is supposed to be executed before SET for the same neighbor | ||
| * A remaining DEL after the SET operation means the DEL operation failed previously and should not be executed anymore | ||
| */ | ||
| auto rit = make_reverse_iterator(it); |
…#2853) *Remove system neighbor DEL operation in m_toSync if SET operation for the same neighbor succeeds. This is to avoid mistakenly removing the system neighbor. Fix sonic-net/sonic-buildimage#15266.
…#2853) *Remove system neighbor DEL operation in m_toSync if SET operation for the same neighbor succeeds. This is to avoid mistakenly removing the system neighbor. Fix sonic-net/sonic-buildimage#15266.
…sonic-net#2853) *Remove system neighbor DEL operation in m_toSync if SET operation for the same neighbor succeeds. This is to avoid mistakenly removing the system neighbor. Fix sonic-net/sonic-buildimage#15266.
8 participants
What I did
Remove system neighbor DEL operation in m_toSync if SET operation for the same neighbor succeeds. This is to avoid mistakenly removing the system neighbor.
Why I did it
Fix sonic-net/sonic-buildimage#15266
How I verified it
sonic-net/sonic-buildimage#15266 was uncovered by https://github.com/sonic-net/sonic-mgmt/blob/master/tests/voq/test_voq_disrupts.py. The issue was not seen in the test anymore with the fix.
Details if related