Implement "BridgeDevice" with Pyroute2

Change-Id: I9e64a4d4b931a132d25434eaeb9dcec6ebf0e6f8
Story: #2007686
Task: #39975

neutron/agent/linux/bridge_lib.py

@@ -16,11 +16,13 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import functools
 import os
 
-from oslo_utils import excutils
+from pyroute2.netlink import exceptions as netlink_exceptions
 
 from neutron.agent.linux import ip_lib
+from neutron.privileged.agent.linux import ip_lib as priv_ip_lib
 
 # NOTE(toabctl): Don't use /sys/devices/virtual/net here because not all tap
 # devices are listed here (i.e. when using Xen)
@@ -31,6 +33,20 @@ BRIDGE_PORT_FS_FOR_DEVICE = BRIDGE_FS + "%s/brport"
 BRIDGE_PATH_FOR_DEVICE = BRIDGE_PORT_FS_FOR_DEVICE + '/bridge'
 
 
+def catch_exceptions(function):
+    """Catch bridge command exceptions and mimic $? output"""
+
+    @functools.wraps(function)
+    def decorated_function(self, *args, **kwargs):
+        try:
+            function(self, *args, **kwargs)
+            return 0
+        except (RuntimeError, OSError, netlink_exceptions.NetlinkError):
+            return 1
+
+    return decorated_function
+
+
 def is_bridged_interface(interface):
     if not interface:
         return False
@@ -51,19 +67,14 @@ def get_bridge_names():
 
 
 class BridgeDevice(ip_lib.IPDevice):
-    def _ip_link(self, cmd):
-        cmd = ['ip', 'link'] + cmd
-        ip_wrapper = ip_lib.IPWrapper(self.namespace)
-        return ip_wrapper.netns.execute(cmd, run_as_root=True)
 
     @classmethod
     def addbr(cls, name, namespace=None):
         bridge = cls(name, namespace, 'bridge')
         try:
             bridge.link.create()
-        except RuntimeError:
+        except priv_ip_lib.InterfaceAlreadyExists:
-            with excutils.save_and_reraise_exception() as ectx:
+            pass
-                ectx.reraise = not bridge.exists()
         return bridge
 
     @classmethod
@@ -79,19 +90,28 @@ class BridgeDevice(ip_lib.IPDevice):
     def delbr(self):
         return self.link.delete()
 
+    @catch_exceptions
     def addif(self, interface):
-        return self._ip_link(['set', 'dev', interface, 'master', self.name])
+        priv_ip_lib.set_link_bridge_master(interface, self.name,
+                                           namespace=self.namespace)
 
+    @catch_exceptions
     def delif(self, interface):
-        return self._ip_link(['set', 'dev', interface, 'nomaster'])
+        priv_ip_lib.set_link_bridge_master(interface, None,
+                                           namespace=self.namespace)
 
+    @catch_exceptions
     def setfd(self, fd):
-        return self._ip_link(['set', 'dev', self.name, 'type', 'bridge',
+        priv_ip_lib.set_link_bridge_forward_delay(self.name, fd,
-                              'forward_delay', str(fd)])
+                                                  namespace=self.namespace)
 
+    @catch_exceptions
     def disable_stp(self):
-        return self._ip_link(['set', 'dev', self.name, 'type', 'bridge',
+        priv_ip_lib.set_link_bridge_stp(self.name, 0, namespace=self.namespace)
-                              'stp_state', 0])
+
+    @catch_exceptions
+    def enable_stp(self):
+        priv_ip_lib.set_link_bridge_stp(self.name, 1, namespace=self.namespace)
 
     def owns_interface(self, interface):
         return os.path.exists(

neutron/agent/linux/ip_lib.py

@@ -1395,6 +1395,10 @@ def get_devices_info(namespace, **kwargs):
                                                    'IFLA_VXLAN_LINK')
             elif ret['kind'] == 'vlan':
                 ret['vlan_id'] = get_attr(ifla_data, 'IFLA_VLAN_ID')
+            elif ret['kind'] == 'bridge':
+                ret['stp'] = get_attr(ifla_data, 'IFLA_BR_STP_STATE')
+                ret['forward_delay'] = get_attr(ifla_data,
+                                                'IFLA_BR_FORWARD_DELAY')
         retval[device['index']] = ret
 
     for device in retval.values():

neutron/privileged/agent/linux/ip_lib.py

