Skip to content

Conversation

@echuawu
Copy link
Contributor

@echuawu echuawu commented Jun 3, 2025

Description of PR

Including configuration and function coverage:

1.Auto generate 128 SRv6 locators and sids
2.Auto generate 128 matching packets
2.1.Test DIP shift
2.2.Test uSID container copy action
2.3.Test USD flavor
2.4.Test DSCP mapping
2.5.Test ttl and hop limit change
3.Split SRH and non SRH packets test into two test cases required by MSFT 3.1.Test reboot and BGP restart test randomly to save time 4.Test SRv6 counter
5.Test SRv6 CRM resource
6.Add static route for 128 sids validation
7.Add check for route asic db installation
8.Add timeout when captureing packet to override large scale of BGP update packets affection 9.Use a unified locator block for all the sids
10.Use command line to set SRv6 packet type

This PR should be merged after the design PRs merged:

Summary:
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • New Test case
    • Skipped for non-supported platforms
  • Test case improvement

Back port request

  • 202205
  • 202305
  • 202311
  • 202405
  • 202411
  • 202505

Approach

What is the motivation for this PR?

Enhance the SRv6 test coverage for the counter and CRM and 128 my sids

How did you do it?

Add validations for SRv6 counter and CRM and 128 my sids

How did you verify/test it?

Run it in local setup

Any platform specific information?

Mellanox SPC4 and later platforms

Supported testbed topology if it's a new test case?

Documentation

Copy link
Contributor

@BYGX-wcr BYGX-wcr left a comment

Choose a reason for hiding this comment

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

LGTM

@r12f
Copy link
Contributor

r12f commented Jun 3, 2025

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 18775 in repo sonic-net/sonic-mgmt

@abdosi
Copy link
Contributor

abdosi commented Jun 3, 2025

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@BYGX-wcr
Copy link
Contributor

BYGX-wcr commented Jun 3, 2025

@echuawu, please check the formatting issue.

Copy link
Contributor

@BYGX-wcr BYGX-wcr left a comment

Choose a reason for hiding this comment

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

You should use wait_until to validate BGP sessions to avoid failures because BGP daemon not up in time.

@echuawu
Copy link
Contributor Author

echuawu commented Jun 4, 2025

You should use wait_until to validate BGP sessions to avoid failures because BGP daemon not up in time.

There is wait_until during validation of bgp sessions:
def _validate_bgp_session(self, duthost):
config_facts = duthost.config_facts(host=duthost.hostname, source="running")['ansible_facts']
bgp_neighbors = config_facts.get('BGP_NEIGHBOR', {})
pytest_assert(
wait_until(300, 10, 0, duthost.check_bgp_session_state, bgp_neighbors),
"bgp sessions {} are not up".format(bgp_neighbors)
)

@echuawu
Copy link
Contributor Author

echuawu commented Jun 4, 2025

@echuawu, please check the formatting issue.

Fixed

@BYGX-wcr
Copy link
Contributor

BYGX-wcr commented Jun 4, 2025

You should use wait_until to validate BGP sessions to avoid failures because BGP daemon not up in time.

There is wait_until during validation of bgp sessions: def _validate_bgp_session(self, duthost): config_facts = duthost.config_facts(host=duthost.hostname, source="running")['ansible_facts'] bgp_neighbors = config_facts.get('BGP_NEIGHBOR', {}) pytest_assert( wait_until(300, 10, 0, duthost.check_bgp_session_state, bgp_neighbors), "bgp sessions {} are not up".format(bgp_neighbors) )

That is not sufficient. When I ran this in our lab, this function triggered a runtime error because the BGP daemon was not up yet when the duthost.check_bgp_session_state calls bgp_facts.

@echuawu
Copy link
Contributor Author

echuawu commented Jun 4, 2025

You should use wait_until to validate BGP sessions to avoid failures because BGP daemon not up in time.

There is wait_until during validation of bgp sessions: def _validate_bgp_session(self, duthost): config_facts = duthost.config_facts(host=duthost.hostname, source="running")['ansible_facts'] bgp_neighbors = config_facts.get('BGP_NEIGHBOR', {}) pytest_assert( wait_until(300, 10, 0, duthost.check_bgp_session_state, bgp_neighbors), "bgp sessions {} are not up".format(bgp_neighbors) )

That is not sufficient. When I ran this in our lab, this function triggered a runtime error because the BGP daemon was not up yet when the duthost.check_bgp_session_state calls bgp_facts.

