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.

Sign up for GitHub

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

[Multi-Asic] Forward SNMP requests received on front panel interface to SNMP agent in host. #5420

Merged
merged 9 commits into from
Sep 26, 2020
Merged

[Multi-Asic] Forward SNMP requests received on front panel interface to SNMP agent in host. #5420

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
acb30c0
[Multi-Asic] Forward SNMP requests destined to loopback IP, and comin…
judyjoseph Sep 21, 2020
1cea713
Updates based on comments
judyjoseph Sep 21, 2020
1ee5a2c
Further updates in docker_image_ctl.j2 and caclmgrd
judyjoseph Sep 23, 2020
b3759fd
Change the variable for net config file.
judyjoseph Sep 23, 2020
847127d
Updated the comments in the code.
judyjoseph Sep 23, 2020
02c493a
No need to clean up the exising NAT rules if present, which could be …
judyjoseph Sep 23, 2020
db5b102
Delete our rule first and add it back, to take care of caclmgrd restart.
judyjoseph Sep 24, 2020
1139058
Keeping the original logic to clean the NAT entries, to revist when N…
judyjoseph Sep 24, 2020
1d84c7d
Missing updates to log_info call.
judyjoseph Sep 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions files/image_config/caclmgrd/caclmgrd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,6 @@ class ControlPlaneAclManager(daemon_base.DaemonBase):

if namespace:
# IPv4 rules
fwd_snmp_traffic_from_namespace_to_host_cmds.append(self.iptables_cmd_ns_prefix[namespace] + "iptables -t nat -X")
fwd_snmp_traffic_from_namespace_to_host_cmds.append(self.iptables_cmd_ns_prefix[namespace] + "iptables -t nat -F")

# For namespace docker allow all tcp/udp traffic from host docker bridge to its eth0 management ip
fwd_snmp_traffic_from_namespace_to_host_cmds.append(self.iptables_cmd_ns_prefix[namespace] +
judyjoseph marked this conversation as resolved.
Show resolved Hide resolved
"iptables -t nat -A PREROUTING -p udp --dport {} -j DNAT --to-destination {}".format
(self.ACL_SERVICES['SNMP']['dst_ports'][0], self.namespace_mgmt_ip))
Expand All @@ -235,9 +231,6 @@ class ControlPlaneAclManager(daemon_base.DaemonBase):
(self.ACL_SERVICES['SNMP']['dst_ports'][0], self.namespace_docker_mgmt_ip[namespace]))

# IPv6 rules
fwd_snmp_traffic_from_namespace_to_host_cmds.append(self.iptables_cmd_ns_prefix[namespace] + "ip6tables -t nat -X")
fwd_snmp_traffic_from_namespace_to_host_cmds.append(self.iptables_cmd_ns_prefix[namespace] + "ip6tables -t nat -F")
Copy link
Contributor

@abdosi abdosi Sep 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@judyjoseph why we remove this ? If calclmgrd starts again we will keep on adding rules.
For namespace we should not remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok Thanks for pointing out .. Let me keep it back, I mistook the caclmgrd service as a one shot service, but it has restart option if it fails for some reason/exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@abdosi, instead of keeping the original code, have added new delete rules with the "iptables -D" option. This will delete only the rules which we create/own. I feel this is always better than doing "ip6tables/iptables -t nat -F/-X" which will delete all the rules present in the system ( created by other processes also) at the time when caclmgrd restarts.

Copy link
Contributor

Choose a reason for hiding this comment

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

@judyjoseph that is fine but I feel that might not be helpful since if there are other process installing nat rules we have other challenges how to maintain order of rules since other rules can be higher priority then these rules.

I feel once we nat feature in namespace or any other process then there will be more thought and refactoring will be needed.

I still feel since it is multi asic specific change we can start from clean slate on every restart


fwd_snmp_traffic_from_namespace_to_host_cmds.append(self.iptables_cmd_ns_prefix[namespace] +
"ip6tables -t nat -A PREROUTING -p udp --dport {} -j DNAT --to-destination {}".format
(self.ACL_SERVICES['SNMP']['dst_ports'][0], self.namespace_mgmt_ipv6))
Expand Down