Increase timeout and set port AutonNeg according to the links file #16647

Pterosaur · 2025-01-23T11:03:43Z

Description of PR

Summary:
Fixes # (issue)

Type of change

Back port request

Approach

What is the motivation for this PR?

Some testbeds have more ports than the previous testbed, so increasing the waiting time
The AutoNeg is not set in config db even if this value was specified in link files

How did you do it?

Increasing the timeout from 180 to 300
Update the autoneg in config db according to the device_conn

How did you verify/test it?

Check it in the local physical testbed

Any platform specific information?

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

Documentation

mssonicbld · 2025-01-23T11:03:46Z

/azp run

azure-pipelines · 2025-01-23T11:03:59Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-01-23T11:11:45Z

/azp run

azure-pipelines · 2025-01-23T11:11:57Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-01-23T11:25:51Z

/azp run

azure-pipelines · 2025-01-23T11:26:05Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-01-23T11:29:58Z

/azp run

azure-pipelines · 2025-01-23T11:30:10Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Ze Gan <[email protected]>

mssonicbld · 2025-01-23T20:23:21Z

/azp run

azure-pipelines · 2025-01-23T20:23:33Z

Azure Pipelines successfully started running 1 pipeline(s).

r12f · 2025-01-24T23:04:17Z

ansible/roles/fanout/templates/sonic_deploy_202405.j2

+        {% if port_name in device_conn[inventory_hostname] and 'autoneg' in device_conn[inventory_hostname][port_name] %}
+            "autoneg": "{{ device_conn[inventory_hostname][port_name]['autoneg'] }}",
+        {% else %}
+            {% if fanout_hwsku == "Mellanox-SN5600-C256S1" %}


maybe better to check against platform, since it is needed for all SP3 and SP4 platforms.

Signed-off-by: Ze Gan <[email protected]>

mssonicbld · 2025-01-27T23:24:56Z

/azp run

azure-pipelines · 2025-01-27T23:25:07Z

Azure Pipelines successfully started running 1 pipeline(s).

bingwang-ms · 2025-01-29T17:53:24Z

@vdahiya12 Could you please help review? Please confirm this change doesn't conflict with the change you are working on

bingwang-ms · 2025-01-30T23:03:32Z

ansible/roles/fanout/templates/sonic_deploy_202405.j2

+        {% if port_name in device_conn[inventory_hostname] and 'autoneg' in device_conn[inventory_hostname][port_name] %}
+            "autoneg": "{{ device_conn[inventory_hostname][port_name]['autoneg'] }}",
+        {% else %}
+            {% if "Mellanox-SN5" in fanout_hwsku or "Mellanox-SN4" in fanout_hwsku %}


We also have Mellanox-SN4600, which doesn't require AN. So better to check a list of HWSKU ?

Could you please provide a list of HWSKU to me?

You may find all the HWSKU here https://github.com/sonic-net/sonic-buildimage/tree/master/device/mellanox/x86_64-mlnx_msn4600c-r0

if we already have done the check for autoneg = 'on' in the *links.csv, do we really require a hwsku check ?

This is for the default scenario. Because the *links.csv didn't include all ports.
For example, a SKU has 64 ports, but there is only 32 ports in *links.csv.

I suggest checking the port speed. Enable AN if port speed >= 200Gb.

ansible/library/apply_device_conn_sonic.py

prgeor

@Pterosaur @bingwang-ms lets discuss this before we merge. I don't think we need to put hwsku specific check here. not right. we should follow SAI spec for AN

Signed-off-by: Ze Gan <[email protected]>

mssonicbld · 2025-02-07T00:21:01Z

/azp run

azure-pipelines · 2025-02-07T00:21:12Z

Azure Pipelines successfully started running 1 pipeline(s).

vdahiya12 · 2025-02-07T00:53:42Z

ansible/library/apply_device_conn_sonic.py

+    # Apply Autoneg
+    default_autoneg_value = None
+    # For SP4 and SP3, the default autoneg value is "on"
+    if "nvidia_sn5" in platform or "nvidia_sn4" in platform:


do u still need to have this ?

Don't really need them, remove them.

Signed-off-by: Ze Gan <[email protected]>

mssonicbld · 2025-02-07T00:59:01Z

/azp run

azure-pipelines · 2025-02-07T00:59:12Z

Azure Pipelines successfully started running 1 pipeline(s).

prgeor · 2025-02-07T01:42:08Z

ansible/library/apply_device_conn_sonic.py

@Pterosaur This file change is not needed. The link file has the AN column and the minigraph should be able to generate the configuration for the AN enabled port. @vdahiya12 can you point to that?

vdahiya12 · 2025-02-07T01:42:07Z

ansible/config_sonic_basedon_testbed.yml

@@ -771,6 +771,13 @@

        when: "'dualtor-mixed' in topo or 'dualtor-aa' in topo"

