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

[acl-loader] Fix bugs in acl-loader #991

Merged
merged 1 commit into from
Jul 16, 2020

Conversation

daall
Copy link
Contributor

@daall daall commented Jul 16, 2020

  • Fix issue where protocol == 0 would be ignored
  • Emit warning if --table_name not found in Config DB

Signed-off-by: Danny Allen daall@microsoft.com

- What I did/How I did it
I fixed a bug in acl-loader where rules with protocol=0 would have the protocol field ignored due to python's implicit boolean conversion. I fixed this by explicitly adding the == 0 check to the ip protocol field.

I also fixed/added a warning when the provided --table_name can't be found in Config DB. Previously this case would fail silently, now the user will receive a warning to point them in the right direction.

- How to verify it
Run acl-loader with a non-existent --table_name:

admin@sonic:~$ sudo acl-loader update full test.json --table_name yeehaw
Warning: Table "yeehaw" not found

Run acl-loader with a rule that includes "protocol": 0:

admin@sonic:~$ sudo acl-loader update full test.json --session_name test
admin@sonic:~$ show acl rule
Table       Rule      Priority  Action                Match
----------  ------  ----------  --------------------  --------------
EVERFLOW    RULE_1        9999  MIRROR INGRESS: test  IP_PROTOCOL: 0

- Previous command output (if the output of a command-line utility has changed)

- New command output (if the output of a command-line utility has changed)

- Support protocol number == 0
- Emit warning if --table_name not found in Config DB

Signed-off-by: Danny Allen <daall@microsoft.com>
@daall
Copy link
Contributor Author

daall commented Jul 16, 2020

retest this please

@daall
Copy link
Contributor Author

daall commented Jul 16, 2020

retest this please

@daall daall merged commit e3d6ba0 into sonic-net:master Jul 16, 2020
@daall daall deleted the acl_loader_bug_fixes branch July 16, 2020 23:34
stepanblyschak pushed a commit to stepanblyschak/sonic-utilities that referenced this pull request Apr 18, 2022
Update submodule sonic-sairedis that contains the following commit:

Change the log severity leve from ERROR to NOTICE if getStatus is not supported by vendor (sonic-net#908) (sonic-net#991)
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.

2 participants