@@ -415,6 +415,25 @@ def set_link_vf_feature(device, namespace, vf_config):
     return _run_iproute_link("set", device, namespace=namespace, vf=vf_config)
 
 
+@privileged.default.entrypoint
+def set_link_bridge_forward_delay(device, forward_delay, namespace=None):
+    return _run_iproute_link('set', device, namespace=namespace,
+                             kind='bridge', br_forward_delay=forward_delay)
+
+
+@privileged.default.entrypoint
+def set_link_bridge_stp(device, stp, namespace=None):
+    return _run_iproute_link('set', device, namespace=namespace,
+                             kind='bridge', br_stp_state=stp)
+
+
+@privileged.default.entrypoint
+def set_link_bridge_master(device, bridge, namespace=None):
+    bridge_idx = get_link_id(bridge, namespace) if bridge else 0
+    return _run_iproute_link('set', device, namespace=namespace,
+                             master=bridge_idx)
+
+
 @privileged.default.entrypoint
 def get_link_attributes(device, namespace):
     link = _run_iproute_link("get", device, namespace)[0]

neutron/tests/common/net_helpers.py

@@ -864,7 +864,7 @@ class OVSPortFixture(PortFixture):
         self.addCleanup(self.port.link.delete)
         ip_wrapper.add_device_to_namespace(self.port)
         bridge_port.link.set_up()
-        self.qbr.addif(bridge_port)
+        self.qbr.addif(bridge_port.name)
 
         self.port.link.set_address(self.mac)
         self.port.link.set_up()
@@ -963,7 +963,7 @@ class LinuxBridgePortFixture(PortFixture):
         # bridge side
         br_ip_wrapper = ip_lib.IPWrapper(self.bridge.namespace)
         br_ip_wrapper.add_device_to_namespace(self.br_port)
-        self.bridge.addif(self.br_port)
+        self.bridge.addif(self.br_port.name)
         self.br_port.link.set_up()
 
         # port side

neutron/tests/functional/agent/linux/test_bridge_lib.py

@@ -41,14 +41,23 @@ class BridgeLibTestCase(base.BaseSudoTestCase):
                 bridge, port_id=uuidutils.generate_uuid()))
         return bridge, port_fixture
 
-    def test_is_bridged_interface(self):
+    def test_is_bridged_interface_and_remove(self):
-        self.assertTrue(
+        bridge = bridge_lib.BridgeDevice(self.bridge.name)
-            bridge_lib.is_bridged_interface(self.port_fixture.br_port.name))
+        bridge_port = self.port_fixture.br_port.name
+        self.assertTrue(bridge_lib.is_bridged_interface(bridge_port))
+        bridge.delif(bridge_port)
+        self.assertFalse(bridge_lib.is_bridged_interface(bridge_port))
 
     def test_is_not_bridged_interface(self):
         self.assertFalse(
             bridge_lib.is_bridged_interface(self.port_fixture.port.name))
 
+    def test_delete_bridge(self):
+        bridge = bridge_lib.BridgeDevice(self.bridge.name)
+        self.assertTrue(bridge.exists())
+        bridge.delbr()
+        self.assertFalse(bridge.exists())
+
     def test_get_bridge_names(self):
         self.assertIn(self.bridge.name, bridge_lib.get_bridge_names())
 
@@ -94,6 +103,29 @@ class BridgeLibTestCase(base.BaseSudoTestCase):
             sysfs_disable_ipv6 = sysfs_disable_ipv6_file.read()
             self.assertEqual("1\n", sysfs_disable_ipv6)
 
+    def _get_bridge_info(self):
+        for device in (device for device in ip_lib.get_devices_info(
+                self.bridge.namespace) if device['name'] == self.bridge.name):
+            return device
+        self.fail('Bridge %s not present' % self.bridge.name)
+
+    def test_set_forward_delay(self):
+        bridge = bridge_lib.BridgeDevice(self.bridge.name)
+        for fd in (10, 200, 3000, 40000):
+            bridge.setfd(fd)
+            br_info = self._get_bridge_info()
+            self.assertEqual(fd, br_info['forward_delay'])
+
+    def test_enable_and_disable_stp(self):
+        bridge = bridge_lib.BridgeDevice(self.bridge.name)
+        bridge.disable_stp()
+        br_info = self._get_bridge_info()
+        self.assertEqual(0, br_info['stp'])
+
+        bridge.enable_stp()
+        br_info = self._get_bridge_info()
+        self.assertEqual(1, br_info['stp'])
+
 
 class FdbInterfaceTestCase(testscenarios.WithScenarios, base.BaseSudoTestCase):
 

