Enable link training support in sonic-mgmt#23107
Enable link training support in sonic-mgmt#23107bingwang-ms merged 5 commits intosonic-net:masterfrom
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Hi @sdszhang, does the 202511 branch need this change as well? |
Signed-off-by: Dashuai Zhang <[email protected]>
Signed-off-by: Dashuai Zhang <[email protected]>
32082a7 to
08e0497
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Yes. 202511 will need this too. |
Signed-off-by: Dashuai Zhang <[email protected]>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR adds link-training configuration support to sonic-mgmt, primarily targeting the t0-f2 topology by enabling link training on server-facing ports via golden_config generation, and by allowing link-training settings to flow from testbed link CSVs into fanout SONiC deployment configs.
Changes:
- Add optional
LinkTrainingparsing in the Ansible graph CSV processing and propagate it into per-link facts. - Extend SONiC fanout deployment templates (202405/202505) to emit
PORT.link_trainingbased on connection metadata. - Add a new golden config generator path for
t0-f2to enablePORT.link_trainingon server-facing ports.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| ansible/roles/fanout/templates/sonic_deploy_202505.j2 | Emit link_training in generated fanout config; broaden FEC “rs” speed condition. |
| ansible/roles/fanout/templates/sonic_deploy_202405.j2 | Emit link_training in generated fanout config; add 200G to FEC “rs” speed condition. |
| ansible/module_utils/graph_utils.py | Parse optional LinkTraining column from links CSV into graph facts. |
| ansible/library/generate_golden_config_db.py | Add t0-f2 golden config generation to enable link training on server-facing ports. |
You can also share your feedback on Copilot code review. Take the survey.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Dashuai Zhang <[email protected]>
67c9b05 to
2e3547e
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Code Review✅ LGTM — Good addition of link training support for F2 topology across golden_config generation, graph_utils CSV parsing, and fanout deploy templates (both 202405 and 202505). Minor suggestion: In ori_config = json.loads(self.get_config_from_minigraph())
golden_config = ori_config # This is a reference, not a copyConsider using |
Signed-off-by: Dashuai Zhang <[email protected]>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
cherry-pick sonic-net/sonic-mgmt#23107 into 202503. <!-- 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: Enable link-training support in sonic-mgmt Currently, enabling them for all server facing ports in F2 T0 topo. Once we have port-channel support, we can enable them only for port-channel in F2 topo. ### 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 - [ ] 202511 ### Approach #### What is the motivation for this PR? Enable link-training support in sonic-mgmt #### How did you do it? Enabling link training in golden_config_db for F2 topology. #### How did you verify/test it? local testbed. #### 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? -->
StormLiangMS
left a comment
There was a problem hiding this comment.
Approve with minor comments ✅
Overall this is a solid feature addition enabling link-training for F2 T0 topology. The 4-file change covers the full stack: CSV parsing → graph facts → golden config generation → fanout deployment.
Comments
1. generate_t0_f2_golden_config_db() — reference vs. copy (nit)
golden_config = ori_config # reference, not copy
golden_config["PORT"] = ori_config.get("PORT", {}) # no-op, same dictThis works because ori_config is freshly parsed and never reused, but it reads confusingly. Consider golden_config = {"PORT": ori_config.get("PORT", {})} directly, or use copy.deepcopy for clarity.
2. Fanout template indentation (cosmetic)
In sonic_deploy_202405.j2, "link_training": "on" renders at 16 spaces indentation while other PORT keys ("fec", "alias", etc.) render at 12 spaces. Not a bug (JSON parsers ignore whitespace), but inconsistent. The pre-existing "autoneg" block at line 33 has the same issue (20 spaces).
3. FEC speed list change in sonic_deploy_202505.j2 (notable)
The FEC condition was silently broadened from == "100000" to in ["100000", "200000", "400000", "800000"]. This is a behavior change beyond link-training — now 200G/400G/800G ports on fanouts will also get "fec": "rs". Worth calling out in the PR description since it affects all fanout deployments, not just link-training ones.
4. 202405 vs 202505 template asymmetry
The 202505 template handles both link_training: "on" and "off", but the 202405 template only handles "on". If a CSV entry has LinkTraining=off, it won''t be set in the 202405 fanout config. This may be intentional (202405 default is off), but worth noting.
None of these are blockers — all are minor/cosmetic.
|
@sdszhang PR conflicts with 202511 branch |
Signed-off-by: Dashuai Zhang <[email protected]> (cherry picked from commit 7512a71)
Description of PR
Summary:
Enable link-training support in sonic-mgmt
Currently, enabling them for all server facing ports in F2 T0 topo. Once we have port-channel support, we can enable them only for port-channel in F2 topo.
Type of change
Back port request
Approach
What is the motivation for this PR?
Enable link-training support in sonic-mgmt
How did you do it?
Enabling link training in golden_config_db for F2 topology.
How did you verify/test it?
local testbed.
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation