Skip to content

Enhance minigraph link template for autonegotiation and remove requirement to add autoneg interfaces in topo file#16956

Merged
yxieca merged 15 commits intosonic-net:masterfrom
vdahiya12:dev/vdahiya/add-an
Feb 21, 2025
Merged

Enhance minigraph link template for autonegotiation and remove requirement to add autoneg interfaces in topo file#16956
yxieca merged 15 commits intosonic-net:masterfrom
vdahiya12:dev/vdahiya/add-an

Conversation

@vdahiya12
Copy link
Contributor

Description of PR

This PR updates the minigraph_link_meta.j2 template and topology configuration to enhance how AutoNegotiation (autoneg) interfaces are processed in SONiC's Ansible scripts. The changes streamline interface detection and remove redundant conditions.

Summary:
Fixes # (issue)

  • Removed dependency on msft_an_enabled for processing autoneg_interfaces.
  • Improved logic to check for 'autoneg' directly in device_conn rather than relying on predefined interface lists.
  • Ensured FEC settings are included only if msft_an_enabled is defined.

Type of change

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

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405
  • 202411

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

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

Documentation

and add ports for t0-56 topo to have autoneg interfaces

Signed-off-by: Vaibhav Dahiya <vdahiya@microsoft.com>
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

vm_offset: 7
DUT:
autoneg_interfaces:
intfs: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20]

Choose a reason for hiding this comment

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

This change hardcodes interfaces and remove ability set interface 1,5,7 with autoneg enabled on 1 setup with t0-56 and 2,5,7,8 etc on other setup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed, we dont need to add interfaces in this file anymore

Signed-off-by: Vaibhav Dahiya <vdahiya@microsoft.com>
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vdahiya12 vdahiya12 changed the title Enhance minigraph link template for autonegotiation and add ports for t0-56 topo to have autoneg interfaces Enhance minigraph link template for autonegotiation and remove requirement to add autoneg interfaces in topo file Feb 14, 2025
@prgeor prgeor self-requested a review February 14, 2025 19:15
prgeor
prgeor previously approved these changes Feb 14, 2025
Signed-off-by: Vaibhav Dahiya <vdahiya@microsoft.com>
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Vaibhav Dahiya <vdahiya@microsoft.com>
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Vaibhav Dahiya <vdahiya@microsoft.com>
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Vaibhav Dahiya <vdahiya@microsoft.com>
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Vaibhav Dahiya <vdahiya@microsoft.com>
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Vaibhav Dahiya <vdahiya@microsoft.com>
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Vaibhav Dahiya <vdahiya@microsoft.com>
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yxieca yxieca merged commit 2e05707 into sonic-net:master Feb 21, 2025
17 checks passed
@bingwang-ms
Copy link
Collaborator

Hi @vdahiya12 , this change caused loading minigraph failure in master branch. The error message is as below

fatal: [str3-msn4700-01]: FAILED! => {"changed": true, "cmd": "config load_minigraph --override_config -y", "delta": "0:00:27.702435", "end": "2025-02-27 00:28:29.142253", "failed_when_result": true, "msg": "non-zero return code", "rc": 1, "start": "2025-02-27 00:28:01.439818", "stderr": "Traceback (most recent call last):\n  File \"/usr/local/bin/sonic-cfggen\", line 452, in <module>\n    main()\n  File \"/usr/local/bin/sonic-cfggen\", line 342, in main\n    deep_update(data, parse_xml(minigraph, platform, asic_name=asic_name))\n  File \"/usr/local/lib/python3.9/dist-packages/minigraph.py\", line 1459, in parse_xml\n    root = ET.parse(filename).getroot()\n  File \"src/lxml/etree.pyx\", line 3538, in lxml.etree.parse\n  File \"src/lxml/parser.pxi\", line 1876, in lxml.etree._parseDocument\n  File \"src/lxml/parser.pxi\", line 1902, in lxml.etree._parseDocumentFromURL\n  File \"src/lxml/parser.pxi\", line 1805, in lxml.etree._parseDocFromFile\n  File \"src/lxml/parser.pxi\", line 1177, in lxml.etree._BaseParser._parseDocFromFile\n  File \"src/lxml/parser.pxi\", line 615, in lxml.etree._ParserContext._handleParseResultDoc\n  File \"src/lxml/parser.pxi\", line 725, in lxml.etree._handleParseResult\n  File \"src/lxml/parser.pxi\", line 654, in lxml.etree._raiseParseError\n  File \"/etc/sonic/minigraph.xml\", line 2403\nlxml.etree.XMLSyntaxError: Namespace prefix a on LinkMetadata is not defined, line 2403, column 164", "stderr_lines": ["Traceback (most recent call last):", "  File \"/usr/local/bin/sonic-cfggen\", line 452, in <module>", "    main()", "  File \"/usr/local/bin/sonic-cfggen\", line 342, in main", "    deep_update(data, parse_xml(minigraph, platform, asic_name=asic_name))", "  File \"/usr/local/lib/python3.9/dist-packages/minigraph.py\", line 1459, in parse_xml", "    root = ET.parse(filename).getroot()", "  File \"src/lxml/etree.pyx\", line 3538, in lxml.etree.parse", "  File \"src/lxml/parser.pxi\", line 1876, in lxml.etree._parseDocument", "  File \"src/lxml/parser.pxi\", line 1902, in lxml.etree._parseDocumentFromURL", "  File \"src/lxml/parser.pxi\", line 1805, in lxml.etree._parseDocFromFile", "  File \"src/lxml/parser.pxi\", line 1177, in lxml.etree._BaseParser._parseDocFromFile", "  File \"src/lxml/parser.pxi\", line 615, in lxml.etree._ParserContext._handleParseResultDoc", "  File \"src/lxml/parser.pxi\", line 725, in lxml.etree._handleParseResult", "  File \"src/lxml/parser.pxi\", line 654, in lxml.etree._raiseParseError", "  File \"/etc/sonic/minigraph.xml\", line 2403", "lxml.etree.XMLSyntaxError: Namespace prefix a on LinkMetadata is not defined, line 2403, column 164"], "stdout": "Disabling container monitoring ...\nStopping SONiC target ...\nRunning command: /usr/local/bin/sonic-cfggen -H -m -j /etc/sonic/init_cfg.json --write-to-db", "stdout_lines": ["Disabling container monitoring ...", "Stopping SONiC target ...", "Running command: /usr/local/bin/sonic-cfggen -H -m -j /etc/sonic/init_cfg.json --write-to-db"]}

I think that's because the namespace declaration is removed in this change, can you please check?

{% endif %}

{% if msft_an_enabled is defined and vm_topo_config.get('autoneg_interfaces') is not none %}
<LinkMetadataDeclaration>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This part is probably needed

@saiarcot895
Copy link
Contributor

@bingwang-ms The fix for this is in #17191.

@bingwang-ms
Copy link
Collaborator

@bingwang-ms The fix for this is in #17191.

Yeah, got it after discussing with Vaibhav. Thanks for the reminding!

mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Feb 28, 2025
…ement to add autoneg interfaces in topo file (sonic-net#16956)

This PR updates the minigraph_link_meta.j2 template and topology configuration to enhance how AutoNegotiation (autoneg) interfaces are processed in SONiC's Ansible scripts. The changes streamline interface detection and remove redundant conditions.

Removed dependency on msft_an_enabled for processing autoneg_interfaces.
Improved logic to check for 'autoneg' directly in device_conn rather than relying on predefined interface lists.
Ensured FEC settings are included only if msft_an_enabled is defined.
Type of change

Signed-off-by: Vaibhav Dahiya <vdahiya@microsoft.com>
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202411: #17261

mssonicbld pushed a commit that referenced this pull request Mar 3, 2025
…ement to add autoneg interfaces in topo file (#16956)

This PR updates the minigraph_link_meta.j2 template and topology configuration to enhance how AutoNegotiation (autoneg) interfaces are processed in SONiC's Ansible scripts. The changes streamline interface detection and remove redundant conditions.

Removed dependency on msft_an_enabled for processing autoneg_interfaces.
Improved logic to check for 'autoneg' directly in device_conn rather than relying on predefined interface lists.
Ensured FEC settings are included only if msft_an_enabled is defined.
Type of change

Signed-off-by: Vaibhav Dahiya <vdahiya@microsoft.com>
nnelluri-cisco pushed a commit to nnelluri-cisco/sonic-mgmt that referenced this pull request Mar 15, 2025
…ement to add autoneg interfaces in topo file (sonic-net#16956)

This PR updates the minigraph_link_meta.j2 template and topology configuration to enhance how AutoNegotiation (autoneg) interfaces are processed in SONiC's Ansible scripts. The changes streamline interface detection and remove redundant conditions.

Removed dependency on msft_an_enabled for processing autoneg_interfaces.
Improved logic to check for 'autoneg' directly in device_conn rather than relying on predefined interface lists.
Ensured FEC settings are included only if msft_an_enabled is defined.
Type of change

Signed-off-by: Vaibhav Dahiya <vdahiya@microsoft.com>
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to msft-202405: Azure/sonic-mgmt.msft#169

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.

9 participants