diff --git a/elements/amphora-agent/static/usr/local/bin/lvs-masquerade.sh b/elements/amphora-agent/static/usr/local/bin/lvs-masquerade.sh index 8e26a6a56b..b5621b0b12 100755 --- a/elements/amphora-agent/static/usr/local/bin/lvs-masquerade.sh +++ b/elements/amphora-agent/static/usr/local/bin/lvs-masquerade.sh @@ -19,7 +19,7 @@ set -e usage() { echo - echo "Usage: $(basename "$0") [add|delete] [ipv4|ipv6] " + echo "Usage: $(basename "$0") [add|delete] [ipv4|ipv6] [sriov]" echo exit 1 } @@ -38,10 +38,12 @@ if [ "$1" == "add" ]; then nft add rule ip octavia-ipv4 ip-udp-masq oifname "$3" meta l4proto udp masquerade nft add chain ip octavia-ipv4 ip-sctp-masq { type nat hook postrouting priority 100\;} nft add rule ip octavia-ipv4 ip-sctp-masq oifname "$3" meta l4proto sctp masquerade - nft -- add chain ip octavia-ipv4 prerouting { type filter hook prerouting priority -300 \; } - nft add rule ip octavia-ipv4 prerouting iifname "$3" meta l4proto tcp notrack - nft -- add chain ip octavia-ipv4 output { type filter hook output priority -300 \; } - nft add rule ip octavia-ipv4 output oifname "$3" meta l4proto tcp notrack + if ! [ "$4" == "sriov" ]; then + nft -- add chain ip octavia-ipv4 prerouting { type filter hook prerouting priority -300 \; } + nft add rule ip octavia-ipv4 prerouting iifname "$3" meta l4proto tcp notrack + nft -- add chain ip octavia-ipv4 output { type filter hook output priority -300 \; } + nft add rule ip octavia-ipv4 output oifname "$3" meta l4proto tcp notrack + fi elif [ "$2" == "ipv6" ]; then nft add table ip6 octavia-ipv6 @@ -49,10 +51,12 @@ if [ "$1" == "add" ]; then nft add rule ip6 octavia-ipv6 ip6-udp-masq oifname "$3" meta l4proto udp masquerade nft add chain ip6 octavia-ipv6 ip6-sctp-masq { type nat hook postrouting priority 100\;} nft add rule ip6 octavia-ipv6 ip6-sctp-masq oifname "$3" meta l4proto sctp masquerade - nft -- add chain ip6 octavia-ipv6 prerouting { type filter hook prerouting priority -300 \; } - nft add rule ip6 octavia-ipv6 prerouting iifname "$3" meta l4proto tcp notrack - nft -- add chain ip6 octavia-ipv6 output { type filter hook output priority -300 \; } - nft add rule ip6 octavia-ipv6 output oifname "$3" meta l4proto tcp notrack + if ! [ "$4" == "sriov" ]; then + nft -- add chain ip6 octavia-ipv6 prerouting { type filter hook prerouting priority -300 \; } + nft add rule ip6 octavia-ipv6 prerouting iifname "$3" meta l4proto tcp notrack + nft -- add chain ip6 octavia-ipv6 output { type filter hook output priority -300 \; } + nft add rule ip6 octavia-ipv6 output oifname "$3" meta l4proto tcp notrack + fi else usage fi @@ -62,14 +66,18 @@ if [ "$1" == "add" ]; then /sbin/iptables -t nat -A POSTROUTING -p udp -o $3 -j MASQUERADE /sbin/iptables -t nat -A POSTROUTING -p sctp -o $3 -j MASQUERADE - /sbin/iptables -t raw -A PREROUTING -p tcp -i $3 -j NOTRACK - /sbin/iptables -t raw -A OUTPUT -p tcp -o $3 -j NOTRACK + if ! [ "$4" == "sriov" ]; then + /sbin/iptables -t raw -A PREROUTING -p tcp -i $3 -j NOTRACK + /sbin/iptables -t raw -A OUTPUT -p tcp -o $3 -j NOTRACK + fi elif [ "$2" == "ipv6" ]; then /sbin/ip6tables -t nat -A POSTROUTING -p udp -o $3 -j MASQUERADE /sbin/ip6tables -t nat -A POSTROUTING -p sctp -o $3 -j MASQUERADE - /sbin/ip6tables -t raw -A PREROUTING -p tcp -i $3 -j NOTRACK - /sbin/ip6tables -t raw -A OUTPUT -p tcp -o $3 -j NOTRACK + if ! [ "$4" == "sriov" ]; then + /sbin/ip6tables -t raw -A PREROUTING -p tcp -i $3 -j NOTRACK + /sbin/ip6tables -t raw -A OUTPUT -p tcp -o $3 -j NOTRACK + fi else usage fi @@ -83,19 +91,21 @@ elif [ "$1" == "delete" ]; then nft delete chain ip octavia-ipv4 ip-udp-masq nft flush chain ip octavia-ipv4 ip-sctp-masq nft delete chain ip octavia-ipv4 ip-sctp-masq - nft flush chain ip octavia-ipv4 prerouting - nft delete chain ip octavia-ipv4 prerouting - nft flush chain ip octavia-ipv4 output - nft delete chain ip octavia-ipv4 output + # Don't abort the script if these chains don't exist + nft flush chain ip octavia-ipv4 prerouting || true + nft delete chain ip octavia-ipv4 prerouting || true + nft flush chain ip octavia-ipv4 output || true + nft delete chain ip octavia-ipv4 output || true elif [ "$2" == "ipv6" ]; then nft flush chain ip6 octavia-ipv6 ip6-udp-masq nft delete chain ip6 octavia-ipv6 ip6-udp-masq nft flush chain ip6 octavia-ipv6 ip6-sctp-masq nft delete chain ip6 octavia-ipv6 ip6-sctp-masq - nft flush chain ip6 octavia-ipv6 prerouting - nft delete chain ip6 octavia-ipv6 prerouting - nft flush chain ip6 octavia-ipv6 output - nft delete chain ip6 octavia-ipv6 output + # Don't abort the script if these chains don't exist + nft flush chain ip6 octavia-ipv6 prerouting || true + nft delete chain ip6 octavia-ipv6 prerouting || true + nft flush chain ip6 octavia-ipv6 output || true + nft delete chain ip6 octavia-ipv6 output || true else usage fi @@ -104,13 +114,15 @@ elif [ "$1" == "delete" ]; then if [ "$2" == "ipv4" ]; then /sbin/iptables -t nat -D POSTROUTING -p udp -o $3 -j MASQUERADE /sbin/iptables -t nat -D POSTROUTING -p sctp -o $3 -j MASQUERADE - /sbin/iptables -t raw -D PREROUTING -p tcp -i $3 -j NOTRACK - /sbin/iptables -t raw -D OUTPUT -p tcp -o $3 -j NOTRACK + # Don't abort the script if these chains don't exist + /sbin/iptables -t raw -D PREROUTING -p tcp -i $3 -j NOTRACK || true + /sbin/iptables -t raw -D OUTPUT -p tcp -o $3 -j NOTRACK || true elif [ "$2" == "ipv6" ]; then /sbin/ip6tables -t nat -D POSTROUTING -p udp -o $3 -j MASQUERADE /sbin/ip6tables -t nat -D POSTROUTING -p sctp -o $3 -j MASQUERADE - /sbin/ip6tables -t raw -D PREROUTING -p tcp -i $3 -j NOTRACK - /sbin/ip6tables -t raw -D OUTPUT -p tcp -o $3 -j NOTRACK + # Don't abort the script if these chains don't exist + /sbin/ip6tables -t raw -D PREROUTING -p tcp -i $3 -j NOTRACK || true + /sbin/ip6tables -t raw -D OUTPUT -p tcp -o $3 -j NOTRACK || true else usage fi diff --git a/octavia/amphorae/backends/agent/api_server/osutils.py b/octavia/amphorae/backends/agent/api_server/osutils.py index 169aa38cae..ad4577046a 100644 --- a/octavia/amphorae/backends/agent/api_server/osutils.py +++ b/octavia/amphorae/backends/agent/api_server/osutils.py @@ -75,11 +75,13 @@ class BaseOS: is_sriov=is_sriov) vip_interface.write() - def write_port_interface_file(self, interface, fixed_ips, mtu): + def write_port_interface_file(self, interface, fixed_ips, mtu, + is_sriov=False): port_interface = interface_file.PortInterfaceFile( name=interface, mtu=mtu, - fixed_ips=fixed_ips) + fixed_ips=fixed_ips, + is_sriov=is_sriov) port_interface.write() @classmethod diff --git a/octavia/amphorae/backends/agent/api_server/plug.py b/octavia/amphorae/backends/agent/api_server/plug.py index 3d81836a5e..439a79cfd1 100644 --- a/octavia/amphorae/backends/agent/api_server/plug.py +++ b/octavia/amphorae/backends/agent/api_server/plug.py @@ -152,7 +152,7 @@ class Plug: socket.inet_pton(socket.AF_INET6, ip.get('ip_address')) def plug_network(self, mac_address, fixed_ips, mtu=None, - vip_net_info=None): + vip_net_info=None, is_sriov=False): try: self._check_ip_addresses(fixed_ips=fixed_ips) except OSError: @@ -189,7 +189,8 @@ class Plug: vips=rendered_vips, mtu=mtu, vrrp_info=vrrp_info, - fixed_ips=fixed_ips) + fixed_ips=fixed_ips, + is_sriov=is_sriov) self._osutils.bring_interface_up(existing_interface, 'vip') # Otherwise, we are just plugging a run-of-the-mill network else: @@ -197,7 +198,7 @@ class Plug: self._osutils.write_port_interface_file( interface=existing_interface, fixed_ips=fixed_ips, - mtu=mtu) + mtu=mtu, is_sriov=is_sriov) self._osutils.bring_interface_up(existing_interface, 'network') util.send_member_advertisements(fixed_ips) @@ -222,7 +223,7 @@ class Plug: self._osutils.write_port_interface_file( interface=netns_interface, fixed_ips=fixed_ips, - mtu=mtu) + mtu=mtu, is_sriov=is_sriov) # Update the list of interfaces to add to the namespace self._update_plugged_interfaces_file(netns_interface, mac_address) diff --git a/octavia/amphorae/backends/agent/api_server/server.py b/octavia/amphorae/backends/agent/api_server/server.py index 55d17428a7..7daa82d1cc 100644 --- a/octavia/amphorae/backends/agent/api_server/server.py +++ b/octavia/amphorae/backends/agent/api_server/server.py @@ -223,7 +223,8 @@ class Server: return self._plug.plug_network(port_info['mac_address'], port_info.get('fixed_ips'), port_info.get('mtu'), - port_info.get('vip_net_info')) + port_info.get('vip_net_info'), + port_info.get('is_sriov')) def upload_cert(self): return certificate_update.upload_server_cert() @@ -278,7 +279,7 @@ class Server: raise exceptions.BadRequest( description='Invalid rules information') from e - nftable_utils.write_nftable_vip_rules_file(interface, rules_info) + nftable_utils.write_nftable_rules_file(interface, rules_info) nftable_utils.load_nftables_file() diff --git a/octavia/amphorae/backends/utils/interface.py b/octavia/amphorae/backends/utils/interface.py index 420f7fd3db..fc029f8aa7 100644 --- a/octavia/amphorae/backends/utils/interface.py +++ b/octavia/amphorae/backends/utils/interface.py @@ -172,47 +172,12 @@ class InterfaceController: ip_network = ipaddress.ip_network(address, strict=False) return ip_network.compressed - def _setup_nftables_chain(self, interface): - # TODO(johnsom) Move this to pyroute2 when the nftables library - # improves. - - # Create the nftable - cmd = [consts.NFT_CMD, consts.NFT_ADD, 'table', consts.NFT_FAMILY, - consts.NFT_VIP_TABLE] - try: - subprocess.check_output(cmd, stderr=subprocess.STDOUT) - except Exception as e: - if hasattr(e, 'output'): - LOG.error(e.output) - else: - LOG.error(e) - raise - - # Create the chain with -310 priority to put it in front of the - # lvs-masquerade configured chain - cmd = [consts.NFT_CMD, consts.NFT_ADD, 'chain', consts.NFT_FAMILY, - consts.NFT_VIP_TABLE, consts.NFT_VIP_CHAIN, - '{', 'type', 'filter', 'hook', 'ingress', 'device', - interface.name, 'priority', consts.NFT_SRIOV_PRIORITY, ';', - 'policy', 'drop', ';', '}'] - try: - subprocess.check_output(cmd, stderr=subprocess.STDOUT) - except Exception as e: - if hasattr(e, 'output'): - LOG.error(e.output) - else: - LOG.error(e) - raise - - nftable_utils.write_nftable_vip_rules_file(interface.name, []) - - nftable_utils.load_nftables_file() - def up(self, interface): LOG.info("Setting interface %s up", interface.name) if interface.is_sriov: - self._setup_nftables_chain(interface) + nftable_utils.write_nftable_rules_file(interface.name, []) + nftable_utils.load_nftables_file() with pyroute2.IPRoute() as ipr: idx = ipr.link_lookup(ifname=interface.name)[0] diff --git a/octavia/amphorae/backends/utils/interface_file.py b/octavia/amphorae/backends/utils/interface_file.py index e0658b8db2..674281e9ff 100644 --- a/octavia/amphorae/backends/utils/interface_file.py +++ b/octavia/amphorae/backends/utils/interface_file.py @@ -227,22 +227,28 @@ class VIPInterfaceFile(InterfaceFile): fixed_ip.get('host_routes', [])) self.routes.extend(host_routes) + if is_sriov: + sriov_param = ' sriov' + else: + sriov_param = '' + for ip_v in ip_versions: self.scripts[consts.IFACE_UP].append({ consts.COMMAND: ( - "/usr/local/bin/lvs-masquerade.sh add {} {}".format( - 'ipv6' if ip_v == 6 else 'ipv4', name)) + "/usr/local/bin/lvs-masquerade.sh add {} {}{}".format( + 'ipv6' if ip_v == 6 else 'ipv4', name, sriov_param)) }) self.scripts[consts.IFACE_DOWN].append({ consts.COMMAND: ( - "/usr/local/bin/lvs-masquerade.sh delete {} {}".format( - 'ipv6' if ip_v == 6 else 'ipv4', name)) + "/usr/local/bin/lvs-masquerade.sh delete {} {}{}".format( + 'ipv6' if ip_v == 6 else 'ipv4', name, sriov_param)) }) class PortInterfaceFile(InterfaceFile): - def __init__(self, name, mtu, fixed_ips): - super().__init__(name, if_type=consts.BACKEND, mtu=mtu) + def __init__(self, name, mtu, fixed_ips, is_sriov=False): + super().__init__(name, if_type=consts.BACKEND, mtu=mtu, + is_sriov=is_sriov) if fixed_ips: ip_versions = set() @@ -271,14 +277,21 @@ class PortInterfaceFile(InterfaceFile): consts.IPV6AUTO: True }) + if is_sriov: + sriov_param = ' sriov' + else: + sriov_param = '' + for ip_version in ip_versions: self.scripts[consts.IFACE_UP].append({ consts.COMMAND: ( - "/usr/local/bin/lvs-masquerade.sh add {} {}".format( - 'ipv6' if ip_version == 6 else 'ipv4', name)) + "/usr/local/bin/lvs-masquerade.sh add {} {}{}".format( + 'ipv6' if ip_version == 6 else 'ipv4', name, + sriov_param)) }) self.scripts[consts.IFACE_DOWN].append({ consts.COMMAND: ( - "/usr/local/bin/lvs-masquerade.sh delete {} {}".format( - 'ipv6' if ip_version == 6 else 'ipv4', name)) + "/usr/local/bin/lvs-masquerade.sh delete {} {}{}".format( + 'ipv6' if ip_version == 6 else 'ipv4', name, + sriov_param)) }) diff --git a/octavia/amphorae/backends/utils/nftable_utils.py b/octavia/amphorae/backends/utils/nftable_utils.py index 384f7bc0c5..b03f7813f1 100644 --- a/octavia/amphorae/backends/utils/nftable_utils.py +++ b/octavia/amphorae/backends/utils/nftable_utils.py @@ -26,16 +26,26 @@ from octavia.common import utils LOG = logging.getLogger(__name__) -def write_nftable_vip_rules_file(interface_name, rules): +def write_nftable_rules_file(interface_name, rules): flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC # mode 00600 mode = stat.S_IRUSR | stat.S_IWUSR # Create some strings shared on both code paths - table_string = f'table {consts.NFT_FAMILY} {consts.NFT_VIP_TABLE} {{\n' - chain_string = f' chain {consts.NFT_VIP_CHAIN} {{\n' - hook_string = (f' type filter hook ingress device {interface_name} ' - f'priority {consts.NFT_SRIOV_PRIORITY}; policy drop;\n') + table_string = f'table {consts.NFT_FAMILY} {consts.NFT_TABLE} {{\n' + chain_string = f' chain {consts.NFT_CHAIN} {{\n' + vip_chain_string = f' chain {consts.NFT_VIP_CHAIN} {{\n' + hook_string = (' type filter hook input priority filter; ' + 'policy drop;\n') + + # Conntrack is used to allow flow return traffic + conntrack_string = (' ct state vmap { established : accept, ' + 'related : accept, invalid : drop }\n') + + # Allow loopback traffic on the loopback interface, no where else + loopback_string = ' iif lo accept\n' + loopback_addr_string = ' ip saddr 127.0.0.0/8 drop\n' + loopback_ipv6_addr_string = ' ip6 saddr ::1 drop\n' # Allow ICMP destination unreachable for PMTUD icmp_string = ' icmp type destination-unreachable accept\n' @@ -47,38 +57,50 @@ def write_nftable_vip_rules_file(interface_name, rules): dhcp_string = ' udp sport 67 udp dport 68 accept\n' dhcpv6_string = ' udp sport 547 udp dport 546 accept\n' + # If the packet came in on the VIP interface, goto the VIP rules chain + vip_interface_goto_string = ( + f' iifname {consts.NETNS_PRIMARY_INTERFACE} ' + f'goto {consts.NFT_VIP_CHAIN}\n') + # Check if an existing rules file exists or we be need to create an # "drop all" file with no rules except for VRRP. If it exists, we should # not overwrite it here as it could be a reboot unless we were passed new # rules. - if os.path.isfile(consts.NFT_VIP_RULES_FILE): + if os.path.isfile(consts.NFT_RULES_FILE): if not rules: return with os.fdopen( - os.open(consts.NFT_VIP_RULES_FILE, flags, mode), 'w') as file: + os.open(consts.NFT_RULES_FILE, flags, mode), 'w') as file: # Clear the existing rules in the kernel # Note: The "nft -f" method is atomic, so clearing the rules will # not leave the amphora exposed. # Create and delete the table to not get errors if the table does # not exist yet. - file.write(f'table {consts.NFT_FAMILY} {consts.NFT_VIP_TABLE} ' + file.write(f'table {consts.NFT_FAMILY} {consts.NFT_TABLE} ' '{}\n') file.write(f'delete table {consts.NFT_FAMILY} ' - f'{consts.NFT_VIP_TABLE}\n') + f'{consts.NFT_TABLE}\n') file.write(table_string) file.write(chain_string) file.write(hook_string) + file.write(conntrack_string) + file.write(loopback_string) + file.write(loopback_addr_string) + file.write(loopback_ipv6_addr_string) file.write(icmp_string) file.write(icmpv6_string) file.write(dhcp_string) file.write(dhcpv6_string) + file.write(vip_interface_goto_string) + file.write(' }\n') # close the chain + file.write(vip_chain_string) for rule in rules: file.write(f' {_build_rule_cmd(rule)}\n') file.write(' }\n') # close the chain file.write('}\n') # close the table else: # No existing rules, create the "drop all" base rules with os.fdopen( - os.open(consts.NFT_VIP_RULES_FILE, flags, mode), 'w') as file: + os.open(consts.NFT_RULES_FILE, flags, mode), 'w') as file: file.write(table_string) file.write(chain_string) file.write(hook_string) @@ -113,7 +135,7 @@ def _build_rule_cmd(rule): def load_nftables_file(): - cmd = [consts.NFT_CMD, '-o', '-f', consts.NFT_VIP_RULES_FILE] + cmd = [consts.NFT_CMD, '-o', '-f', consts.NFT_RULES_FILE] try: with network_namespace.NetworkNamespace(consts.AMPHORA_NAMESPACE): subprocess.check_output(cmd, stderr=subprocess.STDOUT) diff --git a/octavia/common/constants.py b/octavia/common/constants.py index 1368c011e8..7e31cc2ad2 100644 --- a/octavia/common/constants.py +++ b/octavia/common/constants.py @@ -972,8 +972,8 @@ AMP_NET_DIR_TEMPLATE = '/etc/octavia/interfaces/' NFT_ADD = 'add' NFT_CMD = '/usr/sbin/nft' NFT_FAMILY = 'inet' -NFT_VIP_RULES_FILE = '/var/lib/octavia/nftables-vip.rules' -NFT_VIP_TABLE = 'amphora_vip' +NFT_RULES_FILE = '/var/lib/octavia/nftables-vip.rules' +NFT_TABLE = 'amphora_table' +NFT_CHAIN = 'amphora_chain' NFT_VIP_CHAIN = 'amphora_vip_chain' -NFT_SRIOV_PRIORITY = '-310' PROTOCOL = 'protocol' diff --git a/octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py b/octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py index c6da180e14..6f109179b9 100644 --- a/octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py +++ b/octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py @@ -2899,7 +2899,7 @@ class TestServerTestCase(base.TestCase): @mock.patch('octavia.amphorae.backends.utils.nftable_utils.' 'load_nftables_file') @mock.patch('octavia.amphorae.backends.utils.nftable_utils.' - 'write_nftable_vip_rules_file') + 'write_nftable_rules_file') @mock.patch('octavia.amphorae.backends.agent.api_server.amphora_info.' 'AmphoraInfo.get_interface') def test_set_interface_rules(self, mock_get_int, mock_write_rules, diff --git a/octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py b/octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py index ca34fe14b7..44f5b76c3b 100644 --- a/octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py +++ b/octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py @@ -227,5 +227,5 @@ class TestOSUtils(base.TestCase): mock_port_interface_file.assert_called_once_with( name=netns_interface, fixed_ips=fixed_ips, - mtu=MTU) + mtu=MTU, is_sriov=False) mock_port_interface_file.return_value.write.assert_called_once() diff --git a/octavia/tests/unit/amphorae/backends/agent/api_server/test_plug.py b/octavia/tests/unit/amphorae/backends/agent/api_server/test_plug.py index ec1b2a1c40..fb9fc094af 100644 --- a/octavia/tests/unit/amphorae/backends/agent/api_server/test_plug.py +++ b/octavia/tests/unit/amphorae/backends/agent/api_server/test_plug.py @@ -284,7 +284,7 @@ class TestPlug(base.TestCase): self.test_plug.plug_network(FAKE_MAC_ADDRESS, fixed_ips, 1400) mock_write_port_interface.assert_called_once_with( - interface='eth2', fixed_ips=fixed_ips, mtu=mtu) + interface='eth2', fixed_ips=fixed_ips, mtu=mtu, is_sriov=False) mock_if_up.assert_called_once_with('eth2', 'network') mock_send_member_adv.assert_called_once_with(fixed_ips) @@ -331,7 +331,8 @@ class TestPlug(base.TestCase): self.test_plug.plug_network(FAKE_MAC_ADDRESS, fixed_ips, 1400) mock_write_port_interface.assert_called_once_with( - interface=FAKE_INTERFACE, fixed_ips=fixed_ips, mtu=mtu) + interface=FAKE_INTERFACE, fixed_ips=fixed_ips, mtu=mtu, + is_sriov=False) mock_if_up.assert_called_once_with(FAKE_INTERFACE, 'network') mock_send_member_adv.assert_called_once_with(fixed_ips) @@ -399,7 +400,7 @@ class TestPlug(base.TestCase): 'gateway': vip_net_info['gateway'], 'host_routes': [], }, - fixed_ips=fixed_ips, mtu=mtu) + fixed_ips=fixed_ips, mtu=mtu, is_sriov=False) mock_if_up.assert_called_once_with(FAKE_INTERFACE, 'vip') mock_send_member_adv.assert_called_once_with(fixed_ips) diff --git a/octavia/tests/unit/amphorae/backends/utils/test_interface.py b/octavia/tests/unit/amphorae/backends/utils/test_interface.py index d67cb460f2..5d0df0bef2 100644 --- a/octavia/tests/unit/amphorae/backends/utils/test_interface.py +++ b/octavia/tests/unit/amphorae/backends/utils/test_interface.py @@ -454,7 +454,7 @@ class TestInterface(base.TestCase): @mock.patch('octavia.amphorae.backends.utils.network_namespace.' 'NetworkNamespace') @mock.patch('octavia.amphorae.backends.utils.nftable_utils.' - 'write_nftable_vip_rules_file') + 'write_nftable_rules_file') @mock.patch('pyroute2.IPRoute.rule') @mock.patch('pyroute2.IPRoute.route') @mock.patch('pyroute2.IPRoute.addr') @@ -580,16 +580,7 @@ class TestInterface(base.TestCase): family=socket.AF_INET6)]) mock_check_output.assert_has_calls([ - mock.call([consts.NFT_CMD, consts.NFT_ADD, 'table', - consts.NFT_FAMILY, consts.NFT_VIP_TABLE], - stderr=subprocess.STDOUT), - mock.call([consts.NFT_CMD, consts.NFT_ADD, 'chain', - consts.NFT_FAMILY, consts.NFT_VIP_TABLE, - consts.NFT_VIP_CHAIN, '{', 'type', 'filter', 'hook', - 'ingress', 'device', 'fake-eth1', 'priority', - consts.NFT_SRIOV_PRIORITY, ';', 'policy', 'drop', ';', - '}'], stderr=subprocess.STDOUT), - mock.call([consts.NFT_CMD, '-o', '-f', consts.NFT_VIP_RULES_FILE], + mock.call([consts.NFT_CMD, '-o', '-f', consts.NFT_RULES_FILE], stderr=subprocess.STDOUT), mock.call(["post-up", "fake-eth1"]) ]) @@ -1445,56 +1436,3 @@ class TestInterface(base.TestCase): addr = controller._normalize_ip_network(None) self.assertIsNone(addr) - - @mock.patch('octavia.amphorae.backends.utils.nftable_utils.' - 'load_nftables_file') - @mock.patch('octavia.amphorae.backends.utils.nftable_utils.' - 'write_nftable_vip_rules_file') - @mock.patch('subprocess.check_output') - def test__setup_nftables_chain(self, mock_check_output, mock_write_rules, - mock_load_rules): - - controller = interface.InterfaceController() - - mock_check_output.side_effect = [ - mock.DEFAULT, mock.DEFAULT, - subprocess.CalledProcessError(cmd=consts.NFT_CMD, returncode=-1), - mock.DEFAULT, - subprocess.CalledProcessError(cmd=consts.NFT_CMD, returncode=-1)] - - interface_mock = mock.MagicMock() - interface_mock.name = 'fake2' - - # Test succeessful path - controller._setup_nftables_chain(interface_mock) - - mock_write_rules.assert_called_once_with('fake2', []) - mock_load_rules.assert_called_once_with() - mock_check_output.assert_has_calls([ - mock.call([consts.NFT_CMD, 'add', 'table', consts.NFT_FAMILY, - consts.NFT_VIP_TABLE], stderr=subprocess.STDOUT), - mock.call([consts.NFT_CMD, 'add', 'chain', consts.NFT_FAMILY, - consts.NFT_VIP_TABLE, consts.NFT_VIP_CHAIN, '{', - 'type', 'filter', 'hook', 'ingress', 'device', - 'fake2', 'priority', consts.NFT_SRIOV_PRIORITY, ';', - 'policy', 'drop', ';', '}'], stderr=subprocess.STDOUT)]) - - # Test first nft call fails - mock_write_rules.reset_mock() - mock_load_rules.reset_mock() - mock_check_output.reset_mock() - - self.assertRaises(subprocess.CalledProcessError, - controller._setup_nftables_chain, interface_mock) - mock_check_output.assert_called_once() - mock_write_rules.assert_not_called() - - # Test second nft call fails - mock_write_rules.reset_mock() - mock_load_rules.reset_mock() - mock_check_output.reset_mock() - - self.assertRaises(subprocess.CalledProcessError, - controller._setup_nftables_chain, interface_mock) - self.assertEqual(2, mock_check_output.call_count) - mock_write_rules.assert_not_called() diff --git a/octavia/tests/unit/amphorae/backends/utils/test_nftable_utils.py b/octavia/tests/unit/amphorae/backends/utils/test_nftable_utils.py index 141a9cbe98..f6a7dc83fa 100644 --- a/octavia/tests/unit/amphorae/backends/utils/test_nftable_utils.py +++ b/octavia/tests/unit/amphorae/backends/utils/test_nftable_utils.py @@ -28,23 +28,22 @@ import octavia.tests.unit.base as base class TestNFTableUtils(base.TestCase): @mock.patch('os.open') @mock.patch('os.path.isfile') - def test_write_nftable_vip_rules_file_exists(self, mock_isfile, mock_open): + def test_write_nftable_rules_file_exists(self, mock_isfile, mock_open): """Test when a rules file exists and no new rules When an existing rules file is present and we call - write_nftable_vip_rules_file with no rules, the method should not + write_nftable_rules_file with no rules, the method should not overwrite the existing rules. """ mock_isfile.return_value = True - nftable_utils.write_nftable_vip_rules_file('fake-eth2', []) + nftable_utils.write_nftable_rules_file('fake-eth2', []) mock_open.assert_not_called() @mock.patch('os.open') @mock.patch('os.path.isfile') - def test_write_nftable_vip_rules_file_rules(self, mock_isfile, - mock_open): + def test_write_nftable_rules_file_rules(self, mock_isfile, mock_open): """Test when a rules file exists and rules are passed in This should create a simple rules file with the base chain and rules. @@ -61,32 +60,40 @@ class TestNFTableUtils(base.TestCase): mocked_open = mock.mock_open() with mock.patch.object(os, 'fdopen', mocked_open): - nftable_utils.write_nftable_vip_rules_file( + nftable_utils.write_nftable_rules_file( 'fake-eth2', [test_rule_1, test_rule_2]) mocked_open.assert_called_once_with('fake-fd', 'w') mock_open.assert_called_once_with( - consts.NFT_VIP_RULES_FILE, + consts.NFT_RULES_FILE, (os.O_WRONLY | os.O_CREAT | os.O_TRUNC), (stat.S_IRUSR | stat.S_IWUSR)) handle = mocked_open() handle.write.assert_has_calls([ - mock.call(f'table {consts.NFT_FAMILY} {consts.NFT_VIP_TABLE} ' + mock.call(f'table {consts.NFT_FAMILY} {consts.NFT_TABLE} ' '{}\n'), mock.call(f'delete table {consts.NFT_FAMILY} ' - f'{consts.NFT_VIP_TABLE}\n'), - mock.call(f'table {consts.NFT_FAMILY} {consts.NFT_VIP_TABLE} ' + f'{consts.NFT_TABLE}\n'), + mock.call(f'table {consts.NFT_FAMILY} {consts.NFT_TABLE} ' '{\n'), - mock.call(f' chain {consts.NFT_VIP_CHAIN} {{\n'), - mock.call(' type filter hook ingress device fake-eth2 ' - f'priority {consts.NFT_SRIOV_PRIORITY}; policy drop;\n'), + mock.call(f' chain {consts.NFT_CHAIN} {{\n'), + mock.call(' type filter hook input priority filter; ' + 'policy drop;\n'), + mock.call(' ct state vmap { established : accept, related : ' + 'accept, invalid : drop }\n'), + mock.call(' iif lo accept\n'), + mock.call(' ip saddr 127.0.0.0/8 drop\n'), + mock.call(' ip6 saddr ::1 drop\n'), mock.call(' icmp type destination-unreachable accept\n'), mock.call(' icmpv6 type { nd-neighbor-solicit, ' 'nd-router-advert, nd-neighbor-advert, packet-too-big, ' 'destination-unreachable } accept\n'), mock.call(' udp sport 67 udp dport 68 accept\n'), mock.call(' udp sport 547 udp dport 546 accept\n'), + mock.call(' iifname eth1 goto amphora_vip_chain\n'), + mock.call(' }\n'), + mock.call(' chain amphora_vip_chain {\n'), mock.call(' tcp dport 1234 accept\n'), mock.call(' ip saddr 192.0.2.0/24 ip protocol 112 accept\n'), mock.call(' }\n'), @@ -95,8 +102,7 @@ class TestNFTableUtils(base.TestCase): @mock.patch('os.open') @mock.patch('os.path.isfile') - def test_write_nftable_vip_rules_file_missing(self, mock_isfile, - mock_open): + def test_write_nftable_rules_file_missing(self, mock_isfile, mock_open): """Test when a rules file does not exist and no new rules This should create a simple rules file with the base chain. @@ -106,21 +112,21 @@ class TestNFTableUtils(base.TestCase): mocked_open = mock.mock_open() with mock.patch.object(os, 'fdopen', mocked_open): - nftable_utils.write_nftable_vip_rules_file('fake-eth2', []) + nftable_utils.write_nftable_rules_file('fake-eth2', []) mocked_open.assert_called_once_with('fake-fd', 'w') mock_open.assert_called_once_with( - consts.NFT_VIP_RULES_FILE, + consts.NFT_RULES_FILE, (os.O_WRONLY | os.O_CREAT | os.O_TRUNC), (stat.S_IRUSR | stat.S_IWUSR)) handle = mocked_open() handle.write.assert_has_calls([ - mock.call(f'table {consts.NFT_FAMILY} {consts.NFT_VIP_TABLE} ' + mock.call(f'table {consts.NFT_FAMILY} {consts.NFT_TABLE} ' '{\n'), - mock.call(f' chain {consts.NFT_VIP_CHAIN} {{\n'), - mock.call(' type filter hook ingress device fake-eth2 ' - f'priority {consts.NFT_SRIOV_PRIORITY}; policy drop;\n'), + mock.call(f' chain {consts.NFT_CHAIN} {{\n'), + mock.call(' type filter hook input priority filter; ' + 'policy drop;\n'), mock.call(' icmp type destination-unreachable accept\n'), mock.call(' icmpv6 type { nd-neighbor-solicit, ' 'nd-router-advert, nd-neighbor-advert, packet-too-big, ' @@ -184,7 +190,7 @@ class TestNFTableUtils(base.TestCase): mock_netns.assert_called_once_with(consts.AMPHORA_NAMESPACE) mock_check_output.assert_called_once_with([ - consts.NFT_CMD, '-o', '-f', consts.NFT_VIP_RULES_FILE], + consts.NFT_CMD, '-o', '-f', consts.NFT_RULES_FILE], stderr=subprocess.STDOUT) self.assertRaises(subprocess.CalledProcessError, diff --git a/releasenotes/notes/Fix-SR-IOV-when-VIP-interface-is-used-for-members-adb150ece454ecff.yaml b/releasenotes/notes/Fix-SR-IOV-when-VIP-interface-is-used-for-members-adb150ece454ecff.yaml new file mode 100644 index 0000000000..7ba615467a --- /dev/null +++ b/releasenotes/notes/Fix-SR-IOV-when-VIP-interface-is-used-for-members-adb150ece454ecff.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixed a bug in the VIP SR-IOV implementation that would cause load balancer + memebers that use the SR-IOV VIP interface to not receive traffic.