Do you have any suggestion on this check? Just do not use duthost.check_bgp_session_state?

@echuawu
Copy link
Contributor Author

echuawu commented Jun 4, 2025

You should use wait_until to validate BGP sessions to avoid failures because BGP daemon not up in time.

There is wait_until during validation of bgp sessions: def _validate_bgp_session(self, duthost): config_facts = duthost.config_facts(host=duthost.hostname, source="running")['ansible_facts'] bgp_neighbors = config_facts.get('BGP_NEIGHBOR', {}) pytest_assert( wait_until(300, 10, 0, duthost.check_bgp_session_state, bgp_neighbors), "bgp sessions {} are not up".format(bgp_neighbors) )

That is not sufficient. When I ran this in our lab, this function triggered a runtime error because the BGP daemon was not up yet when the duthost.check_bgp_session_state calls bgp_facts.

Do you have any suggestion on this check? Just do not use duthost.check_bgp_session_state?

Before the _validate_bgp_session, there is a bgp service check, isn't it enough?

with allure.step('Validate BGP docker UP'):
                    pytest_assert(wait_until(100, 10, 0, rand_selected_dut.is_service_fully_started_per_asic_or_host,
                                             "bgp"),
                                  "BGP not started.")

@BYGX-wcr
Copy link
Contributor

BYGX-wcr commented Jun 4, 2025

You should use wait_until to validate BGP sessions to avoid failures because BGP daemon not up in time.

There is wait_until during validation of bgp sessions: def _validate_bgp_session(self, duthost): config_facts = duthost.config_facts(host=duthost.hostname, source="running")['ansible_facts'] bgp_neighbors = config_facts.get('BGP_NEIGHBOR', {}) pytest_assert( wait_until(300, 10, 0, duthost.check_bgp_session_state, bgp_neighbors), "bgp sessions {} are not up".format(bgp_neighbors) )

That is not sufficient. When I ran this in our lab, this function triggered a runtime error because the BGP daemon was not up yet when the duthost.check_bgp_session_state calls bgp_facts.

Do you have any suggestion on this check? Just do not use duthost.check_bgp_session_state?

Before the _validate_bgp_session, there is a bgp service check, isn't it enough?

with allure.step('Validate BGP docker UP'):
                    pytest_assert(wait_until(100, 10, 0, rand_selected_dut.is_service_fully_started_per_asic_or_host,
                                             "bgp"),
                                  "BGP not started.")

at least not enough in our lab, you can skip checking BGP sessions and add more wait time for checking if the BGP routes are synced

@echuawu
Copy link
Contributor Author

echuawu commented Jun 4, 2025

You should use wait_until to validate BGP sessions to avoid failures because BGP daemon not up in time.

There is wait_until during validation of bgp sessions: def _validate_bgp_session(self, duthost): config_facts = duthost.config_facts(host=duthost.hostname, source="running")['ansible_facts'] bgp_neighbors = config_facts.get('BGP_NEIGHBOR', {}) pytest_assert( wait_until(300, 10, 0, duthost.check_bgp_session_state, bgp_neighbors), "bgp sessions {} are not up".format(bgp_neighbors) )

That is not sufficient. When I ran this in our lab, this function triggered a runtime error because the BGP daemon was not up yet when the duthost.check_bgp_session_state calls bgp_facts.

Do you have any suggestion on this check? Just do not use duthost.check_bgp_session_state?

Before the _validate_bgp_session, there is a bgp service check, isn't it enough?

with allure.step('Validate BGP docker UP'):
                    pytest_assert(wait_until(100, 10, 0, rand_selected_dut.is_service_fully_started_per_asic_or_host,
                                             "bgp"),
                                  "BGP not started.")

at least not enough in our lab, you can skip checking BGP sessions and add more wait time for checking if the BGP routes are synced

Updated, please review.

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1.Auto generate 128 SRv6 locators and sids
2.Auto generate 128 matching packets
2.1.Test DIP shift
2.2.Test uSID container copy action
2.3.Test USD flavor
2.4.Test DSCP mapping
2.5.Test ttl and hop limit change
3.Split SRH and non SRH packets test into two test cases required by MSFT
3.1.Test reboot and BGP restart test randomly to save time
4.Test SRv6 counter
5.Test SRv6 CRM resource
6.Add static route for 128 sids validation
7.Add check for route asic db installation
8.Add timeout when captureing packet to override large scale of BGP update packets affection
9.Use a unified locator block for all the sids
10.Use command line to set SRv6 packet type
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@BYGX-wcr BYGX-wcr left a comment

