[minigraph] remove number of lanes check for changing speed from 400G to 100G and set speed setting before lane reconfiguration #15721

vdahiya12 · 2023-07-05T20:16:43Z

8111 800G interface, split to 2x400G (each has 4 lanes) fails to change interface speed from 400G to 100G during deploy mg.
In minigraph.xml, the interface speed configuration is good, but fails to generate the right value to config_db.json.

In order to support this SKU the speed transitioning should support both 4 lanes and 8 lanes in the port_config.ini.

Why I did it

before this change for a 400G to 100G transition, in all cases except when lanes are 8, we would continue and the line
ports.setdefault(port_name, {})['speed'] = port_speed_png[port_name]
would not be executed, hence the default speed will never be set for a case and config_db will not be updated,
where speed is transitioning from 400G to 100G or 40G, but lanes are not equal to 8.

In order for those cases to pass where lanes are not specifically 8, we need the change

Work item tracking

24242657

Microsoft ADO (number only):

How I did it

How to verify it

Which release branch to backport (provide reason below if selected)

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

100G Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>

prgeor · 2023-07-05T21:37:52Z

@vdahiya12 can you specify in "Why I did it" that 400G interface could be realized with 100G (x 4) PAM4 or 50G (x8)PAM4 and hence the check for 8 lanes will not work for platforms using 100G PAM4?

prgeor · 2023-07-06T21:58:02Z

@lguohan @yxieca please help merge

yxieca · 2023-07-06T22:00:15Z

src/sonic-config-engine/minigraph.py

-            # check if the 400g port has only 8 lanes
-            if len(port_lanes) != 8:
-                continue
+
            updated_lanes = ",".join(port_lanes[:4])


Would it better to update lane map when the number of lanes is 8? Is taking first 4 lanes always right?

in both cases when a port has 4 lanes or 8 lanes, if we want to transition from 400G to 100G, we need only first 4 lanes, so we are right in that regard.

lguohan · 2023-07-07T15:27:44Z

@prgeor , i am not sure i understand the problem. in 8111, 2x400g mode, each 400G port only has 4 lanes, why this 8 lanes check poses issue?

vdahiya12 · 2023-07-07T15:55:06Z

@prgeor , i am not sure i understand the problem. in 8111, 2x400g mode, each 400G port only has 4 lanes, why this 8 lanes check poses issue?

For this Cisco-8111-O64, the minigraph parser fails as we only consider ports with 8 lanes for the 400G to 100G transition, but since we can go from 400G to 100G on a 4 lanes port as well, we need to disable this check.

lguohan · 2023-07-07T16:32:10Z

@prgeor , i am not sure i understand the problem. in 8111, 2x400g mode, each 400G port only has 4 lanes, why this 8 lanes check poses issue?

For this Cisco-8111-O64, the minigraph parser fails as we only consider ports with 8 lanes for the 400G to 100G transition, but since we can go from 400G to 100G on a 4 lanes port as well, we need to disable this check.

if the lane is already 4, then there is no need to do updated_lanes. I still do not understand the code logic here.

prgeor · 2023-07-07T18:32:32Z

@prgeor , i am not sure i understand the problem. in 8111, 2x400g mode, each 400G port only has 4 lanes, why this 8 lanes check poses issue?

For this Cisco-8111-O64, the minigraph parser fails as we only consider ports with 8 lanes for the 400G to 100G transition, but since we can go from 400G to 100G on a 4 lanes port as well, we need to disable this check.

if the lane is already 4, then there is no need to do updated_lanes. I still do not understand the code logic here.

@lguohan agree. minigraph parser should get the 4 lanes from the port_config.ini. Checking with Kevin Wang what was the issue Wang

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>

prgeor · 2023-07-26T18:29:32Z

@vdahiya12 please update the description according to the latest PR changes

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>

vdahiya12 · 2023-07-28T19:29:39Z

@lguohan @yxieca please help merge

lguohan · 2023-08-01T23:54:21Z

src/sonic-config-engine/minigraph.py

@@ -1771,17 +1771,16 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
        port_default_speed =  port_speeds_default.get(port_name, None)
        port_png_speed = port_speed_png[port_name]

+        ports.setdefault(port_name, {})['speed'] = port_speed_png[port_name]


given 1772 set the port_png_speed, it is better to use it here as well.

lguohan · 2023-08-02T00:03:03Z

src/sonic-config-engine/minigraph.py

        # when the port speed is changes from 400g to 100g/40g
        # update the port lanes, use the first 4 lanes of the 400G port to support 100G/40G port
        if port_default_speed == '400000' and (port_png_speed == '100000' or port_png_speed == '40000'):
            port_lanes =  ports[port_name].get('lanes', '').split(',')
-            # check if the 400g port has only 8 lanes
-            if len(port_lanes) != 8:
-                continue


i think this is not the problem, we should keep this.

theoritical speaking, there about 2lane, each lane is 200G, total 400G configuration. in this case, you do not want to select 4 lanes since the port only has 2 lanes.

