Fix test_crm_nexthop_group: split neighbor/route into two phases#23129
Open
xixuej wants to merge 1 commit intosonic-net:masterfrom
Open
Fix test_crm_nexthop_group: split neighbor/route into two phases#23129xixuej wants to merge 1 commit intosonic-net:masterfrom
xixuej wants to merge 1 commit intosonic-net:masterfrom
Conversation
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
github-actions
bot
requested review from
arlakshm,
xwjiang-ms and
yutongzhang-microsoft
nhe-NV
previously approved these changes
Mar 21, 2026
nhe-NV
added
the
Request for 202511 branch
Contributor
Author
|
/azpw run |
Collaborator
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
The configure_nexthop_groups() function had two problems: 1. Chunk batching bug: ip_batch[1:] was intended to skip only the first IP (2.0.0.1, the base neighbor), but when batching with chunk_size=200, it skipped the first element of EVERY batch, silently losing ~9 neighbors and their routes. 2. Race condition: neighbor and route creation were interleaved in the same for-loop, so a route could reference a nexthop before its neighbor was fully programmed in HW. Fix by separating into two phases and removing the chunk batching mechanism (no longer needed with the two-phase approach): - Phase 1: add all neighbors in one shot, then poll CRM ipv4_neighbor counter to confirm they are programmed in HW - Phase 2: add all routes in one shot after neighbors are confirmed Signed-off-by: Xixue Jia <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| del_template = Template(del_template) | ||
| add_template = Template(add_template) | ||
| neigh_template = Template(neigh_template) |
Check warning
Code scanning / CodeQL
Jinja2 templating with autoescape=False Medium test
| del_template = Template(del_template) | ||
| add_template = Template(add_template) | ||
| neigh_template = Template(neigh_template) | ||
| route_template = Template(route_template) |
Check warning
Code scanning / CodeQL
Jinja2 templating with autoescape=False Medium test
| add_template = Template(add_template) | ||
| neigh_template = Template(neigh_template) | ||
| route_template = Template(route_template) | ||
| init_template = Template(init_template) |
Check warning
Code scanning / CodeQL
Jinja2 templating with autoescape=False Medium test
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of PR
Summary:
Fixes # (issue) This fixes the race conditions that were observed on Nvidia switches, this should also address #20563
The configure_nexthop_groups() function had two problems:
Chunk batching bug: ip_batch[1:] was intended to skip only the first IP (2.0.0.1, the base neighbor), but when batching with chunk_size=200, it skipped the first element of EVERY batch, silently losing ~9 neighbors and their routes.
Race condition: neighbor and route creation were interleaved in the same for-loop, so a route could reference a nexthop before its neighbor was fully programmed in HW.
Fix by separating into two phases and removing the chunk batching mechanism (no longer needed with the two-phase approach):
Type of change
Back port request
Approach
What is the motivation for this PR?
test_crm_nexthop_group[group_member=False]fails intermittently on msn4600 and msn4700 platforms with:CRM counter did not reach expected value within 60 seconds.
Expected: used >= 1891, Actual: used=1807
How did you do it?
How did you verify/test it?
Any platform specific information?
Observed on Mellanox LSN4700 and SN4600C — platforms with large NHG resource pools (~180K+) that cause the test to create ~1800+ nexthop groups, widening the race window.
Supported testbed topology if it's a new test case?
Documentation