Choose a reason for hiding this comment

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

LGTM

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@echuawu
Copy link
Contributor Author

echuawu commented Jun 9, 2025

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@abdosi abdosi enabled auto-merge (squash) June 9, 2025 16:48
@abdosi abdosi merged commit 44bd8ce into sonic-net:master Jun 10, 2025
19 checks passed
@r12f
Copy link
Contributor

r12f commented Jun 11, 2025

hi @echuawu , do you mind to help manual picking this change to 202412?

mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Jun 12, 2025
1.Auto generate 128 SRv6 locators and sids
2.Auto generate 128 matching packets
2.1.Test DIP shift
2.2.Test uSID container copy action
2.3.Test USD flavor
2.4.Test DSCP mapping
2.5.Test ttl and hop limit change
3.Split SRH and non SRH packets test into two test cases required by MSFT
3.1.Test reboot and BGP restart test randomly to save time
4.Test SRv6 counter
5.Test SRv6 CRM resource
6.Add static route for 128 sids validation
7.Add check for route asic db installation
8.Add timeout when captureing packet to override large scale of BGP update packets affection
9.Use a unified locator block for all the sids
10.Use command line to set SRv6 packet type
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202505: #18945

@echuawu
Copy link
Contributor Author

echuawu commented Jun 12, 2025

hi @echuawu , do you mind to help manual picking this change to 202412?

Please check Azure/sonic-mgmt.msft#396

mssonicbld pushed a commit that referenced this pull request Jun 14, 2025
1.Auto generate 128 SRv6 locators and sids
2.Auto generate 128 matching packets
2.1.Test DIP shift
2.2.Test uSID container copy action
2.3.Test USD flavor
2.4.Test DSCP mapping
2.5.Test ttl and hop limit change
3.Split SRH and non SRH packets test into two test cases required by MSFT
3.1.Test reboot and BGP restart test randomly to save time
4.Test SRv6 counter
5.Test SRv6 CRM resource
6.Add static route for 128 sids validation
7.Add check for route asic db installation
8.Add timeout when captureing packet to override large scale of BGP update packets affection
9.Use a unified locator block for all the sids
10.Use command line to set SRv6 packet type
mssonicbld added a commit to mssonicbld/sonic-mgmt.msft that referenced this pull request Jul 9, 2025
<!--
Please make sure you've read and understood our contributing guidelines;
https://github.com/sonic-net/SONiC/blob/gh-pages/CONTRIBUTING.md

Please provide following information to help code review process a bit easier:
-->
### Description of PR
1.Add more timeout for ptf to handle a large scale of bgp packets after config reload/bgp restart/reboot
2.Add BGP route sync check
This PR would based on the sonic-net/sonic-mgmt#18775

Summary:
Fixes # (issue)

### Type of change

<!--
- Fill x for your type of change.
- e.g.
- [x] Bug fix
-->

- [ ] Bug fix
- [ ] Testbed and Framework(new/improvement)
- [ ] New Test case
    - [ ] Skipped for non-supported platforms
- [x] Test case improvement

### Back port request
- [ ] 202205
- [ ] 202305
- [ ] 202311
- [ ] 202405
- [ ] 202411
- [ ] 202505

### Approach
#### What is the motivation for this PR?
Case not stable
#### How did you do it?
1.Add more timeout for ptf to handle a large scale of bgp packets after config reload/bgp restart/reboot
2.Add BGP route sync check
#### How did you verify/test it?
Run it in local setup
#### Any platform specific information?

#### Supported testbed topology if it's a new test case?

