Merge "only disable mac ageing for ovs hybrid plug"

os_vif/internal/ip/ip_command.py

@@ -39,7 +39,7 @@ class IpCommand(object):
 
     @abc.abstractmethod
     def add(self, device, dev_type, check_exit_code=None, peer=None, link=None,
-            vlan_id=None):
+            vlan_id=None, ageing=None):
         """Method to add an interface.
 
         :param   device: A network device (string)
@@ -50,6 +50,8 @@ class IpCommand(object):
         :param   link: String root network interface name, 'device' will be a
                        VLAN tagged virtual interface
         :param   vlan_id: Integer VLAN ID for VLAN devices
+        :param   ageing: integer value in seconds before learned
+                         mac addresses are forgotten.
         :return: status of the command execution
         """
 

os_vif/internal/ip/linux/impl_pyroute2.py

@@ -67,7 +67,7 @@ class PyRoute2(ip_command.IpCommand):
             return self._ip_link(ip, 'set', check_exit_code, **args)
 
     def add(self, device, dev_type, check_exit_code=None, peer=None, link=None,
-            vlan_id=None):
+            vlan_id=None, ageing=None):
         check_exit_code = check_exit_code or []
         with iproute.IPRoute() as ip:
             args = {'ifname': device,
@@ -86,10 +86,18 @@ class PyRoute2(ip_command.IpCommand):
                 # in the bridge_data class located in the
                 # pyroute2.netlink.rtnl.ifinfmsg module for mode details
                 # https://github.com/svinota/pyroute2/blob/3ba9cdde34b2346ef8c2f8ba17cef5dbeb4c6d52/pyroute2/netlink/rtnl/ifinfmsg/__init__.py#L776-L820
-                args['IFLA_BR_AGEING_TIME'] = 0  # disable mac learning ageing
                 args['IFLA_BR_FORWARD_DELAY'] = 0  # set no delay
                 args['IFLA_BR_STP_STATE'] = 0  # disable spanning tree
                 args['IFLA_BR_MCAST_SNOOPING'] = 0  # disable snooping
+                # NOTE(sean-k-mooney): we conditionally enable mac ageing as
+                # this code is shared between the ovs and linux bridge
+                # plugins. For linux bridge we want to allow the default
+                # ageing of 300 seconds, whereas for ovs with the ip-tables
+                # firewall we want to disable ageing. None was chosen as
+                # the default value of ageing to allow the caller to determine
+                # what policy to use and keep this code generic.
+                if ageing is not None:
+                    args['IFLA_BR_AGEING_TIME'] = ageing
             else:
                 raise exception.NetworkInterfaceTypeNotDefined(type=dev_type)
 

os_vif/tests/functional/internal/command/ip/test_impl_pyroute2.py

@@ -213,6 +213,28 @@ class TestIpCommand(ShellIpCommands, base.BaseFunctionalTestCase):
         _ip_cmd_add(device, 'bridge')
         self.assertTrue(self.exist_device(device))
         base_path = "/sys/class/net/test_dev_11/bridge/%s"
+        with open(base_path % "forward_delay", "r") as f:
+            self.assertEqual("0", f.readline().rstrip('\n'))
+        with open(base_path % "stp_state", "r") as f:
+            self.assertEqual("0", f.readline().rstrip('\n'))
+        with open(base_path % "multicast_snooping", "r") as f:
+            self.assertEqual("0", f.readline().rstrip('\n'))
+        with open(base_path % "ageing_time", "r") as f:
+            value = int(f.readline().rstrip('\n'))
+            # NOTE(sean-k-mooney): IEEE 8021-Q recommends that the default
+            # ageing should be 300 and the linux kernel defaults to 300
+            # via an unconditional define. As such we expect this to be
+            # 300 however since services like network-manager could change
+            # the default on bridge creation we check that if it is not 300
+            # then the value should not be 0.
+            self.assertTrue(300 == value or value != 0)
+
+    def test_add_bridge_with_mac_ageing_0(self):
+        device = "test_dev_12"
+        self.addCleanup(self.del_device, device)
+        _ip_cmd_add(device, 'bridge', ageing=0)
+        self.assertTrue(self.exist_device(device))
+        base_path = "/sys/class/net/test_dev_12/bridge/%s"
         with open(base_path % "forward_delay", "r") as f:
             self.assertEqual("0", f.readline().rstrip('\n'))
         with open(base_path % "stp_state", "r") as f:
@@ -223,8 +245,8 @@ class TestIpCommand(ShellIpCommands, base.BaseFunctionalTestCase):
             self.assertEqual("0", f.readline().rstrip('\n'))
 
     def test_add_port_to_bridge(self):
-        device = "test_dev_12"
+        device = "test_dev_13"
-        bridge = "test_dev_13"
+        bridge = "test_dev_14"
         self.addCleanup(self.del_device, device)
         self.addCleanup(self.del_device, bridge)
         self.add_device(device, 'dummy')

os_vif/tests/unit/internal/ip/linux/test_impl_pyroute2.py

@@ -95,6 +95,15 @@ class TestIpCommand(base.TestCase):
 
     def test_add_bridge(self):
         self.ip.add(self.DEVICE, self.TYPE_BRIDGE)
+        args = {'ifname': self.DEVICE,
+                'kind': self.TYPE_BRIDGE,
+                'IFLA_BR_FORWARD_DELAY': 0,
+                'IFLA_BR_STP_STATE': 0,
+                'IFLA_BR_MCAST_SNOOPING': 0}
+        self.ip_link.assert_called_once_with('add', **args)
+
+    def test_add_bridge_with_ageing(self):
+        self.ip.add(self.DEVICE, self.TYPE_BRIDGE, ageing=0)
         args = {'ifname': self.DEVICE,
                 'kind': self.TYPE_BRIDGE,
                 'IFLA_BR_AGEING_TIME': 0,

releasenotes/notes/do-not-force-mac-ageing-c6e8d750130c5740.yaml

@@ -0,0 +1,13 @@
+---
+fixes:
+  - |
+    As part of a `bug #1715317`_, MAC ageing was disabled for the intermediate
+    bridge created as part of the hybrid plug mechanism. During the removal
+    of ``brctl``, this behavior was inadvertently applied to all linux bridges
+    created by os-vif including those used in the linuxbridge driver.
+    As a result this can lead to packet flooding (see bug #1837252) when
+    instances are migrated. This behavior has been reverted so that the
+    default mac ageing is determined by the kernel and is not set when using
+    the os-vif linux bridge plugin.
+
+    .. _bug #1715317: https://bugs.launchpad.net/os-vif/+bug/1837252

vif_plug_ovs/linux_net.py

@@ -129,7 +129,10 @@ def _arp_filtering(bridge):
 @privsep.vif_plug.entrypoint
 def ensure_bridge(bridge):
     if not ip_lib.exists(bridge):
-        ip_lib.add(bridge, 'bridge')
+        # NOTE(sean-k-mooney): we set mac ageing to 0 to disable mac ageing
+        # on the hybrid plug bridge to avoid packet loss during live
+        # migration. This avoids bug #1715317 and related bug #1414559
+        ip_lib.add(bridge, 'bridge', ageing=0)
     _disable_ipv6(bridge)
     _arp_filtering(bridge)
     # we bring up the bridge to allow it to switch packets

vif_plug_ovs/tests/unit/test_linux_net.py

@@ -41,7 +41,7 @@ class LinuxNetTest(testtools.TestCase):
         linux_net.ensure_bridge("br0")
 
         mock_dev_exists.assert_called_once_with("br0")
-        mock_add.assert_called_once_with("br0", "bridge")
+        mock_add.assert_called_once_with("br0", "bridge", ageing=0)
         mock_disable_ipv6.assert_called_once_with("br0")
         mock_set_state.assert_called_once_with("br0", "up")
         mock_arp_filtering.assert_called_once_with("br0")