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

[dash]: Don't attempt to bind empty ACL groups #2613

Merged

Conversation

theasianpianist
Copy link
Contributor

@theasianpianist theasianpianist commented Jan 11, 2023

Signed-off-by: Lawrence Lee lawlee@microsoft.com

What I did
Before binding an ACL group to an ENI, make sure that the ACL group contains at least 1 rule. If the group is empty, retry the bind operation later.

Test improvements:

  • Use wait_for* methods in DVSDatabase to verify database contents instead of naively sleeping
  • Leverage pytest fixtures for automatic cleanup

Why I did it
Attempting to bind an empty ACL group to an ENI results in a SAI failure with no opportunity to retry.

How I verified it

  • Run test_dash_acl.py

Details if related

Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
@theasianpianist theasianpianist merged commit 827eb4b into sonic-net:dash Jan 20, 2023
theasianpianist added a commit to theasianpianist/sonic-swss that referenced this pull request Jul 21, 2023
* [dash]: Don't attempt to bind empty ACL groups

Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
theasianpianist added a commit to theasianpianist/sonic-swss that referenced this pull request Jul 25, 2023
* [dash]: Don't attempt to bind empty ACL groups

Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
theasianpianist added a commit to theasianpianist/sonic-swss that referenced this pull request Jul 27, 2023
* [dash]: Don't attempt to bind empty ACL groups

Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
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