Skip to content
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.

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

[Mellanox] Adding SKU Mellanox-SN5600-C256A1 #19619

Merged
merged 12 commits into from
Sep 19, 2024

Conversation

DavidZagury
Copy link
Contributor

@DavidZagury DavidZagury commented Jul 18, 2024

Why I did it

Support Mellanox-SN5600-C256A1

Work item tracking
  • Microsoft ADO (number only):

How I did it

Add relevant files to support the new SKU

How to verify it

Regression test

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

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)

@DavidZagury DavidZagury marked this pull request as ready for review September 9, 2024 14:00
liat-grozovik
liat-grozovik previously approved these changes Sep 9, 2024
@liat-grozovik
Copy link
Collaborator

i believe there is a general issue with master builds, once it is fixed this need to re run.
@prgeor fyi

prgeor
prgeor previously approved these changes Sep 15, 2024
@prgeor
Copy link
Contributor

prgeor commented Sep 15, 2024

dual-TOR test failed


check_result = False
duthosts = [<MultiAsicSonicHost vlab-05>, <MultiAsicSonicHost vlab-06>]
get_mux_status = <function get_mux_status.<locals>._get_mux_status at 0x780bfc49ae50>

    def validate_check_result(check_result, duthosts, get_mux_status):
        """If check_result is False, collect some log and fail the test.
        Args:
            check_result (bool): Check result
            duthosts (list): List of duthost objects.
        """
        if not check_result:
            duthosts.shell('show muxcable config')
            duthosts.shell('show muxcable status')
            simulator_muxstatus = get_mux_status()
            if simulator_muxstatus is not None:
                logger.info('Mux status from mux simulator: {}'.format(json.dumps(simulator_muxstatus)))
            else:
                logger.error('Failed to get mux status from mux simulator')
>           pytest.fail('Toggle mux from simulator test failed')
E           Failed: Toggle mux from simulator test failed

check_result = False
duthosts   = [<MultiAsicSonicHost vlab-05>, <MultiAsicSonicHost vlab-06>]
get_mux_status = <function get_mux_status.<locals>._get_mux_status at 0x780bfc49ae50>
simulator_muxstatus = {'mbr-vms6-4-0': {'active_port': 'vlab-06-1', 'active_side': 'lower_tor', 'bridge': 'mbr-vms6-4-0', 'flap_counter': 1,...11': {'active_port': 'vlab-06-12', 'active_side': 'lower_tor', 'bridge': 'mbr-vms6-4-11', 'flap_counter': 9, ...}, ...}

common/dualtor/mux_simulator_control.py:954: Failed
=============================== warnings summary ===============================
../../../../usr/local/lib/python3.8/dist-packages/paramiko/transport.py:236
  /usr/local/lib/python3.8/dist-packages/paramiko/transport.py:236: CryptographyDeprecationWarning: Blowfish has been deprecated
    "class": algorithms.Blowfish,

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
- generated xml file: /var/src/sonic-mgmt/tests/logs/dualtor_mgmt/test_toggle_mux|||2.xml -

Ethernet501 501 etp63f 63 100000 rs off 6
Ethernet502 502 etp63g 63 100000 rs off 7
Ethernet503 503 etp63h 63 100000 rs off 8
Ethernet512 512 etp65 65 25000 rs
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this one contains 1 25G port, do you mind to rename the SKU to C256X1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, but as far as I am aware this should be C256A1 (e.g. Mellanox-SN4700-A96C8V8 )

Copy link
Contributor

Choose a reason for hiding this comment

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

hi @DavidZagury , not really, the last port is 25Gb, and it should be X not A. @yxieca can correct me, if I am wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@r12f My notes says that the speed / name map is:
10Gb - "S"
25Gb - "A"
50Gb - "D"
100Gb - "C"
200Gb - "V"
400Gb - "O"

And this matches what we see in other SKUs such as SN4700-A96C8V8.
But I don't know where to find an official answer to that.

@liat-grozovik

Copy link
Contributor

@r12f r12f left a comment

Choose a reason for hiding this comment

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

1 comment on the SKU naming to make it align with the naming convension.

@DavidZagury DavidZagury changed the title [Mellanox] Adding SKU Mellanox-SN5600-C256 [Mellanox] Adding SKU Mellanox-SN5600-C256A1 Sep 18, 2024
@yxieca yxieca merged commit 856fcc9 into sonic-net:master Sep 19, 2024
23 checks passed
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Sep 19, 2024
Why I did it
Support Mellanox-SN5600-C256A1

How I did it
Add relevant files to support the new SKU

How to verify it
Regression test
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #20307

mssonicbld pushed a commit that referenced this pull request Sep 20, 2024
Why I did it
Support Mellanox-SN5600-C256A1

How I did it
Add relevant files to support the new SKU

How to verify it
Regression test
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