diff --git a/neutron/agent/linux/ovs_lib.py b/neutron/agent/linux/ovs_lib.py index 78967d1f3a3..c8421a2124a 100644 --- a/neutron/agent/linux/ovs_lib.py +++ b/neutron/agent/linux/ovs_lib.py @@ -18,7 +18,6 @@ from oslo.config import cfg from neutron.agent.linux import ip_lib from neutron.agent.linux import utils from neutron.common import exceptions -from neutron.common import utils as common_utils from neutron.openstack.common import excutils from neutron.openstack.common import jsonutils from neutron.openstack.common import log as logging @@ -551,26 +550,3 @@ def _build_flow_expr_str(flow_dict, cmd): flow_expr_arr.append(actions) return ','.join(flow_expr_arr) - - -def ofctl_arg_supported(root_helper, cmd, args): - '''Verify if ovs-ofctl binary supports command with specific args. - - :param root_helper: utility to use when running shell cmds. - :param cmd: ovs-vsctl command to use for test. - :param args: arguments to test with command. - :returns: a boolean if the args supported. - ''' - supported = True - br_name = 'br-test-%s' % common_utils.get_random_string(6) - test_br = OVSBridge(br_name, root_helper) - test_br.reset_bridge() - - full_args = ["ovs-ofctl", cmd, test_br.br_name] + args - try: - utils.execute(full_args, root_helper=root_helper) - except Exception: - supported = False - - test_br.destroy() - return supported diff --git a/neutron/cmd/sanity/checks.py b/neutron/cmd/sanity/checks.py index 60081573e0d..68cf32f226e 100644 --- a/neutron/cmd/sanity/checks.py +++ b/neutron/cmd/sanity/checks.py @@ -13,11 +13,17 @@ # License for the specific language governing permissions and limitations # under the License. +import netaddr + from neutron.agent.linux import ovs_lib +from neutron.agent.linux import utils as agent_utils from neutron.common import utils +from neutron.openstack.common import log as logging from neutron.plugins.common import constants as const from neutron.plugins.openvswitch.common import constants as ovs_const +LOG = logging.getLogger(__name__) + def vxlan_supported(root_helper, from_ip='192.0.2.1', to_ip='192.0.2.2'): name = "vxlantest-" + utils.get_random_string(6) @@ -42,3 +48,44 @@ def nova_notify_supported(): return True except ImportError: return False + + +def ofctl_arg_supported(root_helper, cmd, **kwargs): + """Verify if ovs-ofctl binary supports cmd with **kwargs. + + :param root_helper: utility to use when running shell commands. + :param cmd: ovs-ofctl command to use for test. + :param **kwargs: arguments to test with the command. + :returns: a boolean if the supplied arguments are supported. + """ + br_name = 'br-test-%s' % utils.get_random_string(6) + with ovs_lib.OVSBridge(br_name, root_helper) as test_br: + full_args = ["ovs-ofctl", cmd, test_br.br_name, + ovs_lib._build_flow_expr_str(kwargs, cmd.split('-')[0])] + try: + agent_utils.execute(full_args, root_helper=root_helper) + except RuntimeError as e: + LOG.debug("Exception while checking supported feature via " + "command %s. Exception: %s" % (full_args, e)) + return False + except Exception: + LOG.exception(_("Unexpected exception while checking supported" + " feature via command: %s") % full_args) + return False + else: + return True + + +def arp_responder_supported(root_helper): + mac = netaddr.EUI('dead:1234:beef', dialect=netaddr.mac_unix) + ip = netaddr.IPAddress('240.0.0.1') + actions = ovs_const.ARP_RESPONDER_ACTIONS % {'mac': mac, 'ip': ip} + + return ofctl_arg_supported(root_helper, + cmd='add-flow', + table=21, + priority=1, + proto='arp', + dl_vlan=42, + nw_dst='%s' % ip, + actions=actions) diff --git a/neutron/cmd/sanity_check.py b/neutron/cmd/sanity_check.py index 16e65f80be1..b6522d6d79e 100644 --- a/neutron/cmd/sanity_check.py +++ b/neutron/cmd/sanity_check.py @@ -61,6 +61,16 @@ def check_nova_notify(): return result +def check_arp_responder(): + result = checks.arp_responder_supported( + root_helper=cfg.CONF.AGENT.root_helper) + if not result: + LOG.error(_('Check for Open vSwitch ARP responder support failed. ' + 'Please ensure that the version of openvswitch ' + 'being used has ARP flows support.')) + return result + + # Define CLI opts to test specific features, with a calback for the test OPTS = [ BoolOptCallback('ovs_vxlan', check_ovs_vxlan, default=False, @@ -69,6 +79,8 @@ OPTS = [ help=_('Check for patch port support')), BoolOptCallback('nova_notify', check_nova_notify, default=False, help=_('Check for nova notification support')), + BoolOptCallback('arp_responder', check_arp_responder, default=False, + help=_('Check for ARP responder support')), ] @@ -87,6 +99,8 @@ def enable_tests_from_config(): if (cfg.CONF.notify_nova_on_port_status_changes or cfg.CONF.notify_nova_on_port_data_changes): cfg.CONF.set_override('nova_notify', True) + if cfg.CONF.AGENT.arp_responder: + cfg.CONF.set_override('arp_responder', True) def all_tests_passed(): diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py index 956ef708238..d2eec998a62 100644 --- a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py @@ -172,12 +172,10 @@ class OVSNeutronAgent(n_rpc.RpcCallback, q_const.MAX_VLAN_TAG)) self.tunnel_types = tunnel_types or [] self.l2_pop = l2_population - # TODO(ethuleau): Initially, local ARP responder is be dependent to the + # TODO(ethuleau): Change ARP responder so it's not dependent on the # ML2 l2 population mechanism driver. - self.arp_responder_enabled = (arp_responder and - self._check_arp_responder_support() and - self.l2_pop) self.enable_distributed_routing = enable_distributed_routing + self.arp_responder_enabled = arp_responder and self.l2_pop self.agent_state = { 'binary': 'neutron-openvswitch-agent', 'host': cfg.CONF.host, @@ -251,20 +249,6 @@ class OVSNeutronAgent(n_rpc.RpcCallback, self.iter_num = 0 self.run_daemon_loop = True - def _check_arp_responder_support(self): - '''Check if OVS supports to modify ARP headers. - - This functionality is only available since the development branch 2.1. - ''' - args = ['arp,action=load:0x2->NXM_OF_ARP_OP[],' - 'move:NXM_NX_ARP_SHA[]->NXM_NX_ARP_THA[],' - 'move:NXM_OF_ARP_SPA[]->NXM_OF_ARP_TPA[]'] - supported = ovs_lib.ofctl_arg_supported(self.root_helper, 'add-flow', - args) - if not supported: - LOG.warning(_('OVS version can not support ARP responder.')) - return supported - def _report_state(self): # How many devices are likely used by a VM self.agent_state.get('configurations')['devices'] = ( @@ -477,14 +461,7 @@ class OVSNeutronAgent(n_rpc.RpcCallback, ip = netaddr.IPAddress(ip_str) if action == 'add': - actions = ('move:NXM_OF_ETH_SRC[]->NXM_OF_ETH_DST[],' - 'mod_dl_src:%(mac)s,' - 'load:0x2->NXM_OF_ARP_OP[],' - 'move:NXM_NX_ARP_SHA[]->NXM_NX_ARP_THA[],' - 'move:NXM_OF_ARP_SPA[]->NXM_OF_ARP_TPA[],' - 'load:%(mac)#x->NXM_NX_ARP_SHA[],' - 'load:%(ip)#x->NXM_OF_ARP_SPA[],' - 'in_port' % {'mac': mac, 'ip': ip}) + actions = constants.ARP_RESPONDER_ACTIONS % {'mac': mac, 'ip': ip} self.tun_br.add_flow(table=constants.ARP_RESPONDER, priority=1, proto='arp', diff --git a/neutron/plugins/openvswitch/common/constants.py b/neutron/plugins/openvswitch/common/constants.py index b74242b0b06..5fe702484dd 100644 --- a/neutron/plugins/openvswitch/common/constants.py +++ b/neutron/plugins/openvswitch/common/constants.py @@ -68,3 +68,12 @@ INVALID_OFPORT = '-1' # Represent invalid OF Port OFPORT_INVALID = -1 + +ARP_RESPONDER_ACTIONS = ('move:NXM_OF_ETH_SRC[]->NXM_OF_ETH_DST[],' + 'mod_dl_src:%(mac)s,' + 'load:0x2->NXM_OF_ARP_OP[],' + 'move:NXM_NX_ARP_SHA[]->NXM_NX_ARP_THA[],' + 'move:NXM_OF_ARP_SPA[]->NXM_OF_ARP_TPA[],' + 'load:%(mac)#x->NXM_NX_ARP_SHA[],' + 'load:%(ip)#x->NXM_OF_ARP_SPA[],' + 'in_port') diff --git a/neutron/tests/functional/sanity/test_sanity.py b/neutron/tests/functional/sanity/test_sanity.py index 2873dc7a586..bbe8911e534 100644 --- a/neutron/tests/functional/sanity/test_sanity.py +++ b/neutron/tests/functional/sanity/test_sanity.py @@ -49,3 +49,6 @@ class SanityTestCaseRoot(functional_base.BaseSudoTestCase): def test_ovs_patch_support_runs(self): checks.patch_supported(self.root_helper) + + def test_arp_responder_runs(self): + checks.arp_responder_supported(self.root_helper) diff --git a/neutron/tests/unit/agent/linux/test_ovs_lib.py b/neutron/tests/unit/agent/linux/test_ovs_lib.py index e60e62cbdbc..24ae10c3f0f 100644 --- a/neutron/tests/unit/agent/linux/test_ovs_lib.py +++ b/neutron/tests/unit/agent/linux/test_ovs_lib.py @@ -932,35 +932,3 @@ class OVS_Lib_Test(base.BaseTestCase): data = [[["map", external_ids], "tap99", 1]] self.assertIsNone(self._test_get_vif_port_by_id('tap99id', data, "br-ext")) - - def test_ofctl_arg_supported(self): - with mock.patch('neutron.common.utils.get_random_string') as utils: - utils.return_value = 'test' - supported = ovs_lib.ofctl_arg_supported(self.root_helper, 'cmd', - ['args']) - self.execute.assert_has_calls([ - mock.call(['ovs-vsctl', self.TO, '--', '--if-exists', 'del-br', - 'br-test-test'], root_helper=self.root_helper), - mock.call(['ovs-vsctl', self.TO, '--', '--may-exist', 'add-br', - 'br-test-test'], root_helper=self.root_helper), - mock.call(['ovs-ofctl', 'cmd', 'br-test-test', 'args'], - root_helper=self.root_helper), - mock.call(['ovs-vsctl', self.TO, '--', '--if-exists', 'del-br', - 'br-test-test'], root_helper=self.root_helper) - ]) - self.assertTrue(supported) - - self.execute.side_effect = Exception - supported = ovs_lib.ofctl_arg_supported(self.root_helper, 'cmd', - ['args']) - self.execute.assert_has_calls([ - mock.call(['ovs-vsctl', self.TO, '--', '--if-exists', 'del-br', - 'br-test-test'], root_helper=self.root_helper), - mock.call(['ovs-vsctl', self.TO, '--', '--may-exist', 'add-br', - 'br-test-test'], root_helper=self.root_helper), - mock.call(['ovs-ofctl', 'cmd', 'br-test-test', 'args'], - root_helper=self.root_helper), - mock.call(['ovs-vsctl', self.TO, '--', '--if-exists', 'del-br', - 'br-test-test'], root_helper=self.root_helper) - ]) - self.assertFalse(supported) diff --git a/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py b/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py index 5aa43f0e256..084bd020e99 100644 --- a/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py @@ -137,10 +137,7 @@ class TestOvsNeutronAgent(base.BaseTestCase): 'get_bridges'), mock.patch('neutron.openstack.common.loopingcall.' 'FixedIntervalLoopingCall', - new=MockFixedIntervalLoopingCall), - mock.patch('neutron.plugins.openvswitch.agent.ovs_neutron_agent.' - 'OVSNeutronAgent._check_arp_responder_support', - return_value=True)): + new=MockFixedIntervalLoopingCall)): self.agent = ovs_neutron_agent.OVSNeutronAgent(**kwargs) self.agent.tun_br = mock.Mock() self.agent.sg_agent = mock.Mock() @@ -1022,14 +1019,7 @@ class TestOvsNeutronAgent(base.BaseTestCase): ) as (add_flow_fn, mod_flow_fn, add_tun_fn): self.agent.fdb_add(None, fdb_entry) self.assertFalse(add_tun_fn.called) - actions = ('move:NXM_OF_ETH_SRC[]->NXM_OF_ETH_DST[],' - 'mod_dl_src:%(mac)s,' - 'load:0x2->NXM_OF_ARP_OP[],' - 'move:NXM_NX_ARP_SHA[]->NXM_NX_ARP_THA[],' - 'move:NXM_OF_ARP_SPA[]->NXM_OF_ARP_TPA[],' - 'load:%(mac)#x->NXM_NX_ARP_SHA[],' - 'load:%(ip)#x->NXM_OF_ARP_SPA[],' - 'in_port' % + actions = (constants.ARP_RESPONDER_ACTIONS % {'mac': netaddr.EUI(FAKE_MAC, dialect=netaddr.mac_unix), 'ip': netaddr.IPAddress(FAKE_IP1)}) add_flow_fn.assert_has_calls([ @@ -1121,14 +1111,7 @@ class TestOvsNeutronAgent(base.BaseTestCase): mock.patch.object(self.agent.tun_br, 'delete_flows') ) as (add_flow_fn, del_flow_fn): self.agent.fdb_update(None, fdb_entries) - actions = ('move:NXM_OF_ETH_SRC[]->NXM_OF_ETH_DST[],' - 'mod_dl_src:%(mac)s,' - 'load:0x2->NXM_OF_ARP_OP[],' - 'move:NXM_NX_ARP_SHA[]->NXM_NX_ARP_THA[],' - 'move:NXM_OF_ARP_SPA[]->NXM_OF_ARP_TPA[],' - 'load:%(mac)#x->NXM_NX_ARP_SHA[],' - 'load:%(ip)#x->NXM_OF_ARP_SPA[],' - 'in_port' % + actions = (constants.ARP_RESPONDER_ACTIONS % {'mac': netaddr.EUI(FAKE_MAC, dialect=netaddr.mac_unix), 'ip': netaddr.IPAddress(FAKE_IP2)}) add_flow_fn.assert_called_once_with(table=constants.ARP_RESPONDER, @@ -1400,10 +1383,7 @@ class AncillaryBridgesTest(base.BaseTestCase): return_value=bridges), mock.patch( 'neutron.agent.linux.ovs_lib.get_bridge_external_bridge_id', - side_effect=pullup_side_effect), - mock.patch('neutron.plugins.openvswitch.agent.ovs_neutron_agent.' - 'OVSNeutronAgent._check_arp_responder_support', - return_value=True)): + side_effect=pullup_side_effect)): self.agent = ovs_neutron_agent.OVSNeutronAgent(**self.kwargs) self.assertEqual(len(ancillary), len(self.agent.ancillary_brs)) if ancillary: diff --git a/neutron/tests/unit/openvswitch/test_ovs_tunnel.py b/neutron/tests/unit/openvswitch/test_ovs_tunnel.py index 095f761c426..8ddfa64be77 100644 --- a/neutron/tests/unit/openvswitch/test_ovs_tunnel.py +++ b/neutron/tests/unit/openvswitch/test_ovs_tunnel.py @@ -78,12 +78,6 @@ class TunnelTest(base.BaseTestCase): 'neutron.openstack.common.rpc.impl_fake') cfg.CONF.set_override('report_interval', 0, 'AGENT') - check_arp_responder_str = ('neutron.plugins.openvswitch.agent.' - 'ovs_neutron_agent.OVSNeutronAgent.' - '_check_arp_responder_support') - self.mock_check_arp_resp = mock.patch(check_arp_responder_str).start() - self.mock_check_arp_resp.return_value = True - self.INT_BRIDGE = 'integration_bridge' self.TUN_BRIDGE = 'tunnel_bridge' self.MAP_TUN_BRIDGE = 'tun_br_map'