ovs-fw: Fix firewall blink
Previously, when security group was updated for given port, the firewall removed all flows related to the port and added new rules. That introduced a time window where there were no rules for the port. This patch adds a new mechanism using cookie that can be described in three states: 1) Create new openflow rules with non-default cookie that is considered an updated cookie. All newly generated flows will be added with the next cookie and all existing rules with default cookie are rewritten with the default cookie. 2) Delete all rules for given port with the old default cookie. This will leave the newly added rules in place. 3) Update the newly added flows with update cookie back to the default cookie in order to avoid such flows being cleaned on the next restart of ovs agent, as it fetches for stale flows. Change-Id: I85d9e49c24ee7c91229b43cd329c42149637f254 Closes-bug: #1708731
This commit is contained in:
parent
ecc60df945
commit
6f7ba76075
|
@ -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
|
||||
|
||||
|
|
|
@ -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',
|
||||
|
|
|
@ -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)
|
||||
|
|
Loading…
Reference in New Issue