+      - name: Apply device conn to config db
+        apply_device_conn_sonic:


why do we need this for applying sonic config?
much of the config DB config will happen via #13990

Are you sure the AutoNeg value in minigraph will be applied into the config DB?

Signed-off-by: Ze Gan <[email protected]>

mssonicbld · 2025-02-10T00:55:34Z

/azp run

azure-pipelines · 2025-02-10T00:55:45Z

Azure Pipelines successfully started running 1 pipeline(s).

prgeor · 2025-02-10T15:52:27Z

ansible/library/update_device_conn_in_vm_topo.py

@Pterosaur are we going to develop this code for each and every new sku in the future. AN should be SKU agnostic

vdahiya12 · 2025-02-24T23:39:41Z

ansible/library/update_device_conn_in_vm_topo.py

@@ -0,0 +1,66 @@
+#!/usr/bin/env python
+
+from ansible.module_utils.basic import AnsibleModule


this file change should not be needed any more

Pterosaur marked this pull request as ready for review January 23, 2025 11:08

Pterosaur force-pushed the device_conn_autoneg branch from a5b8233 to f886eae Compare January 23, 2025 11:11

Pterosaur force-pushed the device_conn_autoneg branch from f886eae to 8f02cf7 Compare January 23, 2025 11:25

Pterosaur force-pushed the device_conn_autoneg branch from 8f02cf7 to 5aa34b0 Compare January 23, 2025 11:29

Pterosaur requested a review from r12f January 23, 2025 11:46

Increase timeout and set autoneg according to links file

82224ef

Signed-off-by: Ze Gan <[email protected]>

Pterosaur force-pushed the device_conn_autoneg branch from 5aa34b0 to 82224ef Compare January 23, 2025 20:23

r12f reviewed Jan 24, 2025

View reviewed changes

Set autoneg on for both SP3 and SP4

9617015

Signed-off-by: Ze Gan <[email protected]>

r12f approved these changes Jan 28, 2025

View reviewed changes

r12f added Request for msft-202412 Branch Approved for msft-202412 Branch labels Jan 28, 2025

Pterosaur added Request for 202405 branch Request for 202411 branch labels Jan 28, 2025

r12f removed Request for msft-202412 Branch Approved for msft-202412 Branch labels Jan 29, 2025

bingwang-ms reviewed Jan 30, 2025

View reviewed changes

ansible/library/apply_device_conn_sonic.py Outdated Show resolved Hide resolved

prgeor requested changes Feb 3, 2025

View reviewed changes

Remove SP3 SP4 specific code in fanout template

e58b945

Signed-off-by: Ze Gan <[email protected]>

vdahiya12 reviewed Feb 7, 2025

View reviewed changes

Remove SP3 SP4 specific code in dut

d697a79

Signed-off-by: Ze Gan <[email protected]>

prgeor reviewed Feb 7, 2025

View reviewed changes

vdahiya12 reviewed Feb 7, 2025

View reviewed changes

Refactor autoneg via minigraph of DPU

1ed25a8

Signed-off-by: Ze Gan <[email protected]>

Pterosaur requested review from vdahiya12 and prgeor February 10, 2025 10:51

prgeor reviewed Feb 10, 2025

View reviewed changes

Pterosaur marked this pull request as draft February 23, 2025 20:12

vdahiya12 suggested changes Feb 24, 2025

View reviewed changes

		@@ -0,0 +1,66 @@
		#!/usr/bin/env python

		from ansible.module_utils.basic import AnsibleModule

Increase timeout and set port AutonNeg according to the links file #16647

Are you sure you want to change the base?

Increase timeout and set port AutonNeg according to the links file #16647

Uh oh!

Conversation

Pterosaur commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

Type of change

Back port request

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

Uh oh!

mssonicbld commented Jan 23, 2025

Uh oh!

azure-pipelines bot commented Jan 23, 2025

Uh oh!

mssonicbld commented Jan 23, 2025

Uh oh!

azure-pipelines bot commented Jan 23, 2025

Uh oh!

mssonicbld commented Jan 23, 2025

Uh oh!

azure-pipelines bot commented Jan 23, 2025

Uh oh!

mssonicbld commented Jan 23, 2025

Uh oh!

azure-pipelines bot commented Jan 23, 2025

Uh oh!

mssonicbld commented Jan 23, 2025

Uh oh!

azure-pipelines bot commented Jan 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Jan 27, 2025

Uh oh!

azure-pipelines bot commented Jan 27, 2025

Uh oh!

bingwang-ms commented Jan 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

prgeor left a comment

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Feb 7, 2025

Uh oh!

azure-pipelines bot commented Feb 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Feb 7, 2025

Uh oh!

azure-pipelines bot commented Feb 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pterosaur commented Jan 23, 2025 •

edited

Loading