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 traceback issue when running caclmgrd #9836

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ZhaohuiS
Copy link
Contributor

@ZhaohuiS ZhaohuiS commented Jan 24, 2022

#9824 was reported because some iptables rules are not cleaned during scale cacl rules test.
The change is to fix this issue.

Signed-off-by: Zhaohui Sun zhaohuisun@microsoft.com

Why I did it

To fix the traceback reported in #9824

traceback found in /var/log/syslog

WARNING swss2#orchagent: :- removeAclRule: Skip removing rule RULE_11 from ACL table NTP_ACL. Table does not exist

INFO caclmgrd[22796]: Exception in thread Thread-9:
INFO caclmgrd[22796]: Traceback (most recent call last):
INFO caclmgrd[22796]:   File "/usr/lib/python2.7/threading.py", line 801, in __bootstrap_inner
INFO caclmgrd[22796]:     self.run()
INFO caclmgrd[22796]:   File "/usr/lib/python2.7/threading.py", line 754, in run
INFO caclmgrd[22796]:     self.__target(*self.__args, **self.__kwargs)
INFO caclmgrd[22796]:   File "/usr/bin/caclmgrd", line 618, in check_and_update_control_plane_acls
INFO caclmgrd[22796]:     self.update_control_plane_acls(namespace)
INFO caclmgrd[22796]:   File "/usr/bin/caclmgrd", line 574, in update_control_plane_acls
INFO caclmgrd[22796]:     self.update_control_plane_nat_acls(namespace, service_to_source_ip_map)
INFO caclmgrd[22796]:   File "/usr/bin/caclmgrd", line 584, in update_control_plane_nat_acls
INFO caclmgrd[22796]:     iptables_cmds = self.generate_fwd_traffic_from_namespace_to_host_commands(namespace, 
INFO caclmgrd[22796]:   File "/usr/bin/caclmgrd", line 305, in generate_fwd_traffic_from_namespace_to_host_commands
INFO caclmgrd[22796]:     nat_source_ipv4_set = acl_source_ip_map[acl_service]["ipv4"] if acl_source_ip_map and lse { "0.0.0.0/0" }
INFO caclmgrd[22796]: KeyError: 'SSH'
INFO caclmgrd[22796]: Exception in thread Thread-8:
INFO caclmgrd[22796]: Traceback (most recent call last):

How I did it

  • Add checks before reading acl_source_ip_map

How to verify it

Load multiple rules(50 rules for each cacl table) with json file on asic0:

sudo ip netns exec asic0 acl-loader update full /tmp/scale_cacl.json

check iptables rules are created on asci0

sudo ip netns exec asic0 iptables -S

Copy, past and run the following commands on DUTs:

sudo ip netns exec asic0 acl-loader delete SSH_ONLY
sudo ip netns exec asic0 acl-loader delete SNMP_ACL
sudo ip netns exec asic0 acl-loader delete NTP_ACL

check some iptables rules on asci0 if all cacl rules are deleted.

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

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

…es are deleted at the same time

Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com>
@ZhaohuiS ZhaohuiS requested a review from lguohan as a code owner January 24, 2022 10:43
@ZhaohuiS ZhaohuiS requested a review from bingwang-ms January 24, 2022 10:44
Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com>
@bingwang-ms bingwang-ms requested a review from a user January 25, 2022 09:13
@bingwang-ms
Copy link
Contributor

@trzhang-msft Could you help to review this change?
I don't think increasing the DELAY will fully address the issue. Do we need some improvement in design?

@prsunny
Copy link
Contributor

prsunny commented Jan 26, 2022

@trzhang-msft Could you help to review this change? I don't think increasing the DELAY will fully address the issue. Do we need some improvement in design?

@bingwang-ms , this change was introduced long back - #5312. May be we should have this WA to unblock the issue and later plan for a design improvement.

Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com>
@ZhaohuiS ZhaohuiS changed the title Fix the issue of uncleaned cacl rules if there are multiple cacl tables are deleted at the same time Fix traceback of caclmgrd Jun 27, 2022
@ZhaohuiS ZhaohuiS changed the title Fix traceback of caclmgrd Fix traceback issue when running caclmgrd Jun 27, 2022
@ZhaohuiS
Copy link
Contributor Author

ZhaohuiS commented Jun 27, 2022

@bingwang-ms Leave interval as it is. Just fix the traceback issue in this PR. Could you please review it please?

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