[DualToR][caclmgrd] Fix IPtables rules for multiple vlan interfaces f…

…or DualToR config (#82)

This PR is a required for changing the L3 IP forwarding Behavior to SoC in active-active toplogy.
Basically a src IP is added to the SNAT rule so that only packets originating from ToR with src IP as vlan IP get natted by the rule and change the src IP to LoopBack IP
However if there are mutiple vlan IP's we only add the source IP as vlan IP, for which the SoC IP belongs to, this PR adds that change.

How I did it
check the config DB if the ToR is a DualToR and has an SoC IP assigned.
put an iptable rule
iptables -t nat -A POSTROUTING --destination -j SNAT --to-source "

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>

scripts/caclmgrd

@@ -41,8 +41,10 @@ def _ip_prefix_in_key(key):
  """
  return (isinstance(key, tuple))
 
-def get_ip_from_interface_table(table, intf_name):
 
+def get_ipv4_networks_from_interface_table(table, intf_name):
+
+ addresses = []
  if table:
  for key, _ in table.items():
  if not _ip_prefix_in_key(key):
@@ -51,12 +53,11 @@ def get_ip_from_interface_table(table, intf_name):
 
  iface_name, iface_cidr = key
  if iface_name.startswith(intf_name):
- ip_str = iface_cidr.split("/")[0]
- ip_addr = ipaddress.ip_address(ip_str)
- if isinstance(ip_addr, ipaddress.IPv4Address):
- return ip_addr
+ ip_ntwrk = ipaddress.ip_network(iface_cidr, strict=False)
+ if isinstance(ip_ntwrk, ipaddress.IPv4Network):
+ addresses.append(ip_ntwrk)
 
- return None
+ return addresses
 
 # ============================== Classes ==============================
 
@@ -353,11 +354,24 @@ class ControlPlaneAclManager(daemon_base.DaemonBase):
  if self.DualToR:
  loopback_table = config_db_connector.get_table(self.LOOPBACK_TABLE)
  loopback_name = 'Loopback3'
- loopback_address = get_ip_from_interface_table(loopback_table, loopback_name)
+ loopback_networks = get_ipv4_networks_from_interface_table(loopback_table, loopback_name)
+ if len(loopback_networks) == 0:
+ self.log_warning("Loopback 3 IP not available from DualToR active-active config")
+ return fwd_dualtor_grpc_traffic_from_host_to_soc_cmds
+
+ if not isinstance(loopback_networks[0], ipaddress.IPv4Network):
+ self.log_warning("Loopback 3 IP Network not available from DualToR active-active config")
+ return fwd_dualtor_grpc_traffic_from_host_to_soc_cmds
+
+ loopback_address = loopback_networks[0].network_address
  vlan_name = 'Vlan'
  vlan_table = config_db_connector.get_table(self.VLAN_INTF_TABLE)
+ vlan_networks = get_ipv4_networks_from_interface_table(vlan_table, vlan_name)
+
+ if len(vlan_networks) == 0:
+ self.log_warning("Vlan IP not available from DualToR active-active config")
+ return fwd_dualtor_grpc_traffic_from_host_to_soc_cmds
 
- vlan_address = get_ip_from_interface_table(vlan_table, vlan_name)
  fwd_dualtor_grpc_traffic_from_host_to_soc_cmds.append(self.iptables_cmd_ns_prefix[namespace] + 
  ['iptables', '-t', 'nat', '--flush', 'POSTROUTING'])
 
@@ -367,11 +381,18 @@ class ControlPlaneAclManager(daemon_base.DaemonBase):
  for key in mux_table_keys:
  kvp = mux_table.get(key)
  if 'cable_type' in kvp and kvp['cable_type'] == 'active-active':
- fwd_dualtor_grpc_traffic_from_host_to_soc_cmds.append(self.iptables_cmd_ns_prefix[namespace] + 
- ['iptables', '-t', 'nat', '-A', 'POSTROUTING', '--destination', kvp['soc_ipv4'], '--source', vlan_address, '-j', 'SNAT', '--to-source', loopback_address])
+ soc_ipv4_str = kvp['soc_ipv4'].split("/")[0]
+ soc_ipv4_addr = ipaddress.ip_address(soc_ipv4_str)
+ for ip_network in vlan_networks:
+ # Only add the vlan source IP specific soc IP address to IPtables
+ if soc_ipv4_addr in ip_network:
+ vlan_address = ip_network.network_address
+ fwd_dualtor_grpc_traffic_from_host_to_soc_cmds.append(self.iptables_cmd_ns_prefix[namespace] + 
+ ['iptables', '-t', 'nat', '-A', 'POSTROUTING', '--destination', str(soc_ipv4_addr), '--source', str(vlan_address), '-j', 'SNAT', '--to-source', str(loopback_address)])
 
  return fwd_dualtor_grpc_traffic_from_host_to_soc_cmds
 
+
  def generate_fwd_traffic_from_namespace_to_host_commands(self, namespace, acl_source_ip_map):
  """
  The below SNAT and DNAT rules are added in asic namespace in multi-ASIC platforms. It helps to forward request coming

tests/caclmgrd/caclmgrd_soc_rules_test.py

@@ -4,11 +4,11 @@
 
 from parameterized import parameterized
 from sonic_py_common.general import load_module_from_source
-from ipaddress import IPv4Address
+from ipaddress import IPv4Address, IPv4Network
 from unittest import TestCase, mock
 from pyfakefs.fake_filesystem_unittest import patchfs
 
-from .test_soc_rules_vectors import CACLMGRD_SOC_TEST_VECTOR
+from .test_soc_rules_vectors import CACLMGRD_SOC_TEST_VECTOR, CACLMGRD_SOC_TEST_VECTOR_EMPTY
 from tests.common.mock_configdb import MockConfigDb
 from unittest.mock import MagicMock, patch
 
@@ -29,7 +29,7 @@ def setUp(self):
 
  @parameterized.expand(CACLMGRD_SOC_TEST_VECTOR)
  @patchfs
- @patch('caclmgrd.get_ip_from_interface_table', MagicMock(return_value="10.10.10.10"))
+ @patch('caclmgrd.get_ipv4_networks_from_interface_table', MagicMock(return_value=[IPv4Network('10.10.10.18/24', strict=False), IPv4Network('10.10.11.18/24', strict=False)]))
  def test_caclmgrd_soc(self, test_name, test_data, fs):
  if not os.path.exists(DBCONFIG_PATH):
  fs.create_file(DBCONFIG_PATH) # fake database_config.json
@@ -51,11 +51,62 @@ def test_caclmgrd_soc(self, test_name, test_data, fs):
  caclmgrd_daemon.update_control_plane_nat_acls('', {}, MockConfigDb())
  mocked_subprocess.Popen.assert_has_calls(test_data["expected_subprocess_calls"], any_order=True)
 
- def test_get_ip_from_interface_table(self):
+
+ @parameterized.expand(CACLMGRD_SOC_TEST_VECTOR_EMPTY)
+ @patchfs
+ @patch('caclmgrd.get_ipv4_networks_from_interface_table', MagicMock(return_value=[]))
+ def test_caclmgrd_soc_no_ips(self, test_name, test_data, fs):
+ if not os.path.exists(DBCONFIG_PATH):
+ fs.create_file(DBCONFIG_PATH) # fake database_config.json
+
+ MockConfigDb.set_config_db(test_data["config_db"])
+
+ with mock.patch("caclmgrd.ControlPlaneAclManager.run_commands_pipe", return_value='sonic'):
+ with mock.patch("caclmgrd.subprocess") as mocked_subprocess:
+ popen_mock = mock.Mock()
+ popen_attrs = test_data["popen_attributes"]
+ popen_mock.configure_mock(**popen_attrs)
+ mocked_subprocess.Popen.return_value = popen_mock
+ mocked_subprocess.PIPE = -1
+
+ call_rc = test_data["call_rc"]
+ mocked_subprocess.call.return_value = call_rc
+
+ caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd")
+ caclmgrd_daemon.update_control_plane_nat_acls('', {}, MockConfigDb())
+ mocked_subprocess.Popen.assert_has_calls(test_data["expected_subprocess_calls"], any_order=True)
+
+
+ @parameterized.expand(CACLMGRD_SOC_TEST_VECTOR_EMPTY)
+ @patchfs
+ @patch('caclmgrd.get_ipv4_networks_from_interface_table', MagicMock(return_value=['10.10.10.10']))
+ def test_caclmgrd_soc_ip_string(self, test_name, test_data, fs):
+ if not os.path.exists(DBCONFIG_PATH):
+ fs.create_file(DBCONFIG_PATH) # fake database_config.json
+
+ MockConfigDb.set_config_db(test_data["config_db"])
+
+ with mock.patch("caclmgrd.ControlPlaneAclManager.run_commands_pipe", return_value='sonic'):
+ with mock.patch("caclmgrd.subprocess") as mocked_subprocess:
+ popen_mock = mock.Mock()
+ popen_attrs = test_data["popen_attributes"]
+ popen_mock.configure_mock(**popen_attrs)
+ mocked_subprocess.Popen.return_value = popen_mock
+ mocked_subprocess.PIPE = -1
+
+ call_rc = test_data["call_rc"]
+ mocked_subprocess.call.return_value = call_rc
+
+ caclmgrd_daemon = self.caclmgrd.ControlPlaneAclManager("caclmgrd")
+ caclmgrd_daemon.update_control_plane_nat_acls('', {}, MockConfigDb())
+ mocked_subprocess.Popen.assert_has_calls(test_data["expected_subprocess_calls"], any_order=True)
+
+
+ def test_get_ipv4_networks_from_interface_table(self):
  if not os.path.exists(DBCONFIG_PATH):
  fs.create_file(DBCONFIG_PATH) # fake database_config.json
 
  table = {("Vlan1000","10.10.10.1/32"): "val"}
- ip_addr = self.caclmgrd.get_ip_from_interface_table(table, "Vlan")
+ ip_addr = self.caclmgrd.get_ipv4_networks_from_interface_table(table, "Vlan")
 
- assert (ip_addr == IPv4Address('10.10.10.1'))
+ assert (ip_addr == [IPv4Network('10.10.10.1/32')])

tests/caclmgrd/test_soc_rules_vectors.py

@@ -18,11 +18,11 @@
  "MUX_CABLE": {
  "Ethernet4": {
  "cable_type": "active-active",
- "soc_ipv4": "192.168.1.0/32",
+ "soc_ipv4": "10.10.11.7/32",
  }
  },
  "VLAN_INTERFACE": {
- "Vlan1000|10.10.2.2/23": {
+ "Vlan1000|10.10.10.3/24": {
  "NULL": "NULL",
  }
  },
@@ -35,7 +35,7 @@
  },
  },
  "expected_subprocess_calls": [
- call(['iptables', '-t', 'nat', '-A', 'POSTROUTING', '--destination', '192.168.1.0/32', '--source', '10.10.10.10', '-j', 'SNAT', '--to-source', '10.10.10.10'], universal_newlines=True, stdout=-1)
+ call(['iptables', '-t', 'nat', '-A', 'POSTROUTING', '--destination', '10.10.11.7', '--source', '10.10.11.0', '-j', 'SNAT', '--to-source', '10.10.10.0'], universal_newlines=True, stdout=-1)
  ],
  "popen_attributes": {
  'communicate.return_value': ('output', 'error'),
@@ -44,3 +44,43 @@
  }
  ]
 ]
+
+
+CACLMGRD_SOC_TEST_VECTOR_EMPTY = [
+ [
+ "SOC_SESSION_TEST",
+ {
+ "config_db": {
+ "DEVICE_METADATA": {
+ "localhost": {
+ "subtype": "DualToR",
+ "type": "ToRRouter",
+ }
+ },
+ "MUX_CABLE": {
+ "Ethernet4": {
+ "cable_type": "active-active",
+ "soc_ipv4": "10.10.11.7/32",
+ }
+ },
+ "VLAN_INTERFACE": {
+ "Vlan1000|10.10.10.3/24": {
+ "NULL": "NULL",
+ }
+ },
+ "LOOPBACK_INTERFACE": {
+ "Loopback3|10.10.10.10/32": {
+ "NULL": "NULL",
+ }
+ },
+ "FEATURE": {
+ },
+ },
+ "expected_subprocess_calls": [],
+ "popen_attributes": {
+ 'communicate.return_value': ('output', 'error'),
+ },
+ "call_rc": 0,
+ }
+ ]
+]