From d559cd53e86fb0a3313289467bf4c56bbe76bcec Mon Sep 17 00:00:00 2001 From: Jakub Libosvar Date: Fri, 9 Jun 2017 13:59:05 +0000 Subject: [PATCH] ovs-fw: Use TRANSIENT table for traffic classification Commit ce8a0b2b7d73caf078c6634d6dded5117dbed265 introduces a TRANSIENT table where all traffic local to br-int is sent after it's been preprocessed by other features using openflow. This patch adopts the table. Change-Id: Ic66c186ab73bad6fcd133f2b9d15e07fd0eebb33 Related-bug: #1696983 --- .../linux/openvswitch_firewall/firewall.py | 23 ++++++-- neutron/tests/common/conn_testers.py | 7 ++- neutron/tests/tempest/scenario/base.py | 8 +-- .../tempest/scenario/test_portsecurity.py | 53 +++++++++++++++++++ .../openvswitch_firewall/test_firewall.py | 4 +- 5 files changed, 83 insertions(+), 12 deletions(-) create mode 100644 neutron/tests/tempest/scenario/test_portsecurity.py diff --git a/neutron/agent/linux/openvswitch_firewall/firewall.py b/neutron/agent/linux/openvswitch_firewall/firewall.py index 18be61fec45..31f4eabe7b4 100644 --- a/neutron/agent/linux/openvswitch_firewall/firewall.py +++ b/neutron/agent/linux/openvswitch_firewall/firewall.py @@ -391,6 +391,14 @@ class OVSFirewallDriver(firewall.FirewallDriver): else: self.int_br.br.delete_flows(**kwargs) + def _strict_delete_flow(self, **kwargs): + """Delete given flow right away even if bridge is deferred. + + Delete command will use strict delete. + """ + create_reg_numbers(kwargs) + self.int_br.br.delete_flows(strict=True, **kwargs) + @staticmethod def initialize_bridge(int_br): int_br.add_protocols(*OVSFirewallDriver.REQUIRED_PROTOCOLS) @@ -533,7 +541,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): """ # Identify egress flow self._add_flow( - table=ovs_consts.LOCAL_SWITCHING, + table=ovs_consts.TRANSIENT_TABLE, priority=100, in_port=port.ofport, actions='set_field:{:d}->reg{:d},' @@ -548,7 +556,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): # Identify ingress flows after egress filtering self._add_flow( - table=ovs_consts.LOCAL_SWITCHING, + table=ovs_consts.TRANSIENT_TABLE, priority=90, dl_dst=port.mac, actions='set_field:{:d}->reg{:d},' @@ -924,9 +932,14 @@ class OVSFirewallDriver(firewall.FirewallDriver): def delete_all_port_flows(self, port): """Delete all flows for given port""" - self._delete_flows(table=ovs_consts.LOCAL_SWITCHING, dl_dst=port.mac) - self._delete_flows(table=ovs_consts.LOCAL_SWITCHING, - in_port=port.ofport) + self._strict_delete_flow( + priority=90, + table=ovs_consts.TRANSIENT_TABLE, + dl_dst=port.mac) + self._strict_delete_flow( + priority=100, + table=ovs_consts.TRANSIENT_TABLE, + in_port=port.ofport) self._delete_flows(reg_port=port.ofport) self._delete_flows(table=ovs_consts.ACCEPT_OR_INGRESS_TABLE, dl_dst=port.mac) diff --git a/neutron/tests/common/conn_testers.py b/neutron/tests/common/conn_testers.py index ada66c49567..82d705d8496 100644 --- a/neutron/tests/common/conn_testers.py +++ b/neutron/tests/common/conn_testers.py @@ -22,6 +22,8 @@ from oslo_utils import uuidutils from neutron.agent import firewall from neutron.common import constants as n_consts from neutron.common import utils as common_utils +from neutron.plugins.ml2.drivers.openvswitch.agent.openflow.ovs_ofctl import ( + br_int) from neutron.tests.common import machine_fixtures from neutron.tests.common import net_helpers @@ -393,7 +395,10 @@ class OVSConnectionTester(OVSBaseConnectionTester): def _setUp(self): super(OVSConnectionTester, self)._setUp() - self.bridge = self.useFixture(net_helpers.OVSBridgeFixture()).bridge + br_name = self.useFixture( + net_helpers.OVSBridgeFixture()).bridge.br_name + self.bridge = br_int.OVSIntegrationBridge(br_name) + self.bridge.setup_default_table() machines = self.useFixture( machine_fixtures.PeerMachines( self.bridge, self.ip_cidr)).machines diff --git a/neutron/tests/tempest/scenario/base.py b/neutron/tests/tempest/scenario/base.py index 5ccbd1c311e..ac18ab5e8fc 100644 --- a/neutron/tests/tempest/scenario/base.py +++ b/neutron/tests/tempest/scenario/base.py @@ -192,10 +192,10 @@ class BaseTempestTestCase(base_api.BaseNetworkTest): waiters.wait_for_server_status(self.os_primary.servers_client, self.server['server']['id'], constants.SERVER_STATUS_ACTIVE) - port = self.client.list_ports(network_id=self.network['id'], - device_id=self.server[ - 'server']['id'])['ports'][0] - self.fip = self.create_and_associate_floatingip(port['id']) + self.port = self.client.list_ports(network_id=self.network['id'], + device_id=self.server[ + 'server']['id'])['ports'][0] + self.fip = self.create_and_associate_floatingip(self.port['id']) def check_connectivity(self, host, ssh_user, ssh_key, servers=None): ssh_client = ssh.Client(host, ssh_user, pkey=ssh_key) diff --git a/neutron/tests/tempest/scenario/test_portsecurity.py b/neutron/tests/tempest/scenario/test_portsecurity.py new file mode 100644 index 00000000000..76b23a41b40 --- /dev/null +++ b/neutron/tests/tempest/scenario/test_portsecurity.py @@ -0,0 +1,53 @@ +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from tempest.lib import decorators + +from neutron.tests.tempest import config +from neutron.tests.tempest.scenario import base + +CONF = config.CONF + + +class PortSecurityTest(base.BaseTempestTestCase): + credentials = ['primary'] + required_extensions = ['port-security'] + + @decorators.idempotent_id('61ab176e-d48b-42b7-b38a-1ba571ecc033') + def test_port_security_removed_added(self): + """Test connection works after port security has been removed + + Initial test that vm is accessible. Then port security is removed, + checked connectivity. Port security is added back and checked + connectivity again. + """ + self.setup_network_and_server() + self.check_connectivity(self.fip['floating_ip_address'], + CONF.validation.image_ssh_user, + self.keypair['private_key']) + sec_group_id = self.security_groups[0]['id'] + + self.port = self.update_port(port=self.port, + port_security_enabled=False, + security_groups=[]) + self.check_connectivity(self.fip['floating_ip_address'], + CONF.validation.image_ssh_user, + self.keypair['private_key']) + + self.port = self.update_port(port=self.port, + port_security_enabled=True, + security_groups=[sec_group_id]) + self.check_connectivity(self.fip['floating_ip_address'], + CONF.validation.image_ssh_user, + self.keypair['private_key']) diff --git a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py index 42f5b57fb90..e3c45178971 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py @@ -469,7 +469,7 @@ class TestOVSFirewallDriver(base.BaseTestCase): ovs_consts.BASE_EGRESS_TABLE), in_port=self.port_ofport, priority=100, - table=ovs_consts.LOCAL_SWITCHING) + table=ovs_consts.TRANSIENT_TABLE) exp_ingress_classifier = mock.call( actions='set_field:{:d}->reg5,set_field:{:d}->reg6,' 'resubmit(,{:d})'.format( @@ -477,7 +477,7 @@ class TestOVSFirewallDriver(base.BaseTestCase): ovs_consts.BASE_INGRESS_TABLE), dl_dst=self.port_mac, priority=90, - table=ovs_consts.LOCAL_SWITCHING) + table=ovs_consts.TRANSIENT_TABLE) filter_rule = mock.call( actions='ct(commit,zone=NXM_NX_REG6[0..15]),' 'strip_vlan,output:{:d}'.format(self.port_ofport),