From 103cb6bdc321f1f7fdc09bf8a5b178ff6ecd8f9f Mon Sep 17 00:00:00 2001 From: Chet Burgess Date: Tue, 17 Nov 2015 11:43:09 -0800 Subject: [PATCH] Use --concurrent with ebtables Update our usage of ebtables to pass the --concurrent flag. With --concurrent ebtables will attempt to acquire a lock file before making changes. This allows multiple processes, such as nova an libvirt, to safely access ebtables at the same time. Support for using --concurrent was added in libvirt 1.2.11, since we don't know the version of libvirt being used in all deployments we still retry the ebtables call several times just in case libvirt isn't using --concurrent. DocImpact: * nova now requires ebtables 2.0.10 or later * nova now recommends libvirt 1.2.11 or later Change-Id: I00ff805cee9653508f013f8aa6d206362ac0f6cb Partial-Bug: #1501366 --- nova/network/linux_net.py | 24 ++++---- nova/tests/unit/network/test_linux_net.py | 60 +++++++++---------- .../ebtables-version-fde659fe18b0e0c0.yaml | 6 ++ 3 files changed, 50 insertions(+), 40 deletions(-) create mode 100644 releasenotes/notes/ebtables-version-fde659fe18b0e0c0.yaml diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index 6487580a28..8e3c242779 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -1711,13 +1711,17 @@ class LinuxBridgeInterfaceDriver(LinuxNetInterfaceDriver): delete_bridge_dev(bridge) -# NOTE(cfb): This is a temporary fix to LP #1316621. We really want to call -# ebtables with --concurrent. In order to do that though we need -# libvirt to support this. Additionally since ebtables --concurrent -# will hang indefinitely waiting on the lock we need to teach -# oslo_concurrency.processutils how to timeout a long running -# process first. Once those are complete we can replace all of this -# with calls to ebtables --concurrent and a reasonable timeout. +# NOTE(cfb): Fix for LP #1316621, #1501366. +# We call ebtables with --concurrent which causes ebtables to +# use a lock file to deal with concurrent calls. Since we can't +# be sure the libvirt also uses --concurrent we retry in a loop +# to be sure. +# +# ebtables doesn't implement a timeout and doesn't gracefully +# handle cleaning up a lock file if someone sends a SIGKILL to +# ebtables while its holding a lock. As a result we want to add +# a timeout to the ebtables calls but we first need to teach +# oslo_concurrency how to do that. def _exec_ebtables(*cmd, **kwargs): check_exit_code = kwargs.pop('check_exit_code', True) @@ -1768,16 +1772,16 @@ def _exec_ebtables(*cmd, **kwargs): @utils.synchronized('ebtables', external=True) def ensure_ebtables_rules(rules, table='filter'): for rule in rules: - cmd = ['ebtables', '-t', table, '-D'] + rule.split() + cmd = ['ebtables', '--concurrent', '-t', table, '-D'] + rule.split() _exec_ebtables(*cmd, check_exit_code=False, run_as_root=True) - cmd[3] = '-I' + cmd[4] = '-I' _exec_ebtables(*cmd, run_as_root=True) @utils.synchronized('ebtables', external=True) def remove_ebtables_rules(rules, table='filter'): for rule in rules: - cmd = ['ebtables', '-t', table, '-D'] + rule.split() + cmd = ['ebtables', '--concurrent', '-t', table, '-D'] + rule.split() _exec_ebtables(*cmd, check_exit_code=False, run_as_root=True) diff --git a/nova/tests/unit/network/test_linux_net.py b/nova/tests/unit/network/test_linux_net.py index e47f719bb5..e48ba83e4b 100644 --- a/nova/tests/unit/network/test_linux_net.py +++ b/nova/tests/unit/network/test_linux_net.py @@ -841,26 +841,26 @@ class LinuxNetworkTestCase(test.NoDBTestCase): 'bridge_interface': iface} driver.plug(network, 'fakemac') expected = [ - ('ebtables', '-t', 'filter', '-D', 'INPUT', '-p', 'ARP', '-i', - iface, '--arp-ip-dst', dhcp, '-j', 'DROP'), - ('ebtables', '-t', 'filter', '-I', 'INPUT', '-p', 'ARP', '-i', - iface, '--arp-ip-dst', dhcp, '-j', 'DROP'), - ('ebtables', '-t', 'filter', '-D', 'OUTPUT', '-p', 'ARP', '-o', - iface, '--arp-ip-src', dhcp, '-j', 'DROP'), - ('ebtables', '-t', 'filter', '-I', 'OUTPUT', '-p', 'ARP', '-o', - iface, '--arp-ip-src', dhcp, '-j', 'DROP'), - ('ebtables', '-t', 'filter', '-D', 'FORWARD', '-p', 'IPv4', '-i', - iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68', - '-j', 'DROP'), - ('ebtables', '-t', 'filter', '-I', 'FORWARD', '-p', 'IPv4', '-i', - iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68', - '-j', 'DROP'), - ('ebtables', '-t', 'filter', '-D', 'FORWARD', '-p', 'IPv4', '-o', - iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68', - '-j', 'DROP'), - ('ebtables', '-t', 'filter', '-I', 'FORWARD', '-p', 'IPv4', '-o', - iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68', - '-j', 'DROP'), + ('ebtables', '--concurrent', '-t', 'filter', '-D', 'INPUT', '-p', + 'ARP', '-i', iface, '--arp-ip-dst', dhcp, '-j', 'DROP'), + ('ebtables', '--concurrent', '-t', 'filter', '-I', 'INPUT', '-p', + 'ARP', '-i', iface, '--arp-ip-dst', dhcp, '-j', 'DROP'), + ('ebtables', '--concurrent', '-t', 'filter', '-D', 'OUTPUT', '-p', + 'ARP', '-o', iface, '--arp-ip-src', dhcp, '-j', 'DROP'), + ('ebtables', '--concurrent', '-t', 'filter', '-I', 'OUTPUT', '-p', + 'ARP', '-o', iface, '--arp-ip-src', dhcp, '-j', 'DROP'), + ('ebtables', '--concurrent', '-t', 'filter', '-D', 'FORWARD', + '-p', 'IPv4', '-i', iface, '--ip-protocol', 'udp', + '--ip-destination-port', '67:68', '-j', 'DROP'), + ('ebtables', '--concurrent', '-t', 'filter', '-I', 'FORWARD', + '-p', 'IPv4', '-i', iface, '--ip-protocol', 'udp', + '--ip-destination-port', '67:68', '-j', 'DROP'), + ('ebtables', '--concurrent', '-t', 'filter', '-D', 'FORWARD', + '-p', 'IPv4', '-o', iface, '--ip-protocol', 'udp', + '--ip-destination-port', '67:68', '-j', 'DROP'), + ('ebtables', '--concurrent', '-t', 'filter', '-I', 'FORWARD', + '-p', 'IPv4', '-o', iface, '--ip-protocol', 'udp', + '--ip-destination-port', '67:68', '-j', 'DROP'), ('iptables-save', '-c'), ('iptables-restore', '-c'), ('ip6tables-save', '-c'), @@ -879,16 +879,16 @@ class LinuxNetworkTestCase(test.NoDBTestCase): driver.unplug(network) expected = [ - ('ebtables', '-t', 'filter', '-D', 'INPUT', '-p', 'ARP', '-i', - iface, '--arp-ip-dst', dhcp, '-j', 'DROP'), - ('ebtables', '-t', 'filter', '-D', 'OUTPUT', '-p', 'ARP', '-o', - iface, '--arp-ip-src', dhcp, '-j', 'DROP'), - ('ebtables', '-t', 'filter', '-D', 'FORWARD', '-p', 'IPv4', '-i', - iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68', - '-j', 'DROP'), - ('ebtables', '-t', 'filter', '-D', 'FORWARD', '-p', 'IPv4', '-o', - iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68', - '-j', 'DROP'), + ('ebtables', '--concurrent', '-t', 'filter', '-D', 'INPUT', '-p', + 'ARP', '-i', iface, '--arp-ip-dst', dhcp, '-j', 'DROP'), + ('ebtables', '--concurrent', '-t', 'filter', '-D', 'OUTPUT', '-p', + 'ARP', '-o', iface, '--arp-ip-src', dhcp, '-j', 'DROP'), + ('ebtables', '--concurrent', '-t', 'filter', '-D', 'FORWARD', + '-p', 'IPv4', '-i', iface, '--ip-protocol', 'udp', + '--ip-destination-port', '67:68', '-j', 'DROP'), + ('ebtables', '--concurrent', '-t', 'filter', '-D', 'FORWARD', + '-p', 'IPv4', '-o', iface, '--ip-protocol', 'udp', + '--ip-destination-port', '67:68', '-j', 'DROP'), ] self.assertEqual(expected, executes) diff --git a/releasenotes/notes/ebtables-version-fde659fe18b0e0c0.yaml b/releasenotes/notes/ebtables-version-fde659fe18b0e0c0.yaml new file mode 100644 index 0000000000..bb56949f26 --- /dev/null +++ b/releasenotes/notes/ebtables-version-fde659fe18b0e0c0.yaml @@ -0,0 +1,6 @@ +--- +prelude: + ebtables and libvirt requirements +upgrade: + - nova now requires ebtables 2.0.10 or later + - nova recommends libvirt 1.2.11 or later