sonic-buildimage: ignore missing hwsku dict intfs #20618

ccroy-arista · 2024-10-25T18:54:24Z

Why I did it

For new hwskus that leave some front panel ports unpopulated, sonic-config-engine will raise an exception when checking the hwsku dict for interfaces corresponding to those front panel ports. This change removes that exception, allowing duts for those hwskus to be configured.

Work item tracking

Microsoft ADO (number only):

How I did it

Replaced the raised exception with a continue statement.

How to verify it

This can be verfied by configuring an appropriate dut against the Arista-7060X6-64PE-C256S2 hwsku.

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

Tested branch (Please provide the tested image version)

202405

Description for the changelog

With the introduction of new hwskus that do not utilize every front panel port available, i.e. the ports are not fully populated, the
sonic-config-engine will raise an exception when checking the hwsku dict for such interfaces. This change removes that exception, allowing duts for those hwskus to be configured.

With the introduction of new hwskus that do not utilize every front panel port available, i.e. the ports are not fully populated, the sonic-config-engine will raise an exception when checking the hwsku dict for such interfaces. This change removes that exception.

sdszhang

LGTM

sdszhang · 2024-10-26T10:43:45Z

@r12f can you help to review this one?

r12f · 2024-10-30T07:29:50Z

src/sonic-config-engine/portconfig.py

@@ -417,7 +417,7 @@ def parse_platform_json_file(hwsku_json_file, platform_json_file):

    for intf in port_dict[INTF_KEY]:
        if intf not in hwsku_dict[INTF_KEY]:
-            raise Exception("{} is not available in hwsku_dict".format(intf))
+            continue


hi Chris, do you mind to help fix the platform.json instead of hack it here? If the interface doesn't exist, it is reasonable to throw, because things doesn't look right.

Hi @r12f, this change is related to the platform.json complications with unused interfaces, from this discussion here: #20107 (comment)

The platform.json file is common to all hwskus under the same platform folder, so updating platform.json to work with C256S2 would for example break other hwskus such as O128S2.

Atlernatively, if we add the unused interfaces back to the hwsku then we would not need to touch platform.json, but then the unused interfaces would show up in the output of show interface status.

I see. how about reverse the logic? In my understanding, platform.json is more complete and more recent design than port_config.ini, so if a port exists in the hwsku, it should exist in the platform.json as long as the platform.json file exists.

what do you think?

@r12f that sounds like a reasonable alternative, I will test the change and post the update once verified.

hi Chris, how things go with the reversed check? : D

Hi Riff, interestingly, reversing the logic causes at least one unit test to fail, meaning there is likely a fix to be had in the unit tests as well. I'll take a closer look to see what can be done there.

@r12f @ccroy-arista can we introduce dummy entry in hwsku json file :-

"interfaces": { "Ethernet0": { "default_brkout_mode": "8x100G" }, "Ethernet8": { }, <<< DUMMY "Ethernet16": { "default_brkout_mode": "8x100G" }, "Ethernet24": { }, <<< DUMMY "Ethernet32": { "default_brkout_mode": "8x100G" }, "Ethernet40": { } <<< DUMMY "Ethernet48": { "default_brkout_mode": "8x100G" },

…nterface

r12f · 2024-11-21T00:18:52Z

hi @ccroy-arista , I have sync'ed up with PrinceG, to get this working, we can try add dummy entries in the hwsku.json without changing this. In particular:

Do you mind to help give this a try?

ccroy-arista · 2024-11-21T00:21:38Z

Hi @r12f, yes I will try out your suggestions and let you know how it goes.

sdszhang · 2024-11-27T05:00:46Z

@ccroy-arista any update on this?

ccroy-arista · 2024-11-27T17:10:15Z

Hi @sdszhang @r12f unfortunately the dummy entry suggestion fails due to the missing default_brkout_mode field for those entries.

r12f · 2024-11-27T17:43:21Z

Hi @sdszhang @r12f unfortunately the dummy entry suggestion fails due to the missing default_brkout_mode field for those entries.

hi @prgeor , do you have any recommendations on this one?

sdszhang · 2024-12-09T03:18:56Z

@r12f @prgeor any other suggestions on this? If no immediate option is available, can we consider to skip this check for C256S2 and C224O8 hwsku only instead of skip it completely?

r12f · 2024-12-09T03:20:07Z

Hi @prgeor , gently ping on this one : D

r12f · 2024-12-09T03:21:07Z

Maybe we can remove the check of the default breakout?

sdszhang · 2025-01-07T12:40:03Z

@prgeor @r12f is removing default breakout feasible? or should we consider skip the port check for C256S2 and C224O8 sku for now?

sdszhang · 2025-01-08T05:50:05Z

@Gfrom2016 for viz

prgeor · 2025-01-15T03:41:06Z

@prgeor @r12f is removing default breakout feasible? or should we consider skip the port check for C256S2 and C224O8 sku for now?

@ccroy-arista can you make following change in portconfig.py to ignore ports that does not specify the default_brktout_mode

prgeor · 2025-01-15T03:43:09Z

After this I added empty breakout mode to Ethernet0, Ethernet4 and as you can see, the parser does not generate configuration for Ethernet0 and 4

sdszhang · 2025-01-18T06:07:21Z

@ccroy-arista can you make the change suggested by Prince?

ccroy-arista · 2025-01-21T01:19:40Z

@prgeor @sdszhang I'm building with the changes now and will let you know the results.

ccroy-arista · 2025-01-21T22:18:14Z

@sdszhang @prgeor the unit tests make it further now, eventually failing at test_platform_json_all_ethernet_interfaces:

======================================================================
FAIL: test_platform_json_all_ethernet_interfaces (tests.test_cfggen_platformJson.TestCfgGenPlatformJson)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/arsonic/sonic-buildimage/src/sonic-config-engine/tests/test_cfggen_platformJson.py", line 111, in test_platform_json_all_ethernet_interfaces
    self.assertDictEqual(output_dict, expected)
AssertionError: {'Ethernet0': {'alias': 'Eth1', 'lanes': '0,1,2,3',[16488 chars]ff'}} != {'Ethernet8': {'index': '3', 'lanes': '8', 'descrip[16518 chars]rs'}}

ccroy-arista · 2025-01-21T22:26:13Z

I'm looking into the failure at test_platform_json_all_ethernet_interfaces and will post another status update later today.

ccroy-arista · 2025-01-25T02:45:05Z

From the AssertionError, it looks like one of the other test files used for comparison is missing Dpc role entries for Ethernet140 and Ethernet141:

   'Ethernet140': {'alias': 'Eth36/1',
                   'description': 'Eth36/1',
                   'index': '36',
                   'lanes': '140',
                   'mtu': '9100',
                   'pfc_asym': 'off',
+                  'role': 'Dpc',
                   'speed': '25000',
                   'subport': '1',
                   'tpid': '0x8100'},
   'Ethernet141': {'alias': 'Eth36/2',
                   'description': 'Eth36/2',
                   'index': '36',
                   'lanes': '141',
                   'mtu': '9100',
                   'pfc_asym': 'off',
+                  'role': 'Dpc',
                   'speed': '25000',
                   'subport': '2',
                   'tpid': '0x8100'},

I'll add those and check the status of the unit tests once they've had a chance to run.

ccroy-arista · 2025-01-29T22:02:57Z

The unit tests have passed with the latest adjustments. Just waiting for a build to finish to test against Arista-7060X6-64PE-C256S2 to ensure no regression, then will update the PR.

ccroy-arista · 2025-01-31T00:39:00Z

Now that the sonic-config-engine unit tests pass, the build eventually fails at the sonic-device-data hwsku_json_checker validity check on the C256S2 hwsku:

Error: default_brkout_mode of Ethernet8 is/are missing
File ../../../device/arista/x86_64-arista_7060x6_64pe/Arista-7060X6-64PE-C256S2/hwsku.json failed validity check

sdszhang · 2025-01-31T03:15:20Z

@ccroy-arista @prgeor can we add some skips here to skip them for TH5 hwskus?

Now that the sonic-config-engine unit tests pass, the build eventually fails at the sonic-device-data hwsku_json_checker validity check on the C256S2 hwsku:
Error: default_brkout_mode of Ethernet8 is/are missing
File ../../../device/arista/x86_64-arista_7060x6_64pe/Arista-7060X6-64PE-C256S2/hwsku.json failed validity check

ccroy-arista · 2025-01-31T22:51:49Z

@ccroy-arista @prgeor can we add some skips here to skip them for TH5 hwskus?
Now that the sonic-config-engine unit tests pass, the build eventually fails at the sonic-device-data hwsku_json_checker validity check on the C256S2 hwsku:
Error: default_brkout_mode of Ethernet8 is/are missing
File ../../../device/arista/x86_64-arista_7060x6_64pe/Arista-7060X6-64PE-C256S2/hwsku.json failed validity check

@sdszhang I am ok with that.

sdszhang · 2025-02-01T04:25:12Z

@prgeor will it be okay?

@ccroy-arista @prgeor can we add some skips here to skip them for TH5 hwskus?
Now that the sonic-config-engine unit tests pass, the build eventually fails at the sonic-device-data hwsku_json_checker validity check on the C256S2 hwsku:
Error: default_brkout_mode of Ethernet8 is/are missing
File ../../../device/arista/x86_64-arista_7060x6_64pe/Arista-7060X6-64PE-C256S2/hwsku.json failed validity check
@sdszhang I am ok with that.

sdszhang added the Request for 202405 Branch label Oct 26, 2024

sdszhang approved these changes Oct 26, 2024

View reviewed changes

r12f reviewed Oct 30, 2024

View reviewed changes

Merge branch 'sonic-net:master' into continue-on-missing-hwsku-dict-i…

f0bd676

…nterface

StormLiangMS removed the Request for 202405 Branch label Nov 7, 2024

r12f requested a review from prgeor November 20, 2024 04:26

prgeor approved these changes Feb 7, 2025

View reviewed changes

sdszhang added Request for 202411 Branch Request for msft-202412 Branch and removed Request for 202411 Branch labels Feb 11, 2025

r12f approved these changes Feb 12, 2025

View reviewed changes

sdszhang added Request for 202411 Branch and removed Request for msft-202412 Branch labels Feb 12, 2025

r12f added the Request for msft-202412 Branch label Feb 12, 2025

kperumalbfn approved these changes Feb 12, 2025

View reviewed changes

kperumalbfn merged commit 20cfff0 into sonic-net:master Feb 12, 2025
22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sonic-buildimage: ignore missing hwsku dict intfs #20618

sonic-buildimage: ignore missing hwsku dict intfs #20618

ccroy-arista commented Oct 25, 2024

sdszhang left a comment

sdszhang commented Oct 26, 2024

r12f Oct 30, 2024

ccroy-arista Oct 30, 2024

r12f Nov 14, 2024

ccroy-arista Nov 15, 2024

r12f Nov 20, 2024

ccroy-arista Nov 20, 2024

prgeor Nov 20, 2024

r12f commented Nov 21, 2024

ccroy-arista commented Nov 21, 2024

sdszhang commented Nov 27, 2024

ccroy-arista commented Nov 27, 2024

r12f commented Nov 27, 2024 •

edited

Loading

sdszhang commented Dec 9, 2024

r12f commented Dec 9, 2024

r12f commented Dec 9, 2024

sdszhang commented Jan 7, 2025

sdszhang commented Jan 8, 2025

prgeor commented Jan 15, 2025

prgeor commented Jan 15, 2025

sdszhang commented Jan 18, 2025

ccroy-arista commented Jan 21, 2025

ccroy-arista commented Jan 21, 2025

ccroy-arista commented Jan 21, 2025

ccroy-arista commented Jan 25, 2025

ccroy-arista commented Jan 29, 2025

ccroy-arista commented Jan 31, 2025

sdszhang commented Jan 31, 2025

ccroy-arista commented Jan 31, 2025

sdszhang commented Feb 1, 2025

sonic-buildimage: ignore missing hwsku dict intfs #20618

sonic-buildimage: ignore missing hwsku dict intfs #20618

Conversation

ccroy-arista commented Oct 25, 2024

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

sdszhang left a comment

Choose a reason for hiding this comment

sdszhang commented Oct 26, 2024

r12f Oct 30, 2024

Choose a reason for hiding this comment

ccroy-arista Oct 30, 2024

Choose a reason for hiding this comment

r12f Nov 14, 2024

Choose a reason for hiding this comment

ccroy-arista Nov 15, 2024

Choose a reason for hiding this comment

r12f Nov 20, 2024

Choose a reason for hiding this comment

ccroy-arista Nov 20, 2024

Choose a reason for hiding this comment

prgeor Nov 20, 2024

Choose a reason for hiding this comment

r12f commented Nov 21, 2024

ccroy-arista commented Nov 21, 2024

sdszhang commented Nov 27, 2024

ccroy-arista commented Nov 27, 2024

r12f commented Nov 27, 2024 • edited Loading

sdszhang commented Dec 9, 2024

r12f commented Dec 9, 2024

r12f commented Dec 9, 2024

sdszhang commented Jan 7, 2025

sdszhang commented Jan 8, 2025

prgeor commented Jan 15, 2025

prgeor commented Jan 15, 2025

sdszhang commented Jan 18, 2025

ccroy-arista commented Jan 21, 2025

ccroy-arista commented Jan 21, 2025

ccroy-arista commented Jan 21, 2025

ccroy-arista commented Jan 25, 2025

ccroy-arista commented Jan 29, 2025

ccroy-arista commented Jan 31, 2025

sdszhang commented Jan 31, 2025

ccroy-arista commented Jan 31, 2025

sdszhang commented Feb 1, 2025

r12f commented Nov 27, 2024 •

edited

Loading