Skip to content

[bgp_scale] Fix announce_routes for t1-isolated topo#19117

Merged
Blueve merged 4 commits intosonic-net:masterfrom
yaqiangz:azure-master_fix_t1_routes
Jun 23, 2025
Merged

[bgp_scale] Fix announce_routes for t1-isolated topo#19117
Blueve merged 4 commits intosonic-net:masterfrom
yaqiangz:azure-master_fix_t1_routes

Conversation

@yaqiangz
Copy link
Copy Markdown
Contributor

@yaqiangz yaqiangz commented Jun 20, 2025

Description of PR

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?

Fix incorrect route announcing in isolated T1 topos

How did you do it?

  1. For announce_routes.py, update to announce correct routes
  1. For test_ipv6_bgp_scale, because T0 and T2 would both announce default routes, but T0s' has the shorter as path. Hence by default there are only default routes from T0s taking effect. In test, after shutdown all T0 neighbor ports, default routes wouldn't disappear, but default routes from T2 would appear. Hence update routes verification here.

How did you verify/test it?

Run tests

Any platform specific information?

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

Documentation

@yaqiangz yaqiangz requested a review from StormLiangMS as a code owner June 20, 2025 14:57
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Copy Markdown
Contributor

@sdszhang sdszhang left a comment

Choose a reason for hiding this comment

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

topo in BGP_SCALE_T1S list looks good to me.

Blueve
Blueve previously approved these changes Jun 23, 2025
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@Blueve Blueve enabled auto-merge (squash) June 23, 2025 06:00
@Blueve Blueve merged commit 48aca94 into sonic-net:master Jun 23, 2025
18 checks passed
@mssonicbld
Copy link
Copy Markdown
Collaborator

Cherry-pick PR to msft-202412: Azure/sonic-mgmt.msft#437

@mssonicbld
Copy link
Copy Markdown
Collaborator

@yaqiangz PR conflicts with 202505 branch

Blueve pushed a commit that referenced this pull request Jul 3, 2025
What is the motivation for this PR?
Below PRs added support to advertise different route sets from BGP neighbors for T0 / T1, it's based on vm index. But loading dict from yml cannot guarantee the sort always be the same. If we re-announce routes or withdraw routes after order changing, the result would be incorrect.
#19117
#19041

How did you do it?
Add sorted in vm index to make sure order of vm dict would always be the same

How did you verify/test it?
Run tests
mssonicbld added a commit to mssonicbld/sonic-mgmt.msft that referenced this pull request Jul 3, 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
<!--
- Please include a summary of the change and which issue is fixed.
- Please also include relevant motivation and context. Where should reviewer start? background context?
- List any dependencies that are required for this change.
-->

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
- [ ] Test case improvement

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

### Approach
#### What is the motivation for this PR?
Below PRs added support to advertise different route sets from BGP neighbors for T0 / T1, it's based on vm index. But loading dict from yml cannot guarantee the sort always be the same. If we re-announce routes or withdraw routes after order changing, the result would be incorrect.
sonic-net/sonic-mgmt#19117
sonic-net/sonic-mgmt#19041

#### How did you do it?
Add sorted in vm index to make sure order of vm dict would always be the same

#### How did you verify/test it?
Run tests

#### 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 pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Jul 3, 2025
…19340)

What is the motivation for this PR?
Below PRs added support to advertise different route sets from BGP neighbors for T0 / T1, it's based on vm index. But loading dict from yml cannot guarantee the sort always be the same. If we re-announce routes or withdraw routes after order changing, the result would be incorrect.
sonic-net#19117
sonic-net#19041

How did you do it?
Add sorted in vm index to make sure order of vm dict would always be the same