neutron/tests/unit/agent/linux/test_bridge_lib.py

@@ -65,48 +65,10 @@ class BridgeLibTest(base.BaseTestCase):
             br = bridge_lib.BridgeDevice.get_interface_bridge('tap0')
             self.assertIsNone(br)
 
-    def _test_br(self, namespace=None):
-        br = bridge_lib.BridgeDevice.addbr(self._BR_NAME, namespace)
-        self.assertEqual(namespace, br.namespace)
-        self.create.assert_called_once_with(self._BR_NAME, br.namespace,
-                                            'bridge')
-
-        br.setfd(0)
-        self._verify_bridge_mock(['ip', 'link', 'set', 'dev', self._BR_NAME,
-                                  'type', 'bridge', 'forward_delay', '0'])
-
-        br.disable_stp()
-        self._verify_bridge_mock(['ip', 'link', 'set', 'dev', self._BR_NAME,
-                                  'type', 'bridge', 'stp_state', 0])
-
-        br.disable_ipv6()
-        cmd = 'net.ipv6.conf.%s.disable_ipv6=1' % self._BR_NAME
-        self._verify_bridge_sysctl_mock(['sysctl', '-w', cmd])
-
-        br.addif(self._IF_NAME)
-        self._verify_bridge_mock(
-            ['ip', 'link', 'set', 'dev', self._IF_NAME,
-             'master', self._BR_NAME])
-
-        br.delif(self._IF_NAME)
-        self._verify_bridge_mock(
-            ['ip', 'link', 'set', 'dev', self._IF_NAME, 'nomaster'])
-
-        br.delbr()
-        self.delete.assert_called_once_with(self._BR_NAME, br.namespace)
-
-    def test_addbr_with_namespace(self):
-        self._test_br(self._NAMESPACE)
-
-    def test_addbr_without_namespace(self):
-        self._test_br()
-
     def test_addbr_exists(self):
-        self.create.side_effect = RuntimeError()
+        self.create.side_effect = priv_lib.InterfaceAlreadyExists
-        with mock.patch.object(bridge_lib.BridgeDevice, 'exists',
+        bridge_lib.BridgeDevice.addbr(self._BR_NAME)
-                               return_value=True):
+        bridge_lib.BridgeDevice.addbr(self._BR_NAME)
-            bridge_lib.BridgeDevice.addbr(self._BR_NAME)
-            bridge_lib.BridgeDevice.addbr(self._BR_NAME)
 
     def test_owns_interface(self):
         br = bridge_lib.BridgeDevice('br-int')

neutron/tests/unit/plugins/ml2/drivers/linuxbridge/agent/test_linuxbridge_neutron_agent.py

@@ -521,10 +521,10 @@ class TestLinuxBridgeManager(base.BaseTestCase):
                                   'get_interface_bridge') as get_if_br_fn:
             de_fn.return_value = False
             br_fn.addbr.return_value = bridge_device
-            bridge_device.setfd.return_value = False
+            bridge_device.setfd.return_value = 0
-            bridge_device.disable_stp.return_value = False
+            bridge_device.disable_stp.return_value = 0
             bridge_device.disable_ipv6.return_value = False
-            bridge_device.link.set_up.return_value = False
+            bridge_device.link.set_up.return_value = 0
             self.assertEqual("br0", self.lbm.ensure_bridge("br0", None))
 
             bridge_device.owns_interface.return_value = False
@@ -617,7 +617,6 @@ class TestLinuxBridgeManager(base.BaseTestCase):
                     mock.patch.object(self.lbm, '_set_tap_mtu') as set_tap, \
                     mock.patch.object(bridge_lib.BridgeDevice,
                                       "get_interface_bridge") as get_br:
-                bridge_device.addif.retun_value = False
                 get_br.return_value = True
                 self.assertTrue(self.lbm.add_tap_interface(
                     "123", constants.TYPE_LOCAL, "physnet1", None,
@@ -632,7 +631,7 @@ class TestLinuxBridgeManager(base.BaseTestCase):
                 en_fn.assert_called_with("123", "brq999")
 
                 get_br.return_value = False
-                bridge_device.addif.retun_value = True
+                bridge_device.addif.retun_value = 1
                 self.assertFalse(self.lbm.add_tap_interface(
                     "123", constants.TYPE_LOCAL, "physnet1",
                     None, "tap1", dev_owner_prefix, None))