From 75edc1ff28a460342a9b5e5b7d63c6f4fb59862d Mon Sep 17 00:00:00 2001 From: Sudhakar Babu Gariganti Date: Fri, 7 Aug 2015 16:07:12 +0530 Subject: [PATCH] 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 --- .../openvswitch/agent/ovs_neutron_agent.py | 18 ++++++++++-- neutron/plugins/ml2/rpc.py | 2 ++ neutron/tests/functional/agent/l2/base.py | 2 ++ .../agent/test_ovs_neutron_agent.py | 29 +++++++++++++++---- 4 files changed, 43 insertions(+), 8 deletions(-) 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 190c54b3a..c6d76b0e4 100644 --- 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 383bc60b6..8ae7997a2 100644 --- 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 46706d7dd..71687d889 100644 --- 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 72eb801e9..47cba3aa7 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 @@ -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: -- 2.45.2