How did you verify/test it?
Run tests
mssonicbld added a commit to Azure/sonic-mgmt.msft that referenced this pull request Jul 3, 2025
…0 and t1 (#490)

<!--
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
<!--
- Please include a summary of the change and which issue is fixed.
- Please also include relevant motivation and context. Where should reviewer start? background context?
- List any dependencies that are required for this change.
-->

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
- [ ] Test case improvement

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

### Approach
#### What is the motivation for this PR?
Below PRs added support to advertise different route sets from BGP neighbors for T0 / T1, it's based on vm index. But loading dict from yml cannot guarantee the sort always be the same. If we re-announce routes or withdraw routes after order changing, the result would be incorrect.
sonic-net/sonic-mgmt#19117
sonic-net/sonic-mgmt#19041

#### How did you do it?
Add sorted in vm index to make sure order of vm dict would always be the same

#### How did you verify/test it?
Run tests

#### 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 pushed a commit that referenced this pull request Jul 3, 2025
What is the motivation for this PR?
Below PRs added support to advertise different route sets from BGP neighbors for T0 / T1, it's based on vm index. But loading dict from yml cannot guarantee the sort always be the same. If we re-announce routes or withdraw routes after order changing, the result would be incorrect.
#19117
#19041

How did you do it?
Add sorted in vm index to make sure order of vm dict would always be the same

How did you verify/test it?
Run tests
nissampa pushed a commit to nissampa/sonic-mgmt_dpu_test that referenced this pull request Aug 7, 2025
What is the motivation for this PR?
Fix incorrect route announcing in isolated T1 topos

How did you do it?
For announce_routes.py, update to announce correct routes
Announce more routes, details: [doc] Update announce_routes doc for isolated T1 sonic-net#19128
Add support to announce different sets from downstream
For test_ipv6_bgp_scale, because T0 and T2 would both announce default routes, but T0s' has the shorter as path. Hence by default there are only default routes from T0s taking effect. In test, after shutdown all T0 neighbor ports, default routes wouldn't disappear, but default routes from T2 would appear. Hence update routes verification here.

How did you verify/test it?
Run tests
nissampa pushed a commit to nissampa/sonic-mgmt_dpu_test that referenced this pull request Aug 7, 2025
…19340)

What is the motivation for this PR?
Below PRs added support to advertise different route sets from BGP neighbors for T0 / T1, it's based on vm index. But loading dict from yml cannot guarantee the sort always be the same. If we re-announce routes or withdraw routes after order changing, the result would be incorrect.
sonic-net#19117
sonic-net#19041

How did you do it?
Add sorted in vm index to make sure order of vm dict would always be the same

How did you verify/test it?
Run tests
ashutosh-agrawal pushed a commit to ashutosh-agrawal/sonic-mgmt that referenced this pull request Aug 14, 2025
…19340)

What is the motivation for this PR?
Below PRs added support to advertise different route sets from BGP neighbors for T0 / T1, it's based on vm index. But loading dict from yml cannot guarantee the sort always be the same. If we re-announce routes or withdraw routes after order changing, the result would be incorrect.
sonic-net#19117
sonic-net#19041

How did you do it?
Add sorted in vm index to make sure order of vm dict would always be the same

How did you verify/test it?
Run tests
vidyac86 pushed a commit to vidyac86/sonic-mgmt that referenced this pull request Oct 23, 2025
…19340)

What is the motivation for this PR?
Below PRs added support to advertise different route sets from BGP neighbors for T0 / T1, it's based on vm index. But loading dict from yml cannot guarantee the sort always be the same. If we re-announce routes or withdraw routes after order changing, the result would be incorrect.
sonic-net#19117
sonic-net#19041

How did you do it?
Add sorted in vm index to make sure order of vm dict would always be the same

How did you verify/test it?
Run tests
opcoder0 pushed a commit to opcoder0/sonic-mgmt that referenced this pull request Dec 8, 2025
What is the motivation for this PR?
Fix incorrect route announcing in isolated T1 topos

How did you do it?
For announce_routes.py, update to announce correct routes
Announce more routes, details: [doc] Update announce_routes doc for isolated T1 sonic-net#19128
Add support to announce different sets from downstream
For test_ipv6_bgp_scale, because T0 and T2 would both announce default routes, but T0s' has the shorter as path. Hence by default there are only default routes from T0s taking effect. In test, after shutdown all T0 neighbor ports, default routes wouldn't disappear, but default routes from T2 would appear. Hence update routes verification here.

How did you verify/test it?
Run tests

Signed-off-by: opcoder0 <110003254+opcoder0@users.noreply.github.com>
opcoder0 pushed a commit to opcoder0/sonic-mgmt that referenced this pull request Dec 8, 2025
…19340)

What is the motivation for this PR?
Below PRs added support to advertise different route sets from BGP neighbors for T0 / T1, it's based on vm index. But loading dict from yml cannot guarantee the sort always be the same. If we re-announce routes or withdraw routes after order changing, the result would be incorrect.
sonic-net#19117
sonic-net#19041

How did you do it?
Add sorted in vm index to make sure order of vm dict would always be the same

How did you verify/test it?
Run tests

Signed-off-by: opcoder0 <110003254+opcoder0@users.noreply.github.com>
gshemesh2 pushed a commit to gshemesh2/sonic-mgmt that referenced this pull request Dec 16, 2025
What is the motivation for this PR?
Fix incorrect route announcing in isolated T1 topos

