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

[neighorch]: Return false when the next hop IP exists before adding #1032

Closed
wants to merge 1 commit into from

Conversation

stcheng
Copy link
Contributor

@stcheng stcheng commented Aug 19, 2019

It is possible that due to events reordering, a different next hop
entry with same IP will be inserted before the previous next hop
entry gets removed. Thus, return false here to prevent the new entry
from replacing the old one automatically.

Signed-off-by: Shu0T1an ChenG shuche@microsoft.com

@@ -30,6 +30,13 @@ bool NeighOrch::addNextHop(IpAddress ipAddress, string alias)
{
SWSS_LOG_ENTER();

if (hasNextHop(ipAddress))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you want to take care of out-of-sequence in the delete case as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

any update on this @stcheng ?

It is possible that due to events reordering, a different next hop
entry with same IP will be inserted before the previous next hop
entry gets removed. Thus, return false here to prevent the new entry
from replacing the old one automatically.

Update the symmetric removeNextHop() function as well.

Signed-off-by: Shu0T1an ChenG <shuche@microsoft.com>
@stcheng
Copy link
Contributor Author

stcheng commented Sep 17, 2019

updated

@stcheng
Copy link
Contributor Author

stcheng commented Sep 17, 2019

Since the pull request #977 could fix this issue ultimately, this pull request will be closed.

@stcheng stcheng closed this Sep 17, 2019
@stcheng stcheng deleted the neighorch branch September 17, 2019 16:35
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