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

added files to create SKU Mellanox-SN3800-C64 #4812

Merged
merged 3 commits into from
Jun 22, 2020

Conversation

madhanmellanox
Copy link
Contributor

  • What I did
    Created all the relevant files to modify SKU Mellanox-SN3800-C64

  • How I did it
    Created all the SKU related files using a XML file provided and a sonic_sku_create.py script to generate the files.

  • How to verify it
    Load the new HWKU related config using follow command and reload the config.
    sudo sonic-cfggen --preset l2 -p -H -k Mellanox-SN3800-C64 > /tmp/newconfig
    sudo cp /tmp/newconfig /etc/sonic/config_db.json
    sudo config reload
    show interfaces status

  • Description for the changelog
    Created SKU related files buffers_defaults_t0.j2, buffers_defaults_t1.j2, buffers.json.j2, pg_profile_lookup.ini, port_config.ini, qos.json.j2, sai_3800.xml and sai.profile.

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

@prsunny
Copy link
Contributor

prsunny commented Jun 19, 2020

I thought this is same as ACS-MSN3800 and this should only be a "symlink". I believe you duplicated all files from ACS-MSN3800 or is it different?

Copy link
Contributor

@prsunny prsunny left a comment

Choose a reason for hiding this comment

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

Please check comment

@madhanmellanox
Copy link
Contributor Author

I thought this is same as ACS-MSN3800 and this should only be a "symlink". I believe you duplicated all files from ACS-MSN3800 or is it different?

It is same as files of ACS-MSN3800. But, when we create symlink and then try to set the HWSKU on the switch, it does not work properly. That is why I created physical files instead of symlinks.

@prsunny
Copy link
Contributor

prsunny commented Jun 19, 2020

I thought this is same as ACS-MSN3800 and this should only be a "symlink". I believe you duplicated all files from ACS-MSN3800 or is it different?

It is same as files of ACS-MSN3800. But, when we create symlink and then try to set the HWSKU on the switch, it does not work properly. That is why I created physical files instead of symlinks.

Please check this - Mellanox-SN2700 is a symlink

@prsunny
Copy link
Contributor

prsunny commented Jun 19, 2020

Why is it not a symlink?

@madhanmellanox
Copy link
Contributor Author

madhanmellanox commented Jun 19, 2020

Why is it not a symlink?

Nowhere in the SKUs, port_config.ini, sai.profile, sai_xxx.xml is a symlink. Otherwise, we will have problems loading the HWSKU. Rest of all other files, I have made it as symlink.

@prsunny
Copy link
Contributor

prsunny commented Jun 19, 2020

Why is it not a symlink?

Nowhere in the SKUs, port_config.ini, sai.profile, sai_xxx.xml is a symlink. Otherwise, we will have problems loading the HWSKU. Rest of all other files, I have made it as symlink.

Did you check the link I shared for 2700? How is that working?

@madhanmellanox
Copy link
Contributor Author

Not sure, if the whole directory is itself a symlink. I will modify my fix to be the same.

@prsunny
Copy link
Contributor

prsunny commented Jun 19, 2020

@sschlafman, can you sign off?

@madhanmellanox
Copy link
Contributor Author

retest vsimage please

@prsunny
Copy link
Contributor

prsunny commented Jun 21, 2020

Looks different from this PR - can you confirm it is working with the symlink?

@prsunny prsunny merged commit d2366d4 into sonic-net:master Jun 22, 2020
abdosi pushed a commit that referenced this pull request Jun 23, 2020
* added files to create SKU Mellanox-SN3800-C64
Co-authored-by: Madhan Babu <madhan@arc-build-server.mtr.labs.mlnx>
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.

3 participants