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

Add BGP Suppress FIB Pending test plan #7475

Merged
merged 1 commit into from
May 24, 2023

Conversation

echuawu
Copy link
Contributor

@echuawu echuawu commented Feb 15, 2023

Add BGP Suppress FIB Pending test plan

Description of PR

Add test plan for BGP Suppress FIB Pending function added by Nvidia network team.

Summary:
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Back port request

  • 201911
  • 202012
  • 202205

Approach

What is the motivation for this PR?

#7430
Add BGP Suppress FIB Pending test plan for the test script.

Add BGP Suppress FIB Pending test plan

Change-Id: I1be42ed1bcddf0ad7ea9c45a809a89583a6554dd
@liat-grozovik
Copy link
Collaborator

@StormLiangMS @prsunny could you please suggest who should review this test plan as well?

@liat-grozovik
Copy link
Collaborator

/azp run Azure.sonic-mgmt

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

@StormLiangMS could you please help to review?

Also, in the following scenario, a credit loop occurs:
###### Figure 1. Use case scenario

![Use case scenario](Img/Bgp_Suppress_FIB_Pending_Use_Case.png)
Copy link
Collaborator

Choose a reason for hiding this comment

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

could we add stress and performance test?

  1. Stress, to announce and withdraw 1000 routes for 5 or 10 times, then check the routes consistency.
  2. performance, what's the delay we should expect here for like 1000 routes?

Copy link
Contributor Author

@echuawu echuawu Apr 17, 2023

Choose a reason for hiding this comment

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

For the stress test, we have case 4 to enable bgp-suppress-fib-pending function and run all the community bgp cases, it's including stress and consistency test. For performance test, we have another team to cover it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @StormLiangMS, I checked the test_bgp_update_timer.py script, it's the existing test for validating dut performance on handling single bgp update(single route for one time and loop test for 5 routes).
I have two concerns on using exabgp and tcpdump to implement performance test:

  1. How to send a large scale bgp updates in a short time by exabgp?
  2. If it could, then, using tcpdump to capture ingress and egress 1K bgp updates at the same time, does the tcpdump capture result reliable enough?
    Please share your insights.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@echuawu I think it is ok to have separate PR for performance and stress ones, could you open an issue to track this ask?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@StormLiangMS , thank you, and the issue has been created to track the new PR: https://github.com/sonic-net/sonic-buildimage/issues/15194

Copy link
Collaborator

Choose a reason for hiding this comment

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

@echuawu could you add this new PR info to the sonic-net/SONiC#1103 to make sure it get well tracked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @StormLiangMS , Sure done.

@liat-grozovik
Copy link
Collaborator

@echuawu @StormLiangMS any further open items that need to be handled or this PR can be approved?

@echuawu
Copy link
Contributor Author

echuawu commented May 11, 2023

@echuawu @StormLiangMS any further open items that need to be handled or this PR can be approved?

Hi @liat-grozovik , @StormLiangMS suggest to add a performance test. We have internal discussion about it, and decided to add another PR to handle the requrement.

Copy link
Collaborator

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

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

LGTM

@StormLiangMS StormLiangMS merged commit 8b40071 into sonic-net:master May 24, 2023
mrkcmo pushed a commit to Azarack/sonic-mgmt that referenced this pull request Oct 3, 2023
Add BGP Suppress FIB Pending test plan

Change-Id: I1be42ed1bcddf0ad7ea9c45a809a89583a6554dd
AharonMalkin pushed a commit to AharonMalkin/sonic-mgmt that referenced this pull request Jan 25, 2024
Add BGP Suppress FIB Pending test plan

Change-Id: I1be42ed1bcddf0ad7ea9c45a809a89583a6554dd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants