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

[BFD]Retry create BFD with different source UDP port on failure #2225

Merged
merged 5 commits into from
Apr 7, 2022

Conversation

dgsudharsan
Copy link
Collaborator

What I did
The create_bfd_session API can fail when a source port already used by a different application is passed to SAI create. To avoid failure, a retry logic is added to try with a different port number three times before failing

Why I did it
To avoid BFD failure and orchagent crash due to port conflict.

How I verified it
Verified it manually by having an application binding the port and checking if retry is done when BFD tries the same port

Details if related

The create_bfd_session API can fail when a source port already used by a different application is passed to SAI create. To avoid failure, a retry logic is added to try with a different port number three times before failing
orchagent/bfdorch.cpp Outdated Show resolved Hide resolved
@dgsudharsan dgsudharsan requested a review from prsunny April 7, 2022 00:09
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.

are you planning to add any VS test. code coverage may fail if the additional code is not covered by test

orchagent/bfdorch.cpp Outdated Show resolved Hide resolved
@dgsudharsan dgsudharsan requested a review from prsunny April 7, 2022 02:47
@liat-grozovik
Copy link
Collaborator

LGTM. small comment
@prsunny adding a code coverage for that will requires mock and not regular functional test. we need to make sure the port is used by other application and then try to create bfd. Do you still think adding a mock in the VS test is the right way? it is not unit test.

@liat-grozovik
Copy link
Collaborator

/azp run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny prsunny merged commit d1fb3dd into sonic-net:master Apr 7, 2022
qiluo-msft pushed a commit that referenced this pull request Apr 11, 2022
* [BFD]Retry create BFD with different source UDP port on failure. The create_bfd_session API can fail when a source port already used by a different application is passed to SAI create. To avoid failure, a retry logic is added to try with a different port number three times before failing
dprital pushed a commit to dprital/sonic-swss that referenced this pull request May 8, 2022
…c-net#2225)

* [BFD]Retry create BFD with different source UDP port on failure. The create_bfd_session API can fail when a source port already used by a different application is passed to SAI create. To avoid failure, a retry logic is added to try with a different port number three times before failing
@prsunny
Copy link
Collaborator

prsunny commented Jun 15, 2022

@judyjoseph , can you please cherry-pick to 202111?

judyjoseph pushed a commit that referenced this pull request Jun 20, 2022
* [BFD]Retry create BFD with different source UDP port on failure. The create_bfd_session API can fail when a source port already used by a different application is passed to SAI create. To avoid failure, a retry logic is added to try with a different port number three times before failing
preetham-singh pushed a commit to preetham-singh/sonic-swss that referenced this pull request Aug 6, 2022
…c-net#2225)

* [BFD]Retry create BFD with different source UDP port on failure. The create_bfd_session API can fail when a source port already used by a different application is passed to SAI create. To avoid failure, a retry logic is added to try with a different port number three times before failing
@dgsudharsan dgsudharsan deleted the bfd_retry branch March 9, 2023 02:01
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.

5 participants