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

Fix ansible qos_sai failure on 201911/master image #1810

Merged
merged 1 commit into from
Jun 25, 2020

Conversation

abdosi
Copy link
Contributor

@abdosi abdosi commented Jun 24, 2020

As part of this PR#sonic-net/sonic-buildimage#4412
we have added ACCEPT rules for BGP packets as default. Because of this
my earlier change of iptable rule added in qos_sai.yml to Drop BGP packets get ignored because of lower priority
and make test case fails since BGP packets impacts Buffer calculation assumption of testcases.

Fix is to add iptable rule to Drop BGP Packet from test case as highest
priority.

During cleanup highest priority rule will get deleted first.

@neethajohn
Copy link
Contributor

neethajohn commented Jun 24, 2020

This script has been converted to pytest and that will be used for master and 201911. can you make the change there?
https://github.com/Azure/sonic-mgmt/blob/master/tests/qos/qos_sai_base.py

Can you open a PR for 201811 branch with this ansible fix? 201811 will still run using the ansible playbook

@neethajohn Created PR for 201811:
PR##1814

we have added ACCEPT rules for BGP packets as default. Because of this
iptable rule added by qos_sai.yml get ignored because of lower priority
and make test case fails since BGP packets impacts Buffer calcualtion
assumption of testcase.

Fix is to add iptable rule to Drop BGP Packet from test case as highest
priority.

Fix in py script also
@abdosi abdosi merged commit a25951a into sonic-net:master Jun 25, 2020
@abdosi abdosi deleted the qos_sai_fix branch June 25, 2020 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants