diff --git a/neutron/agent/linux/openvswitch_firewall/firewall.py b/neutron/agent/linux/openvswitch_firewall/firewall.py index 1390302ed00..bd1172aa48c 100644 --- a/neutron/agent/linux/openvswitch_firewall/firewall.py +++ b/neutron/agent/linux/openvswitch_firewall/firewall.py @@ -14,6 +14,7 @@ # under the License. import collections +import contextlib import copy import netaddr @@ -391,6 +392,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): """ self.int_br = self.initialize_bridge(integration_bridge) + self._update_cookie = None self.sg_port_map = SGPortMap() self.sg_to_delete = set() self._deferred = False @@ -401,6 +403,15 @@ class OVSFirewallDriver(firewall.FirewallDriver): self.iptables_helper = iptables.Helper(self.int_br.br) self.iptables_helper.load_driver_if_needed() + @contextlib.contextmanager + def update_cookie_context(self): + try: + self._update_cookie = self.int_br.br.request_cookie() + yield + finally: + self.int_br.br.unset_cookie(self._update_cookie) + self._update_cookie = None + def security_group_updated(self, action_type, sec_group_ids, device_ids=None): """The current driver doesn't make use of this method. @@ -418,6 +429,8 @@ class OVSFirewallDriver(firewall.FirewallDriver): create_reg_numbers(kwargs) if isinstance(dl_type, int): kwargs['dl_type'] = "0x{:04x}".format(dl_type) + if self._update_cookie: + kwargs['cookie'] = self._update_cookie if self._deferred: self.int_br.add_flow(**kwargs) else: @@ -499,7 +512,15 @@ class OVSFirewallDriver(firewall.FirewallDriver): if not firewall.port_sec_enabled(port): self._initialize_egress_no_port_security(port['device']) return - self._set_port_filters(port, old_port_expected=False) + + old_of_port = self.get_ofport(port) + of_port = self.get_or_create_ofport(port) + if old_of_port: + LOG.info("Initializing port %s that was already initialized.", + port['device']) + self._update_flows_for_port(of_port, old_of_port) + else: + self._set_port_filters(of_port) def update_port_filter(self, port): """Update rules for given port @@ -521,27 +542,32 @@ class OVSFirewallDriver(firewall.FirewallDriver): self.prepare_port_filter(port) return try: - self._set_port_filters(port, old_port_expected=True) + # Make sure delete old allowed_address_pair MACs because + # allowed_address_pair MACs will be updated in + # self.get_or_create_ofport(port) + old_of_port = self.get_ofport(port) + of_port = self.get_or_create_ofport(port) + if old_of_port: + self._update_flows_for_port(of_port, old_of_port) + else: + self._set_port_filters(of_port) + except exceptions.OVSFWPortNotFound as not_found_error: LOG.info("port %(port_id)s does not exist in ovsdb: %(err)s.", {'port_id': port['device'], 'err': not_found_error}) - def _set_port_filters(self, port, old_port_expected): - old_of_port = self.get_ofport(port) - # Make sure delete old allowed_address_pair MACs because - # allowed_address_pair MACs will be updated in - # self.get_or_create_ofport(port) - if old_of_port: - if not old_port_expected: - LOG.info("Initializing port %s that was already " - "initialized.", port['device']) - self.delete_all_port_flows(old_of_port) - # TODO(jlibosva): Handle firewall blink - of_port = self.get_or_create_ofport(port) + def _set_port_filters(self, of_port): self.initialize_port_flows(of_port) self.add_flows_from_rules(of_port) + def _update_flows_for_port(self, of_port, old_of_port): + with self.update_cookie_context(): + self._set_port_filters(of_port) + self.delete_all_port_flows(old_of_port) + # Rewrite update cookie with default cookie + self._set_port_filters(of_port) + def remove_port_filter(self, port): """Remove port from firewall diff --git a/neutron/tests/functional/agent/test_firewall.py b/neutron/tests/functional/agent/test_firewall.py index 21787f8f63d..bde9090d53a 100644 --- a/neutron/tests/functional/agent/test_firewall.py +++ b/neutron/tests/functional/agent/test_firewall.py @@ -558,7 +558,6 @@ class FirewallTestCase(BaseFirewallTestCase): self._apply_security_group_rules(self.FAKE_SECURITY_GROUP_ID, list()) self.tester.assert_no_established_connection(**connection) - @skip_if_firewall('openvswitch') def test_preventing_firewall_blink(self): direction = self.tester.INGRESS sg_rules = [{'ethertype': 'IPv4', 'direction': 'ingress', 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 c1c8e531c39..67d46fb722c 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py @@ -17,12 +17,15 @@ from neutron_lib import constants import testtools from neutron.agent.common import ovs_lib +from neutron.agent.common import utils from neutron.agent.linux.openvswitch_firewall import constants as ovsfw_consts from neutron.agent.linux.openvswitch_firewall import exceptions from neutron.agent.linux.openvswitch_firewall import firewall as ovsfw from neutron.common import constants as n_const from neutron.plugins.ml2.drivers.openvswitch.agent.common import constants \ as ovs_consts +from neutron.plugins.ml2.drivers.openvswitch.agent.openflow.ovs_ofctl \ + import ovs_bridge from neutron.tests import base TESTING_VLAN_TAG = 1 @@ -715,3 +718,49 @@ class TestOVSFirewallDriver(base.BaseTestCase): def test_remove_trusted_ports_not_managed_port(self): """Check that exception is not propagated outside.""" self.firewall.remove_trusted_ports(['port_id']) + + +class TestCookieContext(base.BaseTestCase): + def setUp(self): + super(TestCookieContext, self).setUp() + # Don't attempt to connect to ovsdb + mock.patch('neutron.agent.ovsdb.api.from_config').start() + # Don't trigger iptables -> ovsfw migration + mock.patch( + 'neutron.agent.linux.openvswitch_firewall.iptables.Helper').start() + + self.execute = mock.patch.object( + utils, "execute", spec=utils.execute).start() + bridge = ovs_bridge.OVSAgentBridge('foo') + mock.patch.object( + ovsfw.OVSFirewallDriver, 'initialize_bridge', + return_value=bridge.deferred( + full_ordered=True, use_bundle=True)).start() + + self.firewall = ovsfw.OVSFirewallDriver(bridge) + # Remove calls from firewall initialization + self.execute.reset_mock() + + def test_cookie_is_different_in_context(self): + default_cookie = self.firewall.int_br.br.default_cookie + with self.firewall.update_cookie_context(): + self.firewall._add_flow(actions='drop') + update_cookie = self.firewall._update_cookie + self.firewall._add_flow(actions='drop') + expected_calls = [ + mock.call( + mock.ANY, + process_input='hard_timeout=0,idle_timeout=0,priority=1,' + 'cookie=%d,actions=drop' % cookie, + run_as_root=mock.ANY, + ) for cookie in (update_cookie, default_cookie) + ] + + self.execute.assert_has_calls(expected_calls) + + def test_context_cookie_is_not_left_as_used(self): + with self.firewall.update_cookie_context(): + update_cookie = self.firewall._update_cookie + self.assertNotIn( + update_cookie, + self.firewall.int_br.br._reserved_cookies)