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

Skip configuring buffer and QoS config on recirc ports #7869

Merged
merged 13 commits into from
Mar 10, 2022

Conversation

ysmanman
Copy link
Contributor

Buffer or QoS config is not supported on recirc ports with today's DNX SAI. Failing to config buffer or QoS on recirc ports results in swss to exit unexpectly. So skip buffer and QoS config on recirc ports for now.

Why I did it

DNX SAI does not support to configure buffer and QoS on recirc ports today. Failing to configure buffer or QoS on recirc port results in SWSS to exit unexpectedly.

How I did it

We skipped configuring buffer and QoS on recirc ports for now. We can revert this change once SAI support is ready.

How to verify it

SWSS container exited unexpectedly when the above error encounters.
SWSS container came up fine with the fix mentioned above.

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

  • 201811
  • 201911
  • 202006
  • 202012

Description for the changelog

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

QoS config is not supported on recirc port yet with DNX SAI. Failing to
config QoS on recirc port results in swss to exit unexpectly. So skip
QoS config on recirc port for now.
@ysmanman ysmanman requested a review from lguohan as a code owner June 15, 2021 04:26
@rlhui rlhui added the Chassis 🤖 Modular chassis support label Oct 31, 2021
@rlhui
Copy link
Contributor

rlhui commented Nov 1, 2021

Is this still needed?

@ysmanman
Copy link
Contributor Author

ysmanman commented Nov 1, 2021

Is this still needed?

@rlhui We will confirm if this is still needed when testing PFC + PFCWD with SAI 5.2.

@arlakshm arlakshm requested a review from smaheshm January 13, 2022 02:14
@smaheshm
Copy link
Contributor

looks good.

Copy link
Contributor

@smaheshm smaheshm left a comment

Choose a reason for hiding this comment

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

@rlhui
Copy link
Contributor

rlhui commented Feb 3, 2022

Can you explore adding a unit test for this.

e.g. https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-config-engine/tests/test_j2files.py

@ysmanman - was unit test added? Thanks.

@ysmanman
Copy link
Contributor Author

ysmanman commented Feb 9, 2022

Can you explore adding a unit test for this.
e.g. https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-config-engine/tests/test_j2files.py

@ysmanman - was unit test added? Thanks.

Can you explore adding a unit test for this.
e.g. https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-config-engine/tests/test_j2files.py

@ysmanman - was unit test added? Thanks.

@rlhui @smaheshm Let me check if this is still issue in SAI 6.0.0.13. If it's, I will add the requested unit test.

@smaheshm
Copy link
Contributor

Can you explore adding a unit test for this.
e.g. https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-config-engine/tests/test_j2files.py

@ysmanman - was unit test added? Thanks.

Can you explore adding a unit test for this.
e.g. https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-config-engine/tests/test_j2files.py

@ysmanman - was unit test added? Thanks.

@rlhui @smaheshm Let me check if this is still issue in SAI 6.0.0.13. If it's, I will add the requested unit test.

@ysmanman we will still need this fix. 202106 release is using SAI 5.x

@ysmanman
Copy link
Contributor Author

ysmanman commented Mar 1, 2022

Can you explore adding a unit test for this.
e.g. https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-config-engine/tests/test_j2files.py

@ysmanman - was unit test added? Thanks.

Can you explore adding a unit test for this.
e.g. https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-config-engine/tests/test_j2files.py

@ysmanman - was unit test added? Thanks.

@rlhui @smaheshm Let me check if this is still issue in SAI 6.0.0.13. If it's, I will add the requested unit test.

@ysmanman we will still need this fix. 202106 release is using SAI 5.x

@smaheshm Thanks. I committed the test change just now.

@smaheshm
Copy link
Contributor

smaheshm commented Mar 1, 2022

@ysmanman Can you debug failure, looks like related to your changes.

@smaheshm
Copy link
Contributor

smaheshm commented Mar 1, 2022

Is there a difference in the sample output between py2 and py3. If so, why? Is it possible to have one sample output for both qos and buffer.

@ysmanman
Copy link
Contributor Author

ysmanman commented Mar 7, 2022

Is there a difference in the sample output between py2 and py3. If so, why? Is it possible to have one sample output for both qos and buffer.

@smaheshm yes, buffer sample outputs of py2 & py3 are different. The differences are because ports are iterated with different ordering. Regarding " Is it possible to have one sample output for both qos and buffer.", qos and buffer outputs are generated by different template files, i.e., qos.json.j2 and buffer.json.j2. So it seems reasonable to have separate sample output. Also, existing tests use separate sample outputs too, e.g., qos-dell6100.json and buffers-dell6100.json.

@smaheshm
Copy link
Contributor

@ysmanman Thanks for adding the test case and keeping up the quality.

@smaheshm smaheshm merged commit 0179844 into sonic-net:master Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants