From 01a97d926c9712a5d374f500884bc45b3634b8c1 Mon Sep 17 00:00:00 2001 From: Kevin Benton Date: Sun, 25 Jun 2017 00:53:26 -0700 Subject: [PATCH] Remove deprecated prevent_arp_spoofing option This was deprecated over a year ago in [1] so let's get rid of it to clean up some code. 1. Ib63ba8ae7050465a0786ea3d50c65f413f4ebe38 Change-Id: I6039fb7e743c5d9a1a313e3c174ada36c9874c70 --- neutron/cmd/sanity_check.py | 5 ++--- neutron/conf/plugins/ml2/drivers/agent.py | 19 ------------------- neutron/conf/plugins/ml2/drivers/ovs_conf.py | 15 --------------- .../ml2/drivers/agent/_common_agent.py | 13 ++++--------- .../openvswitch/agent/ovs_neutron_agent.py | 1 - neutron/tests/functional/agent/l2/base.py | 1 - .../ml2/drivers/agent/test__common_agent.py | 10 +++------- .../agent/test_ovs_neutron_agent.py | 3 ++- ...prevent_arp_spoofing-b49e91a92a93e3e1.yaml | 6 ++++++ 9 files changed, 17 insertions(+), 56 deletions(-) create mode 100644 releasenotes/notes/removed_prevent_arp_spoofing-b49e91a92a93e3e1.yaml diff --git a/neutron/cmd/sanity_check.py b/neutron/cmd/sanity_check.py index f9c32d03d0b..0e1997ec43e 100644 --- a/neutron/cmd/sanity_check.py +++ b/neutron/cmd/sanity_check.py @@ -345,6 +345,8 @@ def enable_tests_from_config(): """ cfg.CONF.set_default('vf_management', True) + cfg.CONF.set_default('arp_header_match', True) + cfg.CONF.set_default('icmpv6_header_match', True) if 'vxlan' in cfg.CONF.AGENT.tunnel_types: cfg.CONF.set_default('ovs_vxlan', True) if 'geneve' in cfg.CONF.AGENT.tunnel_types: @@ -361,9 +363,6 @@ def enable_tests_from_config(): cfg.CONF.set_default('nova_notify', True) if cfg.CONF.AGENT.arp_responder: cfg.CONF.set_default('arp_responder', True) - if cfg.CONF.AGENT.prevent_arp_spoofing: - cfg.CONF.set_default('arp_header_match', True) - cfg.CONF.set_default('icmpv6_header_match', True) if not cfg.CONF.AGENT.use_helper_for_ns_read: cfg.CONF.set_default('read_netns', True) if cfg.CONF.OVS.ovsdb_interface == 'native': diff --git a/neutron/conf/plugins/ml2/drivers/agent.py b/neutron/conf/plugins/ml2/drivers/agent.py index 052ada7363a..16347d99146 100644 --- a/neutron/conf/plugins/ml2/drivers/agent.py +++ b/neutron/conf/plugins/ml2/drivers/agent.py @@ -26,25 +26,6 @@ agent_opts = [ help=_("Set new timeout in seconds for new rpc calls after " "agent receives SIGTERM. If value is set to 0, rpc " "timeout won't be changed")), - # TODO(kevinbenton): The following opt is duplicated between the OVS agent - # and the Linuxbridge agent to make it easy to back-port. These shared opts - # should be moved into a common agent config options location as part of - # the deduplication work. - cfg.BoolOpt('prevent_arp_spoofing', default=True, - deprecated_for_removal=True, - help=_("Enable suppression of ARP responses that don't match " - "an IP address that belongs to the port from which " - "they originate. Note: This prevents the VMs attached " - "to this agent from spoofing, it doesn't protect them " - "from other devices which have the capability to spoof " - "(e.g. bare metal or VMs attached to agents without " - "this flag set to True). Spoofing rules will not be " - "added to any ports that have port security disabled. " - "For LinuxBridge, this requires ebtables. For OVS, it " - "requires a version that supports matching ARP " - "headers. This option will be removed in Ocata so " - "the only way to disable protection will be via the " - "port security extension.")) ] diff --git a/neutron/conf/plugins/ml2/drivers/ovs_conf.py b/neutron/conf/plugins/ml2/drivers/ovs_conf.py index 1267da16a0f..5ef6c1248b3 100644 --- a/neutron/conf/plugins/ml2/drivers/ovs_conf.py +++ b/neutron/conf/plugins/ml2/drivers/ovs_conf.py @@ -124,21 +124,6 @@ agent_opts = [ "Allows the switch (when supporting an overlay) " "to respond to an ARP request locally without " "performing a costly ARP broadcast into the overlay.")), - cfg.BoolOpt('prevent_arp_spoofing', default=True, - deprecated_for_removal=True, - help=_("Enable suppression of ARP responses that don't match " - "an IP address that belongs to the port from which " - "they originate. Note: This prevents the VMs attached " - "to this agent from spoofing, it doesn't protect them " - "from other devices which have the capability to spoof " - "(e.g. bare metal or VMs attached to agents without " - "this flag set to True). Spoofing rules will not be " - "added to any ports that have port security disabled. " - "For LinuxBridge, this requires ebtables. For OVS, it " - "requires a version that supports matching ARP " - "headers. This option will be removed in Ocata so " - "the only way to disable protection will be via the " - "port security extension.")), cfg.BoolOpt('dont_fragment', default=True, help=_("Set or un-set the don't fragment (DF) bit on " "outgoing IP packet carrying GRE/VXLAN tunnel.")), diff --git a/neutron/plugins/ml2/drivers/agent/_common_agent.py b/neutron/plugins/ml2/drivers/agent/_common_agent.py index e4afac11ae5..329a3c74265 100644 --- a/neutron/plugins/ml2/drivers/agent/_common_agent.py +++ b/neutron/plugins/ml2/drivers/agent/_common_agent.py @@ -78,8 +78,6 @@ class CommonAgentLoop(service.Service): sys.exit(1) def start(self): - self.prevent_arp_spoofing = cfg.CONF.AGENT.prevent_arp_spoofing - # stores all configured ports on agent self.network_ports = collections.defaultdict(list) # flag to do a sync after revival @@ -238,9 +236,8 @@ class CommonAgentLoop(service.Service): if 'port_id' in device_details: LOG.info(_LI("Port %(device)s updated. Details: %(details)s"), {'device': device, 'details': device_details}) - if self.prevent_arp_spoofing: - self.mgr.setup_arp_spoofing_protection(device, - device_details) + self.mgr.setup_arp_spoofing_protection(device, + device_details) segment = amb.NetworkSegment( device_details.get('network_type'), @@ -358,8 +355,7 @@ class CommonAgentLoop(service.Service): registry.notify(local_resources.PORT_DEVICE, events.AFTER_DELETE, self, context=self.context, device=device, port_id=port_id) - if self.prevent_arp_spoofing: - self.mgr.delete_arp_spoofing_protection(devices) + self.mgr.delete_arp_spoofing_protection(devices) return resync @staticmethod @@ -390,8 +386,7 @@ class CommonAgentLoop(service.Service): 'timestamps': {}} # clear any orphaned ARP spoofing rules (e.g. interface was # manually deleted) - if self.prevent_arp_spoofing: - self.mgr.delete_unreferenced_arp_protection(current_devices) + self.mgr.delete_unreferenced_arp_protection(current_devices) # check to see if any devices were locally modified based on their # timestamps changing since the previous iteration. If a timestamp diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index a129f397039..2e137daa8e2 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -244,7 +244,6 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, hybrid_plug = getattr(self.sg_agent.firewall, 'OVS_HYBRID_PLUG_REQUIRED', False) self.prevent_arp_spoofing = ( - agent_conf.prevent_arp_spoofing and not self.sg_agent.firewall.provides_arp_spoofing_protection) #TODO(mangelajo): optimize resource_versions to only report diff --git a/neutron/tests/functional/agent/l2/base.py b/neutron/tests/functional/agent/l2/base.py index e7373315471..26efc129f93 100644 --- a/neutron/tests/functional/agent/l2/base.py +++ b/neutron/tests/functional/agent/l2/base.py @@ -106,7 +106,6 @@ class OVSAgentTestFramework(base.BaseOVSLinuxTestCase): bridge_mappings = ['physnet:%s' % self.br_phys] self.config.set_override('tunnel_types', tunnel_types, "AGENT") self.config.set_override('polling_interval', 1, "AGENT") - self.config.set_override('prevent_arp_spoofing', False, "AGENT") self.config.set_override('local_ip', local_ip, "OVS") self.config.set_override('bridge_mappings', bridge_mappings, "OVS") # Physical bridges should be created prior to running diff --git a/neutron/tests/unit/plugins/ml2/drivers/agent/test__common_agent.py b/neutron/tests/unit/plugins/ml2/drivers/agent/test__common_agent.py index 8a55964bf32..e3f397c834d 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/agent/test__common_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/agent/test__common_agent.py @@ -50,7 +50,6 @@ class TestCommonAgentLoop(base.BaseTestCase): super(TestCommonAgentLoop, self).setUp() # disable setting up periodic state reporting cfg.CONF.set_override('report_interval', 0, 'AGENT') - cfg.CONF.set_override('prevent_arp_spoofing', False, 'AGENT') cfg.CONF.set_default('firewall_driver', 'neutron.agent.firewall.NoopFirewallDriver', group='SECURITYGROUP') @@ -180,9 +179,8 @@ class TestCommonAgentLoop(base.BaseTestCase): self.assertTrue(ext_mgr_delete_port.called) self.assertNotIn(PORT_DATA, agent.network_ports[NETWORK_ID]) - def test_treat_devices_removed_with_prevent_arp_spoofing_true(self): + def test_treat_devices_removed_delete_arp_spoofing(self): agent = self.agent - agent.prevent_arp_spoofing = True agent._ensure_port_admin_state = mock.Mock() devices = [DEVICE_1] with mock.patch.object(agent.plugin_rpc, @@ -379,8 +377,7 @@ class TestCommonAgentLoop(base.BaseTestCase): self._test_scan_devices(previous, updated, fake_current, expected, sync=True) - def test_scan_devices_with_prevent_arp_spoofing_true(self): - self.agent.prevent_arp_spoofing = True + def test_scan_devices_with_delete_arp_protection(self): previous = None fake_current = set([1, 2]) updated = set() @@ -474,9 +471,8 @@ class TestCommonAgentLoop(base.BaseTestCase): mock_details['network_id']] ) - def test_treat_devices_added_updated_prevent_arp_spoofing_true(self): + def test_treat_devices_added_updated_setup_arp_protection(self): agent = self.agent - agent.prevent_arp_spoofing = True mock_details = {'device': 'dev123', 'port_id': 'port123', 'network_id': 'net123', diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index 8ff04ea8e0e..10ef4c2503c 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -114,7 +114,6 @@ class TestOvsNeutronAgent(object): 'neutron.agent.firewall.NoopFirewallDriver', group='SECURITYGROUP') cfg.CONF.set_default('quitting_rpc_timeout', 10, 'AGENT') - cfg.CONF.set_default('prevent_arp_spoofing', False, 'AGENT') cfg.CONF.set_default('local_ip', '127.0.0.1', 'OVS') mock.patch( 'neutron.agent.ovsdb.native.helpers.enable_connection_uri').start() @@ -718,9 +717,11 @@ class TestOvsNeutronAgent(object): port_details = [ {'network_id': 'net1', 'vif_port': vif_port1, 'device': devices_up[0], + 'device_owner': 'network:dhcp', 'admin_state_up': True}, {'network_id': 'net1', 'vif_port': vif_port2, 'device': devices_down[0], + 'device_owner': 'network:dhcp', 'admin_state_up': False}] with mock.patch.object( self.agent.plugin_rpc, 'update_device_list', diff --git a/releasenotes/notes/removed_prevent_arp_spoofing-b49e91a92a93e3e1.yaml b/releasenotes/notes/removed_prevent_arp_spoofing-b49e91a92a93e3e1.yaml new file mode 100644 index 00000000000..b85e3f694c2 --- /dev/null +++ b/releasenotes/notes/removed_prevent_arp_spoofing-b49e91a92a93e3e1.yaml @@ -0,0 +1,6 @@ +--- +upgrade: + - | + The deprecated ``prevent_arp_spoofing`` option has been removed and the + default behavior is to always prevent ARP spoofing unless port security + is disabled on the port (or network).