How did you do it?
For announce_routes.py, update to announce correct routes
Announce more routes, details: [doc] Update announce_routes doc for isolated T1 sonic-net#19128
Add support to announce different sets from downstream
For test_ipv6_bgp_scale, because T0 and T2 would both announce default routes, but T0s' has the shorter as path. Hence by default there are only default routes from T0s taking effect. In test, after shutdown all T0 neighbor ports, default routes wouldn't disappear, but default routes from T2 would appear. Hence update routes verification here.

How did you verify/test it?
Run tests

Signed-off-by: Guy Shemesh <gshemesh@nvidia.com>
gshemesh2 pushed a commit to gshemesh2/sonic-mgmt that referenced this pull request Dec 16, 2025
…19340)

What is the motivation for this PR?
Below PRs added support to advertise different route sets from BGP neighbors for T0 / T1, it's based on vm index. But loading dict from yml cannot guarantee the sort always be the same. If we re-announce routes or withdraw routes after order changing, the result would be incorrect.
sonic-net#19117
sonic-net#19041

How did you do it?
Add sorted in vm index to make sure order of vm dict would always be the same

How did you verify/test it?
Run tests

Signed-off-by: Guy Shemesh <gshemesh@nvidia.com>
AharonMalkin pushed a commit to AharonMalkin/sonic-mgmt that referenced this pull request Dec 16, 2025
What is the motivation for this PR?
Fix incorrect route announcing in isolated T1 topos

How did you do it?
For announce_routes.py, update to announce correct routes
Announce more routes, details: [doc] Update announce_routes doc for isolated T1 sonic-net#19128
Add support to announce different sets from downstream
For test_ipv6_bgp_scale, because T0 and T2 would both announce default routes, but T0s' has the shorter as path. Hence by default there are only default routes from T0s taking effect. In test, after shutdown all T0 neighbor ports, default routes wouldn't disappear, but default routes from T2 would appear. Hence update routes verification here.

How did you verify/test it?
Run tests

Signed-off-by: Aharon Malkin <amalkin@nvidia.com>
AharonMalkin pushed a commit to AharonMalkin/sonic-mgmt that referenced this pull request Dec 16, 2025
…19340)

What is the motivation for this PR?
Below PRs added support to advertise different route sets from BGP neighbors for T0 / T1, it's based on vm index. But loading dict from yml cannot guarantee the sort always be the same. If we re-announce routes or withdraw routes after order changing, the result would be incorrect.
sonic-net#19117
sonic-net#19041

How did you do it?
Add sorted in vm index to make sure order of vm dict would always be the same

How did you verify/test it?
Run tests

Signed-off-by: Aharon Malkin <amalkin@nvidia.com>
gshemesh2 pushed a commit to gshemesh2/sonic-mgmt that referenced this pull request Dec 21, 2025
What is the motivation for this PR?
Fix incorrect route announcing in isolated T1 topos

How did you do it?
For announce_routes.py, update to announce correct routes
Announce more routes, details: [doc] Update announce_routes doc for isolated T1 sonic-net#19128
Add support to announce different sets from downstream
For test_ipv6_bgp_scale, because T0 and T2 would both announce default routes, but T0s' has the shorter as path. Hence by default there are only default routes from T0s taking effect. In test, after shutdown all T0 neighbor ports, default routes wouldn't disappear, but default routes from T2 would appear. Hence update routes verification here.

How did you verify/test it?
Run tests

Signed-off-by: Guy Shemesh <gshemesh@nvidia.com>
gshemesh2 pushed a commit to gshemesh2/sonic-mgmt that referenced this pull request Dec 21, 2025
…19340)

What is the motivation for this PR?
Below PRs added support to advertise different route sets from BGP neighbors for T0 / T1, it's based on vm index. But loading dict from yml cannot guarantee the sort always be the same. If we re-announce routes or withdraw routes after order changing, the result would be incorrect.
sonic-net#19117
sonic-net#19041

How did you do it?
Add sorted in vm index to make sure order of vm dict would always be the same

How did you verify/test it?
Run tests

Signed-off-by: Guy Shemesh <gshemesh@nvidia.com>
venu-nexthop pushed a commit to venu-nexthop/sonic-mgmt that referenced this pull request Jan 13, 2026
What is the motivation for this PR?
Fix incorrect route announcing in isolated T1 topos

How did you do it?
For announce_routes.py, update to announce correct routes
Announce more routes, details: [doc] Update announce_routes doc for isolated T1 sonic-net#19128
Add support to announce different sets from downstream
For test_ipv6_bgp_scale, because T0 and T2 would both announce default routes, but T0s' has the shorter as path. Hence by default there are only default routes from T0s taking effect. In test, after shutdown all T0 neighbor ports, default routes wouldn't disappear, but default routes from T2 would appear. Hence update routes verification here.