### Documentation
<!--
(If it's a new feature, new test case)
Did you update documentation/Wiki relevant to your implementation?
Link to the wiki page?
-->
mssonicbld added a commit to Azure/sonic-mgmt.msft that referenced this pull request Jul 9, 2025
… srv6 test cases (#511)

<!--
Please make sure you've read and understood our contributing guidelines;
https://github.com/sonic-net/SONiC/blob/gh-pages/CONTRIBUTING.md

Please provide following information to help code review process a bit easier:
-->
### Description of PR
1.Add more timeout for ptf to handle a large scale of bgp packets after config reload/bgp restart/reboot
2.Add BGP route sync check
This PR would based on the sonic-net/sonic-mgmt#18775

Summary:
Fixes # (issue)

### Type of change

<!--
- Fill x for your type of change.
- e.g.
- [x] Bug fix
-->

- [ ] Bug fix
- [ ] Testbed and Framework(new/improvement)
- [ ] New Test case
 - [ ] Skipped for non-supported platforms
- [x] Test case improvement

### Back port request
- [ ] 202205
- [ ] 202305
- [ ] 202311
- [ ] 202405
- [ ] 202411
- [ ] 202505

### Approach
#### What is the motivation for this PR?
Case not stable
#### How did you do it?
1.Add more timeout for ptf to handle a large scale of bgp packets after config reload/bgp restart/reboot
2.Add BGP route sync check
#### How did you verify/test it?
Run it in local setup
#### Any platform specific information?

#### Supported testbed topology if it's a new test case?

### Documentation
<!--
(If it's a new feature, new test case)
Did you update documentation/Wiki relevant to your implementation?
Link to the wiki page?
-->
nissampa pushed a commit to nissampa/sonic-mgmt_dpu_test that referenced this pull request Aug 7, 2025
1.Auto generate 128 SRv6 locators and sids
2.Auto generate 128 matching packets
2.1.Test DIP shift
2.2.Test uSID container copy action
2.3.Test USD flavor
2.4.Test DSCP mapping
2.5.Test ttl and hop limit change
3.Split SRH and non SRH packets test into two test cases required by MSFT
3.1.Test reboot and BGP restart test randomly to save time
4.Test SRv6 counter
5.Test SRv6 CRM resource
6.Add static route for 128 sids validation
7.Add check for route asic db installation
8.Add timeout when captureing packet to override large scale of BGP update packets affection
9.Use a unified locator block for all the sids
10.Use command line to set SRv6 packet type
opcoder0 pushed a commit to opcoder0/sonic-mgmt that referenced this pull request Dec 8, 2025
1.Auto generate 128 SRv6 locators and sids
2.Auto generate 128 matching packets
2.1.Test DIP shift
2.2.Test uSID container copy action
2.3.Test USD flavor
2.4.Test DSCP mapping
2.5.Test ttl and hop limit change
3.Split SRH and non SRH packets test into two test cases required by MSFT
3.1.Test reboot and BGP restart test randomly to save time
4.Test SRv6 counter
5.Test SRv6 CRM resource
6.Add static route for 128 sids validation
7.Add check for route asic db installation
8.Add timeout when captureing packet to override large scale of BGP update packets affection
9.Use a unified locator block for all the sids
10.Use command line to set SRv6 packet type

Signed-off-by: opcoder0 <[email protected]>
AharonMalkin pushed a commit to AharonMalkin/sonic-mgmt that referenced this pull request Dec 16, 2025
1.Auto generate 128 SRv6 locators and sids
2.Auto generate 128 matching packets
2.1.Test DIP shift
2.2.Test uSID container copy action
2.3.Test USD flavor
2.4.Test DSCP mapping
2.5.Test ttl and hop limit change
3.Split SRH and non SRH packets test into two test cases required by MSFT
3.1.Test reboot and BGP restart test randomly to save time
4.Test SRv6 counter
5.Test SRv6 CRM resource
6.Add static route for 128 sids validation
7.Add check for route asic db installation
8.Add timeout when captureing packet to override large scale of BGP update packets affection
9.Use a unified locator block for all the sids
10.Use command line to set SRv6 packet type

Signed-off-by: Aharon Malkin <[email protected]>
gshemesh2 pushed a commit to gshemesh2/sonic-mgmt that referenced this pull request Dec 21, 2025
1.Auto generate 128 SRv6 locators and sids
2.Auto generate 128 matching packets
2.1.Test DIP shift
2.2.Test uSID container copy action
2.3.Test USD flavor
2.4.Test DSCP mapping
2.5.Test ttl and hop limit change
3.Split SRH and non SRH packets test into two test cases required by MSFT
3.1.Test reboot and BGP restart test randomly to save time
4.Test SRv6 counter
5.Test SRv6 CRM resource
6.Add static route for 128 sids validation
7.Add check for route asic db installation
8.Add timeout when captureing packet to override large scale of BGP update packets affection
9.Use a unified locator block for all the sids
10.Use command line to set SRv6 packet type

Signed-off-by: Guy Shemesh <[email protected]>
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.

7 participants