Setup firewall filters only for required ports

We can skip trying to setup firewall filters for ports which are
having port_security_enabled as False or which are not associated
to any security group.

Closes-Bug: #1482554

Change-Id: Ie65201308d93c746fe4ef38f402ec300227b7d27

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 190c54b3a7e4bd0c39ae33e2b9df764dad35c8af..c6d76b0e45f1de8bb95d30a8bb111418b7b80a5c 100644 (file)
--- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
+++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
@@ -1228,6 +1228,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
     def treat_devices_added_or_updated(self, devices, ovs_restarted):
         skipped_devices = []
         need_binding_devices = []
+        security_disabled_devices = []
         devices_details_list = (
             self.plugin_rpc.get_devices_details_list_and_failed_devices(
                 self.context,
@@ -1268,12 +1269,18 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
                                                    ovs_restarted)
                 if need_binding:
                     need_binding_devices.append(details)
+
+                port_security = details['port_security_enabled']
+                has_sgs = 'security_groups' in details
+                if not port_security or not has_sgs:
+                    security_disabled_devices.append(device)
+
                 self.ext_manager.handle_port(self.context, details)
             else:
                 LOG.warn(_LW("Device %s not defined on plugin"), device)
                 if (port and port.ofport != -1):
                     self.port_dead(port)
-        return skipped_devices, need_binding_devices
+        return skipped_devices, need_binding_devices, security_disabled_devices
 
     def treat_ancillary_devices_added(self, devices):
         devices_details_list = (
@@ -1356,10 +1363,12 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
         devices_added_updated = (port_info.get('added', set()) |
                                  port_info.get('updated', set()))
         need_binding_devices = []
+        security_disabled_ports = []
         if devices_added_updated:
             start = time.time()
             try:
-                skipped_devices, need_binding_devices = (
+                (skipped_devices, need_binding_devices,
+                    security_disabled_ports) = (
                     self.treat_devices_added_or_updated(
                         devices_added_updated, ovs_restarted))
                 LOG.debug("process_network_ports - iteration:%(iter_num)d - "
@@ -1385,7 +1394,10 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
 
         # TODO(salv-orlando): Optimize avoiding applying filters
         # unnecessarily, (eg: when there are no IP address changes)
-        self.sg_agent.setup_port_filters(port_info.get('added', set()),
+        added_ports = port_info.get('added', set())
+        if security_disabled_ports:
+            added_ports -= set(security_disabled_ports)
+        self.sg_agent.setup_port_filters(added_ports,
                                          port_info.get('updated', set()))
         self._bind_devices(need_binding_devices)
 

diff --git a/neutron/plugins/ml2/rpc.py b/neutron/plugins/ml2/rpc.py
index 383bc60b675efafd245a8862e9549fd562ebc4a8..8ae7997a2187fcecafde73cacf97abfc1ac564a5 100644 (file)
--- a/neutron/plugins/ml2/rpc.py
+++ b/neutron/plugins/ml2/rpc.py
@@ -126,6 +126,8 @@ class RpcCallbacks(type_tunnel.TunnelRpcCallbackMixin):
                  'port_security_enabled': port.get(psec.PORTSECURITY, True),
                  'qos_policy_id': qos_policy_id,
                  'profile': port[portbindings.PROFILE]}
+        if 'security_groups' in port:
+            entry['security_groups'] = port['security_groups']
         LOG.debug("Returning: %s", entry)
         return entry
 

diff --git a/neutron/tests/functional/agent/l2/base.py b/neutron/tests/functional/agent/l2/base.py
index 46706d7ddad9fc7f17e0dab63f0fee0cb157b898..71687d88911fae35e961b38175442458f3d887c3 100644 (file)
--- a/neutron/tests/functional/agent/l2/base.py
+++ b/neutron/tests/functional/agent/l2/base.py
@@ -183,6 +183,8 @@ class OVSAgentTestFramework(base.BaseOVSLinuxTestCase):
                'segmentation_id': 1,
                'fixed_ips': port['fixed_ips'],
                'device_owner': 'compute',
+               'port_security_enabled': True,
+               'security_groups': ['default'],
                'admin_state_up': True}
         return dev
 

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 72eb801e96aa4cde367dd3f23525ed53fde08d8d..47cba3aa75d18c5cb38fb8380ea3388b9d79dec8 100644 (file)
--- 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
@@ -406,7 +406,7 @@ class TestOvsNeutronAgent(object):
                                                 'failed_devices_up': [],
                                                 'failed_devices_down': []}),\
                 mock.patch.object(self.agent, func_name) as func:
-            skip_devs, need_bound_devices = (
+            skip_devs, need_bound_devices, insecure_ports = (
                 self.agent.treat_devices_added_or_updated([{}], False))
             # The function should not raise
             self.assertFalse(skip_devs)
@@ -477,7 +477,7 @@ class TestOvsNeutronAgent(object):
             skip_devs = self.agent.treat_devices_added_or_updated([{}], False)
             # The function should return False for resync and no device
             # processed
-            self.assertEqual((['the_skipped_one'], []), skip_devs)
+            self.assertEqual((['the_skipped_one'], [], []), skip_devs)
             self.assertFalse(treat_vif_port.called)
 
     def test_treat_devices_added_updated_put_port_down(self):
@@ -490,7 +490,8 @@ class TestOvsNeutronAgent(object):
                              'network_type': 'baz',
                              'fixed_ips': [{'subnet_id': 'my-subnet-uuid',
                                             'ip_address': '1.1.1.1'}],
-                             'device_owner': 'compute:None'
+                             'device_owner': 'compute:None',
+                             'port_security_enabled': True
                              }
 
         with mock.patch.object(self.agent.plugin_rpc,
@@ -502,7 +503,7 @@ class TestOvsNeutronAgent(object):
                                   return_value={'xxx': mock.MagicMock()}),\
                 mock.patch.object(self.agent,
                                   'treat_vif_port') as treat_vif_port:
-            skip_devs, need_bound_devices = (
+            skip_devs, need_bound_devices, insecure_ports = (
                 self.agent.treat_devices_added_or_updated([{}], False))
             # The function should return False for resync
             self.assertFalse(skip_devs)
@@ -538,7 +539,7 @@ class TestOvsNeutronAgent(object):
                 mock.patch.object(
                     self.agent,
                     "treat_devices_added_or_updated",
-                    return_value=([], [])) as device_added_updated,\
+                    return_value=([], [], [])) as device_added_updated,\
                 mock.patch.object(self.agent.int_br, "get_ports_attributes",
                                   return_value=[]),\
                 mock.patch.object(self.agent,
@@ -573,6 +574,24 @@ class TestOvsNeutronAgent(object):
     def test_process_network_port_with_empty_port(self):
         self._test_process_network_ports({})
 
+    def test_process_network_ports_with_insecure_ports(self):
+        port_info = {'current': set(['tap0', 'tap1']),
+                     'updated': set(['tap1']),
+                     'removed': set([]),
+                     'added': set(['eth1'])}
+        with mock.patch.object(self.agent.sg_agent,
+                               "setup_port_filters") as setup_port_filters,\
+                mock.patch.object(
+                    self.agent,
+                    "treat_devices_added_or_updated",
+                    return_value=([], [], ['eth1'])) as device_added_updated:
+            self.assertFalse(self.agent.process_network_ports(port_info,
+                                                              False))
+            device_added_updated.assert_called_once_with(
+                set(['eth1', 'tap1']), False)
+            setup_port_filters.assert_called_once_with(
+                set(), port_info.get('updated', set()))
+
     def test_report_state(self):
         with mock.patch.object(self.agent.state_rpc,
                                "report_state") as report_st: