Skip to content

sonic-buildimage: ignore missing hwsku dict intfs #20618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kperumalbfn merged 2 commits into from
Feb 12, 2025
Merged

sonic-buildimage: ignore missing hwsku dict intfs #20618

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a01868e
sonic-buildimage: ignore missing hwsku dict intfs
ccroy-arista Oct 25, 2024
f0bd676
Merge branch 'sonic-net:master' into continue-on-missing-hwsku-dict-i…
ccroy-arista Oct 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/sonic-config-engine/portconfig.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Copy Markdown
Contributor

@r12f r12f Oct 30, 2024

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

@ccroy-arista ccroy-arista Oct 30, 2024

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor

@r12f r12f Nov 14, 2024

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Contributor Author

@ccroy-arista ccroy-arista Nov 15, 2024

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor

@r12f r12f Nov 20, 2024

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

@ccroy-arista ccroy-arista Nov 20, 2024

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor

@prgeor prgeor Nov 20, 2024

Choose a reason for hiding this comment

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

@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"
        },


# take default_brkout_mode from hwsku.json
brkout_mode = hwsku_dict[INTF_KEY][intf][BRKOUT_MODE]
Expand Down