From 2c8f61b816bf531a17a7b45d35a5388e8a2f607a Mon Sep 17 00:00:00 2001 From: Edan David Date: Tue, 24 May 2016 11:54:02 -0400 Subject: [PATCH] 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 | 172 +++++++++++++++++ neutron/agent/linux/bridge_lib.py | 22 +++ .../l2/extensions/test_fdb_population.py | 176 ++++++++++++++++++ .../fdb_population-70d751c8c2e4395f.yaml | 17 ++ setup.cfg | 1 + 5 files changed, 388 insertions(+) create mode 100644 neutron/agent/l2/extensions/fdb_population.py create mode 100644 neutron/tests/unit/agent/l2/extensions/test_fdb_population.py create mode 100644 releasenotes/notes/fdb_population-70d751c8c2e4395f.yaml diff --git a/neutron/agent/l2/extensions/fdb_population.py b/neutron/agent/l2/extensions/fdb_population.py new file mode 100644 index 00000000000..553f06e2bb7 --- /dev/null +++ b/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 " + ": 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 diff --git a/neutron/agent/linux/bridge_lib.py b/neutron/agent/linux/bridge_lib.py index a4f231d6fd2..731643ea8bf 100644 --- a/neutron/agent/linux/bridge_lib.py +++ b/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) diff --git a/neutron/tests/unit/agent/l2/extensions/test_fdb_population.py b/neutron/tests/unit/agent/l2/extensions/test_fdb_population.py new file mode 100644 index 00000000000..3a7f4012ef3 --- /dev/null +++ b/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) diff --git a/releasenotes/notes/fdb_population-70d751c8c2e4395f.yaml b/releasenotes/notes/fdb_population-70d751c8c2e4395f.yaml new file mode 100644 index 00000000000..1fad4fc917a --- /dev/null +++ b/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. diff --git a/setup.cfg b/setup.cfg index 21676c1f92c..6512517ca5a 100644 --- a/setup.cfg +++ b/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