From 8b2c40366b3b65876e5465efae05b171be1bc473 Mon Sep 17 00:00:00 2001 From: Jakub Libosvar Date: Wed, 18 Apr 2018 10:25:01 +0000 Subject: [PATCH] ovs-fw: Apply openflow rules immediately during update Because update operation updates openflow rules three times: 1) New rules with new cookie 2) Delete old rules with old cookie 3) Change new cookie back to old cookie and the step 2) uses --strict parameter, it's needed to apply rules before deleting the old rules because --strict parameter cannot be combined with non-strict. This patch applies openflow rules after step 1), then --strict rules in step 2 are applied right away and then rest of delete part from 2) and all new rules from 3) are applied together. This patch adds optional interval parameter to Pinger class which sends more ICMP packets per second in the firewall blink tests to increase a chance of sending a packet while firewall is in inconsistent state. Change-Id: I25d9c87225feda1b5ddd442dd01529424186e05b Closes-bug: #1708731 --- neutron/agent/linux/openvswitch_firewall/firewall.py | 7 +++++++ neutron/tests/common/conn_testers.py | 3 ++- neutron/tests/common/net_helpers.py | 6 +++++- .../agent/linux/openvswitch_firewall/test_firewall.py | 10 ++++++++++ 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/neutron/agent/linux/openvswitch_firewall/firewall.py b/neutron/agent/linux/openvswitch_firewall/firewall.py index d11275e40a5..87e487dd68b 100644 --- a/neutron/agent/linux/openvswitch_firewall/firewall.py +++ b/neutron/agent/linux/openvswitch_firewall/firewall.py @@ -575,6 +575,13 @@ class OVSFirewallDriver(firewall.FirewallDriver): def _update_flows_for_port(self, of_port, old_of_port): with self.update_cookie_context(): self._set_port_filters(of_port) + # Flush the flows caused by changes made to deferred bridge. The reason + # is that following delete_all_port_flows() call uses --strict + # parameter that cannot be combined with other non-strict rules, hence + # all parameters with --strict are applied right away. In order to + # avoid applying delete rules with --strict *before* + # _set_port_filters() we dump currently cached flows here. + self.int_br.apply_flows() self.delete_all_port_flows(old_of_port) # Rewrite update cookie with default cookie self._set_port_filters(of_port) diff --git a/neutron/tests/common/conn_testers.py b/neutron/tests/common/conn_testers.py index 1d8b7a4309f..fd6de6ec043 100644 --- a/neutron/tests/common/conn_testers.py +++ b/neutron/tests/common/conn_testers.py @@ -308,7 +308,8 @@ class ConnectionTester(fixtures.Fixture): except KeyError: src_namespace, dst_address = self._get_namespace_and_address( direction) - pinger = net_helpers.Pinger(src_namespace, dst_address) + pinger = net_helpers.Pinger( + src_namespace, dst_address, interval=0.3) self._pingers[direction] = pinger return pinger diff --git a/neutron/tests/common/net_helpers.py b/neutron/tests/common/net_helpers.py index 6f8af60833d..303d1ca7a00 100644 --- a/neutron/tests/common/net_helpers.py +++ b/neutron/tests/common/net_helpers.py @@ -352,7 +352,8 @@ class Pinger(object): r'.* Destination .* Unreachable') TIMEOUT = 15 - def __init__(self, namespace, address, count=None, timeout=1): + def __init__(self, namespace, address, count=None, timeout=1, + interval=None): self.proc = None self.namespace = namespace self.address = address @@ -361,6 +362,7 @@ class Pinger(object): self.destination_unreachable = False self.sent = 0 self.received = 0 + self.interval = interval def _wait_for_death(self): is_dead = lambda: self.proc.poll() is not None @@ -390,6 +392,8 @@ class Pinger(object): cmd = [ping_exec, self.address, '-W', str(self.timeout)] if self.count: cmd.extend(['-c', str(self.count)]) + if self.interval: + cmd.extend(['-i', str(self.interval)]) self.proc = RootHelperProcess(cmd, namespace=self.namespace) def stop(self): 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 67d46fb722c..0685ae82def 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py @@ -609,6 +609,16 @@ class TestOVSFirewallDriver(base.BaseTestCase): self.firewall.update_port_filter(port_dict) self.assertTrue(self.mock_bridge.br.delete_flows.called) + def test_update_port_filter_applies_added_flows(self): + """Check flows are applied right after _set_flows is called.""" + port_dict = {'device': 'port-id', + 'security_groups': [1]} + self._prepare_security_group() + self.firewall.prepare_port_filter(port_dict) + with self.firewall.defer_apply(): + self.firewall.update_port_filter(port_dict) + self.assertEqual(2, self.mock_bridge.apply_flows.call_count) + def test_remove_port_filter(self): port_dict = {'device': 'port-id', 'security_groups': [1]}