openstack/octavia
Code Issues Proposed changes

Fix SR-IOV when VIP interface is used for members

Browse Source 
This patch fixes 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.

Closes-Bug: #2070312
Change-Id: Ice6e4ee98edb446a8b0547ced477598dcf22f791
This commit is contained in:
Michael Johnson 2024-06-24 21:12:00 +00:00
parent 9873200f68
commit 8b8d969d00
14 changed files with 151 additions and 185 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
elements/amphora-agent/static/usr/local/bin/lvs-masquerade.sh
View File

@ -19,7 +19,7 @@ set -e
usage() {
echo
echo "Usage: $(basename "$0") [add|delete] [ipv4|ipv6] <interface>"
echo "Usage: $(basename "$0") [add|delete] [ipv4|ipv6] <interface> [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
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
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
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
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

6
octavia/amphorae/backends/agent/api_server/osutils.py
View File

@ -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

9
octavia/amphorae/backends/agent/api_server/plug.py
View File

@ -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)

5
octavia/amphorae/backends/agent/api_server/server.py
View File

@ -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()

39
octavia/amphorae/backends/utils/interface.py
View File

@ -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]

33
octavia/amphorae/backends/utils/interface_file.py
View File

@ -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))
})

44
octavia/amphorae/backends/utils/nftable_utils.py
View File

@ -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)

6
octavia/common/constants.py
View File

@ -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'

2
octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py
View File

@ -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,

2
octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py
View File

@ -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()

7
octavia/tests/unit/amphorae/backends/agent/api_server/test_plug.py
View File

@ -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)

66
octavia/tests/unit/amphorae/backends/utils/test_interface.py
View File

@ -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()

50
octavia/tests/unit/amphorae/backends/utils/test_nftable_utils.py
View File

@ -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,

5
releasenotes/notes/Fix-SR-IOV-when-VIP-interface-is-used-for-members-adb150ece454ecff.yaml Normal file
View File

@ -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.