Do no use "--strict" for OF deletion in TRANSIENT_TABLE
There are two types of OF rules in TRANSIENT_TABLE:
- With priority 100: these rules match by "in_port", that is a
unique identifier.
- With priority 90: these rules match by MAC address and VLAN ID.
This combination (MAC, VLAN) is unique.
That means when a deleting an OF rule in TRANSIENT_TABLE, it is
enough to specify the "in_port" or the (MAC, VLAN) tuple. The
"--strict" parameter, added to also define the priority, is not
needed.
By removing the "--strict" parameter, these deletion commands can
be executed synchronously at the end of the OVS deferred context,
when all the OF rule commands (addition or deletion), are executed
at the same time. That removes the small window, detected in the
related bug, when the OF rule set for a port is not complete.
Conflicts:
neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py
Closes-Bug: #1952770
Change-Id: I9f5bd8a1404dde3a0aa163ce72aef2961f537676
(cherry picked from commit ef7f673098)
This commit is contained in:
Rodolfo Alonso Hernandez
Slawek Kaplonski
parent
51b2c1b859
commit
c886bea8d9
|
|
@ -546,14 +546,6 @@ 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)
|
||||
|
|
@ -734,13 +726,6 @@ 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)
|
||||
|
|
@ -853,15 +838,13 @@ class OVSFirewallDriver(firewall.FirewallDriver):
|
|||
|
||||
def delete_physical_direct_flow(self, mac, segment_id):
|
||||
if segment_id:
|
||||
self._strict_delete_flow(priority=90,
|
||||
table=ovs_consts.TRANSIENT_TABLE,
|
||||
dl_dst=mac,
|
||||
dl_vlan=segment_id)
|
||||
self._delete_flows(table=ovs_consts.TRANSIENT_TABLE,
|
||||
dl_dst=mac,
|
||||
dl_vlan=segment_id)
|
||||
else:
|
||||
self._strict_delete_flow(priority=90,
|
||||
table=ovs_consts.TRANSIENT_TABLE,
|
||||
dl_dst=mac,
|
||||
vlan_tci=ovs_consts.FLAT_VLAN_TCI)
|
||||
self._delete_flows(table=ovs_consts.TRANSIENT_TABLE,
|
||||
dl_dst=mac,
|
||||
vlan_tci=ovs_consts.FLAT_VLAN_TCI)
|
||||
|
||||
def initialize_port_flows(self, port):
|
||||
"""Set base flows for port
|
||||
|
|
@ -1507,19 +1490,17 @@ class OVSFirewallDriver(firewall.FirewallDriver):
|
|||
def delete_all_port_flows(self, port):
|
||||
"""Delete all flows for given port"""
|
||||
for mac_addr in port.all_allowed_macs:
|
||||
self._strict_delete_flow(priority=90,
|
||||
table=ovs_consts.TRANSIENT_TABLE,
|
||||
dl_dst=mac_addr,
|
||||
dl_vlan=port.vlan_tag)
|
||||
self._delete_flows(table=ovs_consts.TRANSIENT_TABLE,
|
||||
dl_dst=mac_addr,
|
||||
dl_vlan=port.vlan_tag)
|
||||
self.delete_physical_direct_flow(mac_addr, port.segment_id)
|
||||
self._delete_flows(table=ovs_consts.ACCEPT_OR_INGRESS_TABLE,
|
||||
dl_dst=mac_addr, reg_net=port.vlan_tag)
|
||||
|
||||
self.delete_accepted_egress_direct_flow(
|
||||
port.mac, port.vlan_tag)
|
||||
self._strict_delete_flow(priority=100,
|
||||
table=ovs_consts.TRANSIENT_TABLE,
|
||||
in_port=port.ofport)
|
||||
self._delete_flows(table=ovs_consts.TRANSIENT_TABLE,
|
||||
in_port=port.ofport)
|
||||
self._delete_flows(reg_port=port.ofport)
|
||||
|
||||
def delete_flows_for_flow_state(
|
||||
|
|
|
|||
|
|
@ -773,16 +773,12 @@ class TestOVSFirewallDriver(base.BaseTestCase):
|
|||
|
||||
self.firewall.delete_all_port_flows(port)
|
||||
|
||||
call_args1 = {"strict": True,
|
||||
"priority": 90,
|
||||
"table": ovs_consts.TRANSIENT_TABLE,
|
||||
call_args1 = {"table": ovs_consts.TRANSIENT_TABLE,
|
||||
"dl_dst": port.mac,
|
||||
"dl_vlan": port.vlan_tag}
|
||||
flow1 = mock.call(**call_args1)
|
||||
|
||||
call_args2 = {"strict": True,
|
||||
"priority": 90,
|
||||
"table": ovs_consts.TRANSIENT_TABLE,
|
||||
call_args2 = {"table": ovs_consts.TRANSIENT_TABLE,
|
||||
"dl_dst": port.mac,
|
||||
"dl_vlan": port.segment_id}
|
||||
flow2 = mock.call(**call_args2)
|
||||
|
|
@ -793,8 +789,6 @@ class TestOVSFirewallDriver(base.BaseTestCase):
|
|||
flow3 = mock.call(**call_args3)
|
||||
|
||||
call_args4 = {"in_port": port.ofport,
|
||||
"strict": True,
|
||||
"priority": 100,
|
||||
"table": ovs_consts.TRANSIENT_TABLE}
|
||||
flow4 = mock.call(**call_args4)
|
||||
|
||||
|
|
@ -895,7 +889,7 @@ class TestOVSFirewallDriver(base.BaseTestCase):
|
|||
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)
|
||||
self.assertEqual(1, self.mock_bridge.apply_flows.call_count)
|
||||
|
||||
def test_update_port_filter_clean_when_port_not_found(self):
|
||||
"""Check flows are cleaned if port is not found in the bridge."""
|
||||
|
|
|
|||
Loading…
Reference in New Issue