How did you verify/test it?
Run tests
venu-nexthop pushed a commit to venu-nexthop/sonic-mgmt that referenced this pull request Jan 13, 2026
…19340)

What is the motivation for this PR?
Below PRs added support to advertise different route sets from BGP neighbors for T0 / T1, it's based on vm index. But loading dict from yml cannot guarantee the sort always be the same. If we re-announce routes or withdraw routes after order changing, the result would be incorrect.
sonic-net#19117
sonic-net#19041

How did you do it?
Add sorted in vm index to make sure order of vm dict would always be the same

How did you verify/test it?
Run tests
gshemesh2 pushed a commit to gshemesh2/sonic-mgmt that referenced this pull request Jan 26, 2026
What is the motivation for this PR?
Fix incorrect route announcing in isolated T1 topos

How did you do it?
For announce_routes.py, update to announce correct routes
Announce more routes, details: [doc] Update announce_routes doc for isolated T1 sonic-net#19128
Add support to announce different sets from downstream
For test_ipv6_bgp_scale, because T0 and T2 would both announce default routes, but T0s' has the shorter as path. Hence by default there are only default routes from T0s taking effect. In test, after shutdown all T0 neighbor ports, default routes wouldn't disappear, but default routes from T2 would appear. Hence update routes verification here.

How did you verify/test it?
Run tests

Signed-off-by: Guy Shemesh <gshemesh@nvidia.com>
gshemesh2 pushed a commit to gshemesh2/sonic-mgmt that referenced this pull request Jan 26, 2026
…19340)

What is the motivation for this PR?
Below PRs added support to advertise different route sets from BGP neighbors for T0 / T1, it's based on vm index. But loading dict from yml cannot guarantee the sort always be the same. If we re-announce routes or withdraw routes after order changing, the result would be incorrect.
sonic-net#19117
sonic-net#19041

How did you do it?
Add sorted in vm index to make sure order of vm dict would always be the same

How did you verify/test it?
Run tests

Signed-off-by: Guy Shemesh <gshemesh@nvidia.com>
ytzur1 pushed a commit to ytzur1/sonic-mgmt that referenced this pull request Feb 2, 2026
What is the motivation for this PR?
Fix incorrect route announcing in isolated T1 topos

How did you do it?
For announce_routes.py, update to announce correct routes
Announce more routes, details: [doc] Update announce_routes doc for isolated T1 sonic-net#19128
Add support to announce different sets from downstream
For test_ipv6_bgp_scale, because T0 and T2 would both announce default routes, but T0s' has the shorter as path. Hence by default there are only default routes from T0s taking effect. In test, after shutdown all T0 neighbor ports, default routes wouldn't disappear, but default routes from T2 would appear. Hence update routes verification here.

How did you verify/test it?
Run tests

Signed-off-by: Yael Tzur <ytzur@nvidia.com>
ytzur1 pushed a commit to ytzur1/sonic-mgmt that referenced this pull request Feb 2, 2026
…19340)

What is the motivation for this PR?
Below PRs added support to advertise different route sets from BGP neighbors for T0 / T1, it's based on vm index. But loading dict from yml cannot guarantee the sort always be the same. If we re-announce routes or withdraw routes after order changing, the result would be incorrect.
sonic-net#19117
sonic-net#19041

How did you do it?
Add sorted in vm index to make sure order of vm dict would always be the same

How did you verify/test it?
Run tests

Signed-off-by: Yael Tzur <ytzur@nvidia.com>
venu-nexthop pushed a commit to venu-nexthop/sonic-mgmt that referenced this pull request Mar 27, 2026
What is the motivation for this PR?
Fix incorrect route announcing in isolated T1 topos

How did you do it?
For announce_routes.py, update to announce correct routes
Announce more routes, details: [doc] Update announce_routes doc for isolated T1 sonic-net#19128
Add support to announce different sets from downstream
For test_ipv6_bgp_scale, because T0 and T2 would both announce default routes, but T0s' has the shorter as path. Hence by default there are only default routes from T0s taking effect. In test, after shutdown all T0 neighbor ports, default routes wouldn't disappear, but default routes from T2 would appear. Hence update routes verification here.

How did you verify/test it?
Run tests
venu-nexthop pushed a commit to venu-nexthop/sonic-mgmt that referenced this pull request Mar 27, 2026
…19340)

What is the motivation for this PR?
Below PRs added support to advertise different route sets from BGP neighbors for T0 / T1, it's based on vm index. But loading dict from yml cannot guarantee the sort always be the same. If we re-announce routes or withdraw routes after order changing, the result would be incorrect.
sonic-net#19117
sonic-net#19041

How did you do it?
Add sorted in vm index to make sure order of vm dict would always be the same

How did you verify/test it?
Run tests
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