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

@@ -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
+# NOTE(cfb): Fix for LP #1316621, #1501366.
-#            ebtables with --concurrent. In order to do that though we need
+#            We call ebtables with --concurrent which causes ebtables to
-#            libvirt to support this. Additionally since ebtables --concurrent
+#            use a lock file to deal with concurrent calls. Since we can't
-#            will hang indefinitely waiting on the lock we need to teach
+#            be sure the libvirt also uses --concurrent we retry in a loop
-#            oslo_concurrency.processutils how to timeout a long running
+#            to be sure.
-#            process first. Once those are complete we can replace all of this
+#
-#            with calls to ebtables --concurrent and a reasonable timeout.
+#            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)
 
 

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',
+            ('ebtables', '--concurrent', '-t', 'filter', '-D', 'INPUT', '-p',
-             iface, '--arp-ip-dst', dhcp, '-j', 'DROP'),
+             'ARP', '-i', iface, '--arp-ip-dst', dhcp, '-j', 'DROP'),
-            ('ebtables', '-t', 'filter', '-I', 'INPUT', '-p', 'ARP', '-i',
+            ('ebtables', '--concurrent', '-t', 'filter', '-I', 'INPUT', '-p',
-             iface, '--arp-ip-dst', dhcp, '-j', 'DROP'),
+             'ARP', '-i', iface, '--arp-ip-dst', dhcp, '-j', 'DROP'),
-            ('ebtables', '-t', 'filter', '-D', 'OUTPUT', '-p', 'ARP', '-o',
+            ('ebtables', '--concurrent', '-t', 'filter', '-D', 'OUTPUT', '-p',
-             iface, '--arp-ip-src', dhcp, '-j', 'DROP'),
+             'ARP', '-o', iface, '--arp-ip-src', dhcp, '-j', 'DROP'),
-            ('ebtables', '-t', 'filter', '-I', 'OUTPUT', '-p', 'ARP', '-o',
+            ('ebtables', '--concurrent', '-t', 'filter', '-I', 'OUTPUT', '-p',
-             iface, '--arp-ip-src', dhcp, '-j', 'DROP'),
+             'ARP', '-o', iface, '--arp-ip-src', dhcp, '-j', 'DROP'),
-            ('ebtables', '-t', 'filter', '-D', 'FORWARD', '-p', 'IPv4', '-i',
+            ('ebtables', '--concurrent', '-t', 'filter', '-D', 'FORWARD',
-             iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68',
+             '-p', 'IPv4', '-i', iface, '--ip-protocol', 'udp',
-             '-j', 'DROP'),
+             '--ip-destination-port', '67:68', '-j', 'DROP'),
-            ('ebtables', '-t', 'filter', '-I', 'FORWARD', '-p', 'IPv4', '-i',
+            ('ebtables', '--concurrent', '-t', 'filter', '-I', 'FORWARD',
-             iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68',
+             '-p', 'IPv4', '-i', iface, '--ip-protocol', 'udp',
-             '-j', 'DROP'),
+             '--ip-destination-port', '67:68', '-j', 'DROP'),
-            ('ebtables', '-t', 'filter', '-D', 'FORWARD', '-p', 'IPv4', '-o',
+            ('ebtables', '--concurrent', '-t', 'filter', '-D', 'FORWARD',
-             iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68',
+             '-p', 'IPv4', '-o', iface, '--ip-protocol', 'udp',
-             '-j', 'DROP'),
+             '--ip-destination-port', '67:68', '-j', 'DROP'),
-            ('ebtables', '-t', 'filter', '-I', 'FORWARD', '-p', 'IPv4', '-o',
+            ('ebtables', '--concurrent', '-t', 'filter', '-I', 'FORWARD',
-             iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68',
+             '-p', 'IPv4', '-o', iface, '--ip-protocol', 'udp',
-             '-j', 'DROP'),
+             '--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',
+            ('ebtables', '--concurrent', '-t', 'filter', '-D', 'INPUT', '-p',
-             iface, '--arp-ip-dst', dhcp, '-j', 'DROP'),
+             'ARP', '-i', iface, '--arp-ip-dst', dhcp, '-j', 'DROP'),
-            ('ebtables', '-t', 'filter', '-D', 'OUTPUT', '-p', 'ARP', '-o',
+            ('ebtables', '--concurrent', '-t', 'filter', '-D', 'OUTPUT', '-p',
-             iface, '--arp-ip-src', dhcp, '-j', 'DROP'),
+             'ARP', '-o', iface, '--arp-ip-src', dhcp, '-j', 'DROP'),
-            ('ebtables', '-t', 'filter', '-D', 'FORWARD', '-p', 'IPv4', '-i',
+            ('ebtables', '--concurrent', '-t', 'filter', '-D', 'FORWARD',
-             iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68',
+             '-p', 'IPv4', '-i', iface, '--ip-protocol', 'udp',
-             '-j', 'DROP'),
+             '--ip-destination-port', '67:68', '-j', 'DROP'),
-            ('ebtables', '-t', 'filter', '-D', 'FORWARD', '-p', 'IPv4', '-o',
+            ('ebtables', '--concurrent', '-t', 'filter', '-D', 'FORWARD',
-             iface, '--ip-protocol', 'udp', '--ip-destination-port', '67:68',
+             '-p', 'IPv4', '-o', iface, '--ip-protocol', 'udp',
-             '-j', 'DROP'),
+             '--ip-destination-port', '67:68', '-j', 'DROP'),
         ]
         self.assertEqual(expected, executes)
 

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