@vdahiya12
consider following cases:-

400G using 8x50G

400G using 4x100G

400G using 2x200G (in the future)

A. We should skip in the #3 case because we don't have 4 lanes in port_config.ini to choose from for 100G

B. #2 In this case we don't need to select the lanes for 100G speed because its already 4 lanes

So, the only case which can support 100G NRZ is when 400G is 8*50G so we should skip updating the lanes to first 4 lanes if the number of lanes is not 8

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>

… to 100G and set speed setting before lane reconfiguration (sonic-net#15721) 8111 800G interface, split to 2x400G (each has 4 lanes) fails to change interface speed from 400G to 100G during deploy mg. In minigraph.xml, the interface speed configuration is good, but fails to generate the right value to config_db.json. In order to support this SKU the speed transitioning should support both 4 lanes and 8 lanes in the port_config.ini. Why I did it before this change for a 400G to 100G transition, in all cases except when lanes are 8, we would continue and the line ports.setdefault(port_name, {})['speed'] = port_speed_png[port_name] would not be executed, hence the default speed will never be set for a case and config_db will not be updated, where speed is transitioning from 400G to 100G or 40G, but lanes are not equal to 8. In order for those cases to pass where lanes are not specifically 8, we need the change Work item tracking 24242657 Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>

mssonicbld · 2023-08-06T14:39:16Z

Cherry-pick PR to 202305: #16054

… to 100G and set speed setting before lane reconfiguration (#15721) (#16054)

mssonicbld · 2023-08-29T04:14:40Z

@vdahiya12 PR conflicts with 202205 branch

mssonicbld · 2023-08-29T04:15:09Z

@vdahiya12 PR conflicts with 202211 branch

… to 100G and set speed setting before lane reconfiguration (sonic-net#15721) 8111 800G interface, split to 2x400G (each has 4 lanes) fails to change interface speed from 400G to 100G during deploy mg. In minigraph.xml, the interface speed configuration is good, but fails to generate the right value to config_db.json. In order to support this SKU the speed transitioning should support both 4 lanes and 8 lanes in the port_config.ini. Why I did it before this change for a 400G to 100G transition, in all cases except when lanes are 8, we would continue and the line ports.setdefault(port_name, {})['speed'] = port_speed_png[port_name] would not be executed, hence the default speed will never be set for a case and config_db will not be updated, where speed is transitioning from 400G to 100G or 40G, but lanes are not equal to 8. In order for those cases to pass where lanes are not specifically 8, we need the change Work item tracking 24242657 Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>

… to 100G and set speed setting before lane reconfiguration (#16452) * [minigraph] remove number of lanes check for changing speed from 400G to 100G and set speed setting before lane reconfiguration (#15721) 8111 800G interface, split to 2x400G (each has 4 lanes) fails to change interface speed from 400G to 100G during deploy mg. In minigraph.xml, the interface speed configuration is good, but fails to generate the right value to config_db.json. In order to support this SKU the speed transitioning should support both 4 lanes and 8 lanes in the port_config.ini. Why I did it before this change for a 400G to 100G transition, in all cases except when lanes are 8, we would continue and the line ports.setdefault(port_name, {})['speed'] = port_speed_png[port_name] would not be executed, hence the default speed will never be set for a case and config_db will not be updated, where speed is transitioning from 400G to 100G or 40G, but lanes are not equal to 8. In order for those cases to pass where lanes are not specifically 8, we need the change Work item tracking 24242657 Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com> * fix UT Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com> --------- Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>

… to 100G and set speed setting before lane reconfiguration (sonic-net#15721) 8111 800G interface, split to 2x400G (each has 4 lanes) fails to change interface speed from 400G to 100G during deploy mg. In minigraph.xml, the interface speed configuration is good, but fails to generate the right value to config_db.json. In order to support this SKU the speed transitioning should support both 4 lanes and 8 lanes in the port_config.ini. Why I did it before this change for a 400G to 100G transition, in all cases except when lanes are 8, we would continue and the line ports.setdefault(port_name, {})['speed'] = port_speed_png[port_name] would not be executed, hence the default speed will never be set for a case and config_db will not be updated, where speed is transitioning from 400G to 100G or 40G, but lanes are not equal to 8. In order for those cases to pass where lanes are not specifically 8, we need the change Work item tracking 24242657 Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>

[minigraph] remove number of lanes check for changing speed from 400G to

e41759f

100G Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>

vdahiya12 requested a review from prgeor July 5, 2023 20:17

abdosi previously approved these changes Jul 5, 2023

View reviewed changes

prgeor approved these changes Jul 5, 2023

View reviewed changes

yxieca reviewed Jul 6, 2023

View reviewed changes

lguohan added the Request for 202305 Branch label Jul 7, 2023

vdahiya12 marked this pull request as draft July 7, 2023 17:55

fix the files

1cbe8fc

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>

vdahiya12 dismissed abdosi’s stale review via 1cbe8fc July 21, 2023 00:32

vdahiya12 marked this pull request as ready for review July 21, 2023 00:32

add UT

c6a0456

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>

vdahiya12 changed the title ~~[minigraph] remove number of lanes check for changing speed from 400G to 100G~~ [minigraph] remove number of lanes check for changing speed from 400G to 100G and set speed setting before lane reconfiguration Aug 1, 2023

lguohan reviewed Aug 1, 2023

View reviewed changes

lguohan reviewed Aug 2, 2023

View reviewed changes

vdahiya12 added 2 commits August 2, 2023 22:12

update changes for comments

ac29a71

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>

add changes

01ba5de

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>

lguohan merged commit f41aad9 into sonic-net:master Aug 4, 2023
16 checks passed

StormLiangMS added the Approved for 202305 Branch label Aug 6, 2023

mssonicbld added the Created PR to 202305 Branch label Aug 6, 2023

mssonicbld mentioned this pull request Aug 6, 2023

[action] [PR:15721] [minigraph] remove number of lanes check for changing speed from 400G to 100G and set speed setting before lane reconfiguration #16054

Merged

11 tasks

mssonicbld added a commit that referenced this pull request Aug 6, 2023

[minigraph] remove number of lanes check for changing speed from 400G…

b812e56

… to 100G and set speed setting before lane reconfiguration (#15721) (#16054)

mssonicbld added Included in 202305 Branch and removed Created PR to 202305 Branch labels Aug 6, 2023

kevinskwang added the Request for 202205 Branch label Aug 16, 2023

yxieca added the Approved for 202205 Branch label Aug 29, 2023

mssonicbld added the Cherry Pick Conflict_202205 label Aug 29, 2023

yxieca added Request for 202111 Branch For PRs being requested for 202111 branch Approved for 202211 Branch and removed Cherry Pick Conflict_202205 Request for 202111 Branch For PRs being requested for 202111 branch labels Aug 29, 2023

mssonicbld added the Cherry Pick Conflict_202211 label Aug 29, 2023

vdahiya12 mentioned this pull request Sep 5, 2023

[minigraph] remove number of lanes check for changing speed from 400G to 100G and set speed setting before lane reconfiguration #16452

Merged

11 tasks

vdahiya12 mentioned this pull request Sep 5, 2023

[minigraph] remove number of lanes check for changing speed from 400G to 100G and set speed setting before lane reconfiguration (#15721) #16453

Open

11 tasks

yxieca added Included in 202205 Branch and removed Approved for 202205 Branch labels Sep 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[minigraph] remove number of lanes check for changing speed from 400G to 100G and set speed setting before lane reconfiguration #15721

[minigraph] remove number of lanes check for changing speed from 400G to 100G and set speed setting before lane reconfiguration #15721

vdahiya12 commented Jul 5, 2023 •

edited

Loading

prgeor commented Jul 5, 2023

prgeor commented Jul 6, 2023

yxieca Jul 6, 2023

vdahiya12 Jul 7, 2023

lguohan commented Jul 7, 2023

vdahiya12 commented Jul 7, 2023

lguohan commented Jul 7, 2023

prgeor commented Jul 7, 2023

prgeor commented Jul 26, 2023

vdahiya12 commented Jul 28, 2023

lguohan Aug 1, 2023

vdahiya12 Aug 2, 2023

lguohan Aug 2, 2023

prgeor Aug 2, 2023

vdahiya12 Aug 2, 2023

mssonicbld commented Aug 6, 2023

mssonicbld commented Aug 29, 2023

mssonicbld commented Aug 29, 2023

[minigraph] remove number of lanes check for changing speed from 400G to 100G and set speed setting before lane reconfiguration #15721

[minigraph] remove number of lanes check for changing speed from 400G to 100G and set speed setting before lane reconfiguration #15721

Conversation

vdahiya12 commented Jul 5, 2023 • edited Loading

Why I did it

Work item tracking

How I did it

How to verify it

Which release branch to backport (provide reason below if selected)

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

prgeor commented Jul 5, 2023

prgeor commented Jul 6, 2023

yxieca Jul 6, 2023

Choose a reason for hiding this comment

vdahiya12 Jul 7, 2023

Choose a reason for hiding this comment

lguohan commented Jul 7, 2023

vdahiya12 commented Jul 7, 2023

lguohan commented Jul 7, 2023

prgeor commented Jul 7, 2023

prgeor commented Jul 26, 2023

vdahiya12 commented Jul 28, 2023

lguohan Aug 1, 2023

Choose a reason for hiding this comment

vdahiya12 Aug 2, 2023

Choose a reason for hiding this comment

lguohan Aug 2, 2023

Choose a reason for hiding this comment

prgeor Aug 2, 2023

Choose a reason for hiding this comment

vdahiya12 Aug 2, 2023

Choose a reason for hiding this comment

mssonicbld commented Aug 6, 2023

mssonicbld commented Aug 29, 2023

mssonicbld commented Aug 29, 2023

vdahiya12 commented Jul 5, 2023 •

edited

Loading