Skip to content

Commit

Permalink
[DualToR][caclmgrd] Fix IPtables rules for multiple vlan interfaces f…
Browse files Browse the repository at this point in the history 
…or DualToR config (sonic-net#17093)

* [DualToR][caclmgrd] Fix IPtables rules for multiple vlan interfaces for
DualToR config

Signed-off-by: vaibhav-dahiya <[email protected]>
  • Loading branch information
@vdahiya12
vdahiya12 authored Nov 6, 2023
1 parent 213c18e commit dfe4521
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 22 deletions.
43 changes: 31 additions & 12 deletions src/sonic-host-services/scripts/caclmgrd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ 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):
Expand All @@ -51,12 +52,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 ==============================

Expand Down Expand Up @@ -342,22 +342,41 @@ 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")

if loopback_address is not None:
mux_table = config_db_connector.get_table(self.CONFIG_MUX_CABLE)
mux_table_keys = mux_table.keys()
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 {} --source {} -j SNAT --to-source {}".format(kvp['soc_ipv4'], vlan_address, 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 {} --source {} -j SNAT --to-source {}".format(str(soc_ipv4_addr), str(vlan_address), str(loopback_address)))

return fwd_dualtor_grpc_traffic_from_host_to_soc_cmds

Expand Down
61 changes: 55 additions & 6 deletions src/sonic-host-services/tests/caclmgrd/caclmgrd_soc_rules_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -50,11 +50,60 @@ 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.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.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')])
48 changes: 44 additions & 4 deletions src/sonic-host-services/tests/caclmgrd/test_soc_rules_vectors.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}
},
Expand All @@ -35,8 +35,48 @@
},
},
"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",shell=True, 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', shell=True, universal_newlines=True, stdout=-1)
],
"popen_attributes": {
'communicate.return_value': ('output', 'error'),
},
"call_rc": 0,
}
]
]


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'),
},
Expand Down

0 comments on commit dfe4521

Please sign in to comment.