Ensure Router Advertisements are disabled on renumber_topo as well as existing mgmt interface#25065
Conversation
|
|
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
github-actions
Bot
requested review from
lolyu,
wangxin and
yutongzhang-microsoft
… pre-existing Signed-off-by: Matt Hoffman <matthoffman@microsoft.com>
mdhoff-ms
force-pushed
the
dev/matthoffman/fix-ra-on-ptf-conf
branch
from
19e7253 to
582749f
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
mssonicbld
added
the
Request for 202511 branch
|
This PR has backport request for branch(es): 202511. ---Powered by SONiC BuildBot
|
anders-nexthop
left a comment
anders-nexthop
left a comment
There was a problem hiding this comment.
Confirmed what this closes: add_topo.yml already disabled RA on the PTF container (net.ipv6.conf.default.accept_ra=0, its line 194), but renumber_topo.yml didn't — so a renumbered testbed's PTF could still pick up router advertisements. This adds the missing default.accept_ra=0 to renumber and a per-interface mgmt.accept_ra=0 guard to both. Looks correct — one note inline on why the default + per-interface split is the right idiom.
— anders-bot (AI-assisted, on behalf of @anders-nexthop)
| become: yes | ||
|
|
||
| - name: Don't accept ipv6 router advertisements for docker container ptf_{{ vm_set_name }} | ||
| command: docker exec -i ptf_{{ vm_set_name }} sysctl -w net.ipv6.conf.default.accept_ra=0 |
There was a problem hiding this comment.
Good split: setting default.accept_ra=0 here — before the data/vlan interfaces are created (create_dut_port/bind/renumber below) — means those interfaces inherit it, since net.ipv6.conf.default.* only seeds interfaces brought up afterward. The mgmt interface already exists by this point, so default wouldn't cover it; that's why the explicit per-interface net.ipv6.conf.mgmt.accept_ra=0 task is needed (with the test -e guard for topos that have no mgmt interface). Matches add_topo.yml:194.
… existing mgmt interface (sonic-net#25065) … pre-existing <!-- 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 --> - [x] Bug fix - [ ] Testbed and Framework(new/improvement) - [ ] New Test case - [ ] Skipped for non-supported platforms - [ ] Test case improvement ### Back port request - [ ] 202311 - [ ] 202405 - [ ] 202411 - [ ] 202505 - [x] 202511 - [ ] 202512 - [ ] 202605 ### Approach #### What is the motivation for this PR? We noticed that some PTF containers had accept_ra=1 on nightly tests, causing a bad route to be discovered and the DUT becoming unreachable. Currently the accept_ra is only applied to default template interface during add_topo.yml. The proposed fix is to ensure accept_ra is forced to 0 in all additional scenarios: 1. after interface is configured and set up, should explicitly set mgmt.accept_ra=0 2. on renumber_topo.yml, since this may be run without add_topo. #### How did you do it? Added sysctl calls in the appropriate place in accept_topo.yml and renumber_topo.yml. For renumber_topo, we duplicate the default.accept_ra=0 that was pre-existing in add_topo and to both we also add an explicit set of mgmt.accept_ra=0 if the interface exists. #### How did you verify/test it? testbed-cli was used to remove, add and renumber the topology. The value of net.ipv6.conf.mgmt.accept_ra was checked after this and it was set correctly to 0, and the bad route did not appear. #### 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? --> Signed-off-by: Matt Hoffman <matthoffman@microsoft.com>
5 participants
… pre-existing
Description of PR
Summary:
Fixes # (issue)
Type of change
Back port request
Approach
What is the motivation for this PR?
We noticed that some PTF containers had accept_ra=1 on nightly tests, causing a bad route to be discovered and the DUT becoming unreachable. Currently the accept_ra is only applied to default template interface during add_topo.yml. The proposed fix is to ensure accept_ra is forced to 0 in all additional scenarios:
How did you do it?
Added sysctl calls in the appropriate place in accept_topo.yml and renumber_topo.yml. For renumber_topo, we duplicate the default.accept_ra=0 that was pre-existing in add_topo and to both we also add an explicit set of mgmt.accept_ra=0 if the interface exists.
How did you verify/test it?
testbed-cli was used to remove, add and renumber the topology. The value of net.ipv6.conf.mgmt.accept_ra was checked after this and it was set correctly to 0, and the bad route did not appear.
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation