From ea4165c2af2ad5c7b5423e25c507598ebd30f7b5 Mon Sep 17 00:00:00 2001 From: Kevin Benton Date: Wed, 25 Nov 2015 15:42:46 -0800 Subject: [PATCH] Don't drop ARP table jump during OVS rewiring The previous OVS ARP spoofing code was dropping the rule to jump to the ARP protection table each time it was called. This call was unnecessary since the majority of port updates are not turning off port security. This patch adjusts the logic to only drop the jump rule if port-sec is disabled or if it is a network port. The existing functional tests ensure that connectivity works as expected. Closes-Bug: #1520013 Change-Id: I7b396d758c2d4c7e1004257d432b210bf3ee5c66 --- .../drivers/openvswitch/agent/openflow/native/br_int.py | 3 +++ .../openvswitch/agent/openflow/ovs_ofctl/br_int.py | 3 +++ .../ml2/drivers/openvswitch/agent/ovs_neutron_agent.py | 8 ++++++-- .../drivers/openvswitch/agent/test_ovs_neutron_agent.py | 4 ++-- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_int.py b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_int.py index ce4b79028..26d15cc11 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_int.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_int.py @@ -206,5 +206,8 @@ class OVSIntegrationBridge(ovs_bridge.OVSAgentBridge): match = self._icmpv6_reply_match(ofp, ofpp, port=port) self.delete_flows(table_id=constants.LOCAL_SWITCHING, match=match) + self.delete_arp_spoofing_allow_rules(port) + + def delete_arp_spoofing_allow_rules(self, port): self.delete_flows(table_id=constants.ARP_SPOOF_TABLE, in_port=port) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/br_int.py b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/br_int.py index ef232cd0d..d0aca03d7 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/br_int.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/br_int.py @@ -149,5 +149,8 @@ class OVSIntegrationBridge(ovs_bridge.OVSAgentBridge): self.delete_flows(table_id=constants.LOCAL_SWITCHING, in_port=port, nw_proto=const.PROTO_NUM_ICMP_V6, icmp_type=const.ICMPV6_TYPE_NA) + self.delete_arp_spoofing_allow_rules(port) + + def delete_arp_spoofing_allow_rules(self, port): self.delete_flows(table_id=constants.ARP_SPOOF_TABLE, in_port=port) 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 1e8154706..334547ceb 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -879,17 +879,19 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, @staticmethod def setup_arp_spoofing_protection(bridge, vif, port_details): - # clear any previous flows related to this port in our ARP table - bridge.delete_arp_spoofing_protection(port=vif.ofport) if not port_details.get('port_security_enabled', True): LOG.info(_LI("Skipping ARP spoofing rules for port '%s' because " "it has port security disabled"), vif.port_name) + bridge.delete_arp_spoofing_protection(port=vif.ofport) return if port_details['device_owner'].startswith( n_const.DEVICE_OWNER_NETWORK_PREFIX): LOG.debug("Skipping ARP spoofing rules for network owned port " "'%s'.", vif.port_name) + bridge.delete_arp_spoofing_protection(port=vif.ofport) return + # clear any previous flows related to this port in our ARP table + bridge.delete_arp_spoofing_allow_rules(port=vif.ofport) # collect all of the addresses and cidrs that belong to the port addresses = {f['ip_address'] for f in port_details['fixed_ips']} mac_addresses = {vif.vif_mac} @@ -921,6 +923,8 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, # match on /1 or more. bridge.install_arp_spoofing_protection(port=vif.ofport, ip_addresses=ipv4_addresses) + else: + bridge.delete_arp_spoofing_protection(port=vif.ofport) def port_unbound(self, vif_id, net_uuid=None): '''Unbind port. 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 a89526bdf..bebd08199 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 @@ -1487,7 +1487,7 @@ class TestOvsNeutronAgent(object): self.agent.setup_arp_spoofing_protection(int_br, vif, fake_details) self.assertEqual( [mock.call(port=vif.ofport)], - int_br.delete_arp_spoofing_protection.mock_calls) + int_br.delete_arp_spoofing_allow_rules.mock_calls) self.assertEqual( [mock.call(ip_addresses=set(), port=vif.ofport)], int_br.install_arp_spoofing_protection.mock_calls) @@ -1501,7 +1501,7 @@ class TestOvsNeutronAgent(object): self.agent.setup_arp_spoofing_protection(br, vif, fake_details) self.assertEqual( [mock.call(port=vif.ofport)], - br.delete_arp_spoofing_protection.mock_calls) + br.delete_arp_spoofing_allow_rules.mock_calls) self.assertTrue(br.install_icmpv6_na_spoofing_protection.called) def test_arp_spoofing_fixed_and_allowed_addresses(self): -- 2.45.2