Adding FDB population agent extension

The purpose of this extension is updating the FDB table upon changes of
normal port instances thus enabling communication between direct port
SR-IOV instances and normal port instances.
Additionally enabling communication to direct port
instances with floating ips.
Support for OVS agent and linux bridge.

DocImpact
Change-Id: I61a8aacb1b21b2a6e452389633d7dcccf9964fea
Closes-Bug: #1492228
Closes-Bug: #1527991

neutron/agent/l2/extensions/fdb_population.py

@@ -0,0 +1,172 @@
+# Copyright (c) 2016 Mellanox Technologies, Ltd
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import sys
+
+from oslo_config import cfg
+from oslo_log import log as logging
+
+from neutron._i18n import _, _LE, _LW
+from neutron.agent.l2 import agent_extension
+from neutron.agent.linux import bridge_lib
+from neutron.common import utils as n_utils
+from neutron.plugins.ml2.drivers.linuxbridge.agent.common import (
+     constants as linux_bridge_constants)
+from neutron.plugins.ml2.drivers.openvswitch.agent.common import (
+     constants as ovs_constants)
+
+# if shared_physical_device_mappings is not configured KeyError will be thrown
+fdb_population_opt = [
+    cfg.ListOpt('shared_physical_device_mappings', default=[],
+                help=_("Comma-separated list of "
+                       "<physical_network>:<network_device> tuples mapping "
+                       "physical network names to the agent's node-specific "
+                       "shared physical network device between "
+                       "SR-IOV and OVS or SR-IOV and linux bridge"))
+]
+cfg.CONF.register_opts(fdb_population_opt, 'FDB')
+
+LOG = logging.getLogger(__name__)
+
+
+class FdbPopulationAgentExtension(agent_extension.AgentCoreResourceExtension):
+    """The FDB population is an agent extension to OVS or linux bridge
+    who's objective is to update the FDB table for existing instance
+    using normal port, thus enabling communication between SR-IOV instances
+    and normal instances.
+    Additional information describing the problem can be found here:
+    http://events.linuxfoundation.org/sites/events/files/slides/LinuxConJapan2014_makita_0.pdf
+    """
+
+    # FDB udpates are triggered for ports with a certain device_owner only:
+    # - device owner "compute": updates the FDB with normal port instances,
+    #       required in order to enable communication between
+    #       SR-IOV direct port instances and normal port instance.
+    # - device owner "router_interface": updates the FDB woth OVS/LB ports,
+    #       required in order to enable communication for SR-IOV instances
+    #       with floating ip that are located with the network node.
+    # - device owner "DHCP": not required because messages are broadcast.
+    PERMITTED_DEVICE_OWNERS = {'compute', 'network:router_interface'}
+
+    class FdbTableTracker(object):
+        """FDB table tracker is a helper class
+        intended to keep track of the existing FDB rules.
+        """
+        def __init__(self, devices):
+            self.device_to_macs = {}
+            self.portid_to_mac = {}
+            # update macs already in the physical interface's FDB table
+            for device in devices:
+                try:
+                    _stdout = bridge_lib.FdbInterface.show(device)
+                except RuntimeError as e:
+                    LOG.warning(_LW(
+                        'Unable to find FDB Interface %(device)s. '
+                        'Exception: %(e)s'), {'device': device, 'e': e})
+                    continue
+                self.device_to_macs[device] = _stdout.split()[::3]
+
+        def update_port(self, device, port_id, mac):
+            # check if port id is updated
+            if self.portid_to_mac.get(port_id) == mac:
+                return
+            # delete invalid port_id's mac from the FDB,
+            # in case the port was updated to another mac
+            self.delete_port([device], port_id)
+            # update port id
+            self.portid_to_mac[port_id] = mac
+            # check if rule for mac already exists
+            if mac in self.device_to_macs[device]:
+                return
+            try:
+                bridge_lib.FdbInterface.add(mac, device)
+            except RuntimeError as e:
+                LOG.warning(_LW(
+                    'Unable to add mac %(mac)s '
+                    'to FDB Interface %(device)s. '
+                    'Exception: %(e)s'),
+                    {'mac': mac, 'device': device, 'e': e})
+                return
+            self.device_to_macs[device].append(mac)
+
+        def delete_port(self, devices, port_id):
+            mac = self.portid_to_mac.get(port_id)
+            if mac is None:
+                LOG.warning(_LW('Port Id %(port_id)s does not have a rule for '
+                    'devices %(devices)s in FDB table'),
+                    {'port_id': port_id, 'devices': devices})
+                return
+            for device in devices:
+                if mac in self.device_to_macs[device]:
+                    try:
+                        bridge_lib.FdbInterface.delete(mac, device)
+                    except RuntimeError as e:
+                        LOG.warning(_LW(
+                            'Unable to delete mac %(mac)s '
+                            'from FDB Interface %(device)s. '
+                            'Exception: %(e)s'),
+                            {'mac': mac, 'device': device, 'e': e})
+                        return
+                    self.device_to_macs[device].remove(mac)
+                    del self.portid_to_mac[port_id]
+
+    # class FdbPopulationAgentExtension implementation:
+    def initialize(self, connection, driver_type):
+        """Perform FDB Agent Extension initialization."""
+        valid_driver_types = (linux_bridge_constants.EXTENSION_DRIVER_TYPE,
+                              ovs_constants.EXTENSION_DRIVER_TYPE)
+        if driver_type not in valid_driver_types:
+            LOG.error(_LE('FDB extension is only supported for OVS and '
+                          'linux bridge agent, currently uses '
+                          '%(driver_type)s'), {'driver_type': driver_type})
+            sys.exit(1)
+
+        self.device_mappings = n_utils.parse_mappings(
+            cfg.CONF.FDB.shared_physical_device_mappings, unique_keys=False)
+        devices = self._get_devices()
+        if not devices:
+            LOG.error(_LE('Invalid configuration provided for FDB extension: '
+                          'no physical devices'))
+            sys.exit(1)
+        self.fdb_tracker = self.FdbTableTracker(devices)
+
+    def handle_port(self, context, details):
+        """Handle agent FDB population extension for port."""
+        device_owner = details['device_owner']
+        if self._is_valid_device_owner(device_owner):
+            mac = details['mac_address']
+            port_id = details['port_id']
+            physnet = details.get('physical_network')
+            if physnet and physnet in self.device_mappings:
+                for device in self.device_mappings[physnet]:
+                    self.fdb_tracker.update_port(device, port_id, mac)
+
+    def delete_port(self, context, details):
+        """Delete port from FDB population extension."""
+        port_id = details['port_id']
+        devices = self._get_devices()
+        self.fdb_tracker.delete_port(devices, port_id)
+
+    def _get_devices(self):
+        def _flatten_list(l):
+            return [item for sublist in l for item in sublist]
+
+        return _flatten_list(self.device_mappings.values())
+
+    def _is_valid_device_owner(self, device_owner):
+        for permitted_device_owner in self.PERMITTED_DEVICE_OWNERS:
+            if device_owner.startswith(permitted_device_owner):
+                return True
+        return False

neutron/agent/linux/bridge_lib.py

@@ -21,6 +21,7 @@ import os
 from oslo_log import log as logging
 
 from neutron.agent.linux import ip_lib
+from neutron.agent.linux import utils
 
 LOG = logging.getLogger(__name__)
 
@@ -98,3 +99,24 @@ class BridgeDevice(ip_lib.IPDevice):
             return os.listdir(BRIDGE_INTERFACES_FS % self.name)
         except OSError:
             return []
+
+
+class FdbInterface(object):
+    """provide basic functionality to edit the FDB table"""
+
+    @classmethod
+    def add(cls, mac, dev):
+        return utils.execute(['bridge', 'fdb', 'add', mac, 'dev', dev],
+                run_as_root=True)
+
+    @classmethod
+    def delete(cls, mac, dev):
+        return utils.execute(['bridge', 'fdb', 'delete', mac, 'dev', dev],
+                             run_as_root=True)
+
+    @classmethod
+    def show(cls, dev=None):
+        cmd = ['bridge', 'fdb', 'show']
+        if dev:
+            cmd += ['dev', dev]
+        return utils.execute(cmd, run_as_root=True)

neutron/tests/unit/agent/l2/extensions/test_fdb_population.py

@@ -0,0 +1,176 @@
+# Copyright (c) 2016 Mellanox Technologies, Ltd
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import copy
+
+import mock
+from oslo_config import cfg
+import six
+
+from neutron.agent.l2.extensions.fdb_population import (
+        FdbPopulationAgentExtension)
+from neutron.common import utils as n_utils
+from neutron.plugins.ml2.drivers.linuxbridge.agent.common import (
+     constants as linux_bridge_constants)
+from neutron.plugins.ml2.drivers.openvswitch.agent.common import (
+     constants as ovs_constants)
+from neutron.tests import base
+
+
+class FdbPopulationExtensionTestCase(base.BaseTestCase):
+
+    UPDATE_MSG = {u'device_owner': u'network:router_interface',
+                  u'physical_network': u'physnet1',
+                  u'mac_address': u'fa:16:3e:ba:bc:21',
+                  u'port_id': u'17ceda02-43e1-48d8-beb6-35885b20cae6'}
+    DELETE_MSG = {u'port_id': u'17ceda02-43e1-48d8-beb6-35885b20cae6'}
+    FDB_TABLE = ("aa:aa:aa:aa:aa:aa self permanent\n"
+                 "bb:bb:bb:bb:bb:bb self permanent")
+
+    def setUp(self):
+        super(FdbPopulationExtensionTestCase, self).setUp()
+        cfg.CONF.set_override('shared_physical_device_mappings',
+                              ['physnet1:p1p1'], 'FDB')
+        self.DEVICE = self._get_existing_device()
+
+    def _get_existing_device(self):
+        device_mappings = n_utils.parse_mappings(
+            cfg.CONF.FDB.shared_physical_device_mappings, unique_keys=False)
+        DEVICES = six.next(six.itervalues(device_mappings))
+        return DEVICES[0]
+
+    def _get_fdb_extension(self, mock_execute, fdb_table):
+        mock_execute.return_value = fdb_table
+        fdb_pop = FdbPopulationAgentExtension()
+        fdb_pop.initialize(None, ovs_constants.EXTENSION_DRIVER_TYPE)
+        return fdb_pop
+
+    @mock.patch('neutron.agent.linux.utils.execute')
+    def test_initialize(self, mock_execute):
+        fdb_extension = FdbPopulationAgentExtension()
+        fdb_extension.initialize(None, ovs_constants.EXTENSION_DRIVER_TYPE)
+        fdb_extension.initialize(None,
+                                 linux_bridge_constants.EXTENSION_DRIVER_TYPE)
+
+    @mock.patch('neutron.agent.linux.utils.execute')
+    def test_initialize_invalid_agent(self, mock_execute):
+        fdb_extension = FdbPopulationAgentExtension()
+        self.assertRaises(SystemExit, fdb_extension.initialize, None, 'sriov')
+
+    @mock.patch('neutron.agent.linux.utils.execute')
+    def test_construct_empty_fdb_table(self, mock_execute):
+        self._get_fdb_extension(mock_execute, fdb_table='')
+        cmd = ['bridge', 'fdb', 'show', 'dev', self.DEVICE]
+        mock_execute.assert_called_once_with(cmd, run_as_root=True)
+
+    @mock.patch('neutron.agent.linux.utils.execute')
+    def test_construct_existing_fdb_table(self, mock_execute):
+        fdb_extension = self._get_fdb_extension(mock_execute,
+                                                fdb_table=self.FDB_TABLE)
+        cmd = ['bridge', 'fdb', 'show', 'dev', self.DEVICE]
+        mock_execute.assert_called_once_with(cmd, run_as_root=True)
+        updated_macs_for_device = (
+            fdb_extension.fdb_tracker.device_to_macs.get(self.DEVICE))
+        macs = [line.split()[0] for line in self.FDB_TABLE.split('\n')]
+        for mac in macs:
+            self.assertIn(mac, updated_macs_for_device)
+
+    @mock.patch('neutron.agent.linux.utils.execute')
+    def test_update_port_add_rule(self, mock_execute):
+        fdb_extension = self._get_fdb_extension(mock_execute, self.FDB_TABLE)
+        mock_execute.reset_mock()
+        fdb_extension.handle_port(context=None, details=self.UPDATE_MSG)
+        cmd = ['bridge', 'fdb', 'add', self.UPDATE_MSG['mac_address'],
+               'dev', self.DEVICE]
+        mock_execute.assert_called_once_with(cmd, run_as_root=True)
+        updated_macs_for_device = (
+            fdb_extension.fdb_tracker.device_to_macs.get(self.DEVICE))
+        mac = self.UPDATE_MSG['mac_address']
+        self.assertIn(mac, updated_macs_for_device)
+
+    @mock.patch('neutron.agent.linux.utils.execute')
+    def test_update_port_changed_mac(self, mock_execute):
+        fdb_extension = self._get_fdb_extension(mock_execute, self.FDB_TABLE)
+        mock_execute.reset_mock()
+        mac = self.UPDATE_MSG['mac_address']
+        updated_mac = 'fa:16:3e:ba:bc:33'
+        commands = []
+        fdb_extension.handle_port(context=None, details=self.UPDATE_MSG)
+        commands.append(['bridge', 'fdb', 'add', mac, 'dev', self.DEVICE])
+        self.UPDATE_MSG['mac_address'] = updated_mac
+        fdb_extension.handle_port(context=None, details=self.UPDATE_MSG)
+        commands.append(['bridge', 'fdb', 'delete', mac, 'dev', self.DEVICE])
+        commands.append(['bridge', 'fdb', 'add', updated_mac,
+                         'dev', self.DEVICE])
+        calls = []
+        for cmd in commands:
+            calls.append(mock.call(cmd, run_as_root=True))
+        mock_execute.assert_has_calls(calls)
+        updated_macs_for_device = (
+            fdb_extension.fdb_tracker.device_to_macs.get(self.DEVICE))
+        self.assertIn(updated_mac, updated_macs_for_device)
+        self.assertNotIn(mac, updated_macs_for_device)
+
+    @mock.patch('neutron.agent.linux.utils.execute')
+    def test_unpermitted_device_owner(self, mock_execute):
+        fdb_extension = self._get_fdb_extension(mock_execute, '')
+        mock_execute.reset_mock()
+        details = copy.deepcopy(self.UPDATE_MSG)
+        details['device_owner'] = 'network:dhcp'
+        fdb_extension.handle_port(context=None, details=details)
+        self.assertFalse(mock_execute.called)
+        updated_macs_for_device = (
+            fdb_extension.fdb_tracker.device_to_macs.get(self.DEVICE))
+        mac = self.UPDATE_MSG['mac_address']
+        self.assertNotIn(mac, updated_macs_for_device)
+
+    @mock.patch('neutron.agent.linux.utils.execute')
+    def test_catch_init_exception(self, mock_execute):
+        mock_execute.side_effect = RuntimeError
+        fdb_extension = self._get_fdb_extension(mock_execute, '')
+        updated_macs_for_device = (
+            fdb_extension.fdb_tracker.device_to_macs.get(self.DEVICE))
+        self.assertIsNone(updated_macs_for_device)
+
+    @mock.patch('neutron.agent.linux.utils.execute')
+    def test_catch_update_port_exception(self, mock_execute):
+        fdb_extension = self._get_fdb_extension(mock_execute, '')
+        mock_execute.side_effect = RuntimeError
+        fdb_extension.handle_port(context=None, details=self.UPDATE_MSG)
+        updated_macs_for_device = (
+            fdb_extension.fdb_tracker.device_to_macs.get(self.DEVICE))
+        mac = self.UPDATE_MSG['mac_address']
+        self.assertNotIn(mac, updated_macs_for_device)
+
+    @mock.patch('neutron.agent.linux.utils.execute')
+    def test_catch_delete_port_exception(self, mock_execute):
+        fdb_extension = self._get_fdb_extension(mock_execute, '')
+        fdb_extension.handle_port(context=None, details=self.UPDATE_MSG)
+        mock_execute.side_effect = RuntimeError
+        fdb_extension.delete_port(context=None, details=self.DELETE_MSG)
+        updated_macs_for_device = (
+            fdb_extension.fdb_tracker.device_to_macs.get(self.DEVICE))
+        mac = self.UPDATE_MSG['mac_address']
+        self.assertIn(mac, updated_macs_for_device)
+
+    @mock.patch('neutron.agent.linux.utils.execute')
+    def test_delete_port(self, mock_execute):
+        fdb_extension = self._get_fdb_extension(mock_execute, '')
+        fdb_extension.handle_port(context=None, details=self.UPDATE_MSG)
+        mock_execute.reset_mock()
+        fdb_extension.delete_port(context=None, details=self.DELETE_MSG)
+        cmd = ['bridge', 'fdb', 'delete', self.UPDATE_MSG['mac_address'],
+               'dev', self.DEVICE]
+        mock_execute.assert_called_once_with(cmd, run_as_root=True)

releasenotes/notes/fdb_population-70d751c8c2e4395f.yaml

@@ -0,0 +1,17 @@
+---
+fixes:
+  - In order to fix the communication issues between
+    SR-IOV instances and regular instances
+    the FDB population extension is added to the
+    OVS or linuxbridge agent.
+    the cause was that messages from SR-IOV direct port
+    instance to normal port instances located on
+    the same hypervisor were sent directly to the wire
+    because the FDB table was not yet updated.
+    FDB population extension tracks instances
+    boot/delete operations using the handle_port
+    delete_port extension interface messages
+    and update the hypervisor's FDB table accordingly.
+
+    Please note this L2 agent extension doesn't support
+    allowed address pairs extension.

setup.cfg

@@ -111,6 +111,7 @@ neutron.ipam_drivers =
     internal = neutron.ipam.drivers.neutrondb_ipam.driver:NeutronDbPool
 neutron.agent.l2.extensions =
     qos = neutron.agent.l2.extensions.qos:QosAgentExtension
+    fdb = neutron.agent.l2.extensions.fdb_population:FdbPopulationAgentExtension
 neutron.qos.agent_drivers =
     ovs = neutron.plugins.ml2.drivers.openvswitch.agent.extension_drivers.qos_driver:QosOVSAgentDriver
     sriov = neutron.plugins.ml2.drivers.mech_sriov.agent.extension_drivers.qos_driver:QosSRIOVAgentDriver