From: Kevin Benton <blak111@gmail.com>
Date: Wed, 25 Nov 2015 23:42:46 +0000 (-0800)
Subject: Don't drop ARP table jump during OVS rewiring
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=ea4165c2af2ad5c7b5423e25c507598ebd30f7b5;p=openstack-build%2Fneutron-build.git

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
---

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):