openstack
/
neutron
Code Issues Proposed changes

Make the dead vlan actually dead 

All ports plugged into the dead vlan (DEAD_VLAN_TAG 4095 or 0xfff)
should not be able to send or receive traffic. We install a flow
to br-int to drop all traffic of the dead vlan [1]. However before
this patch the flow we install looks like:

priority=65535,vlan_tci=0x0fff/0x1fff actions=drop

Which is wrong and it usually does not match anything.

According to ovs-fields (7) section Open vSwitch Extension VLAN Field,
VLAN TCI Field [2] (see especially the usage example
vlan_tci=0x1123/0x1fff) we need to explicitly set the bit 0x1000
to match the presence of an 802.1Q header.

Setting that bit this flow becomes:
priority=65535,vlan_tci=0x1fff/0x1fff actions=drop

which is equivalent to:
priority=65535,dl_vlan=4095 actions=drop

which should match and drop dead vlan traffic.

However there's a second problem: ovs access ports were designed to
work together with the NORMAL action. The NORMAL action considers the
vlan of an access port, but the openflow pipeline does not. An openflow
rule does not see the vlan set for an access port, because that vlan
tag is only pushed to the frame if and when the frame leaves the switch
on a trunk port [3][4].

So we have to explicitly push the DEAD_VLAN_TAG if we want the dead
vlan's drop flow match anything.

That means we are adding a flow to push the dead vlan tag from
dhcp-agent/l3-agent but we are deleting that flow from ovs-agent right
after ovs-agent sets the vlan tag of the port to a non-dead vlan. Which
is ugly but we have to keep adding the flow as early as possible if we
want to minimize the window until frames can leak onto the dead vlan.
Even with this change there's a short time window in which the dead vlan
could theoretically leak.

[1] ecdc11a564/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_int.py (L60-L62)
[2] http://www.openvswitch.org/support/dist-docs/ovs-fields.7.html
[3] https://mail.openvswitch.org/pipermail/ovs-discuss/2021-December/051647.html
[4] https://docs.openvswitch.org/en/latest/faq/vlan/
    see 'Q: My OpenFlow controller doesn’t see the VLANs that I expect.'

Change-Id: Ib6b70114efb140cf1393b57ebc350fea4b0a2443
Closes-Bug: #1930414
(cherry picked from commit 7aae31c9f9)
This commit is contained in:
Bence Romsics 2021-10-05 17:02:41 +02:00
parent 97180b0183
commit 5025a6a727
6 changed files with 69 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
neutron/agent/common/ovs_lib.py
View File

@ -372,6 +372,23 @@ class OVSBridge(BaseOVS):
txn.add(self.ovsdb.db_set('Interface', port_name,
*interface_attr_tuples))
# NOTE(bence romsics): We are after the ovsdb transaction therefore
# there's still a short time window between the port created and
# the flow added in which the dead vlan tag is not pushed onto the
# frames arriving at these ports and because of that those frames may
# get through. However before the transaction we cannot create the
# flow because we don't have the ofport. And I'm not aware of a
# combined ovsdb+openflow transaction to do it inside the transaction.
if (self.br_name == cfg.CONF.OVS.integration_bridge):
self.add_flow(
table=constants.LOCAL_SWITCHING,
priority=constants.OPENFLOW_MAX_PRIORITY - 1,
in_port=self.get_port_ofport(port_name),
actions='mod_vlan_vid:{:d},'
'resubmit(,{:d})'.format(
constants.DEAD_VLAN_TAG,
constants.LOCAL_SWITCHING))
def delete_port(self, port_name):
self.ovsdb.del_port(port_name, self.br_name).execute()

9
neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_int.py
View File

@ -51,15 +51,18 @@ class OVSIntegrationBridge(ovs_bridge.OVSAgentBridge,
def setup_default_table(self, enable_openflow_dhcp=False,
enable_dhcpv6=False):
(_dp, ofp, ofpp) = self._get_dp()
self.setup_canary_table()
self.install_goto(dest_table_id=constants.TRANSIENT_TABLE)
self.install_normal(table_id=constants.TRANSIENT_TABLE, priority=3)
self.init_dhcp(enable_openflow_dhcp=enable_openflow_dhcp,
enable_dhcpv6=enable_dhcpv6)
self.install_drop(table_id=constants.ARP_SPOOF_TABLE)
self.install_drop(table_id=constants.LOCAL_SWITCHING,
priority=constants.OPENFLOW_MAX_PRIORITY,
vlan_vid=constants.DEAD_VLAN_TAG)
self.install_drop(
table_id=constants.LOCAL_SWITCHING,
priority=constants.OPENFLOW_MAX_PRIORITY,
vlan_vid=ofp.OFPVID_PRESENT | constants.DEAD_VLAN_TAG,
)
# When openflow firewall is not enabled, we use this table to
# deal with all egress flow.
self.install_normal(table_id=constants.TRANSIENT_EGRESS_TABLE,

11
neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
View File

@ -1199,6 +1199,17 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
if cur_tag != lvm.vlan:
self.int_br.set_db_attribute(
"Port", port.port_name, "tag", lvm.vlan)
# When changing the port's tag from DEAD_VLAN_TAG to
# something else, also delete the previously dead port's
# push DEAD_VLAN_TAG flow which we installed from
# ovs_lib.OVSBridge.replace_port().
if (cur_tag == constants.DEAD_VLAN_TAG and port.ofport != -1):
self.int_br.delete_flows(
cookie=ovs_lib.COOKIE_ANY,
table=constants.LOCAL_SWITCHING,
priority=constants.OPENFLOW_MAX_PRIORITY - 1,
in_port=port.ofport,
strict=True)
# update plugin about port status
# FIXME(salv-orlando): Failures while updating device status

8
neutron/tests/common/conn_testers.py
View File

@ -20,6 +20,7 @@ from neutron_lib import constants
from oslo_config import cfg
from oslo_utils import uuidutils
from neutron.agent.common import ovs_lib
from neutron.common import utils as common_utils
from neutron.plugins.ml2.drivers.openvswitch.agent.common import (
constants as ovs_consts)
@ -385,6 +386,13 @@ class OVSBaseConnectionTester(ConnectionTester):
txn.add(
ovsdb.db_add(
'Port', port_name, 'other_config', {'tag': str(tag)}))
bridge.delete_flows(
cookie=ovs_lib.COOKIE_ANY,
table=ovs_consts.LOCAL_SWITCHING,
priority=ovs_consts.OPENFLOW_MAX_PRIORITY - 1,
in_port=bridge.get_port_ofport(port_name),
strict=True,
)
class OVSConnectionTester(OVSBaseConnectionTester):

35
neutron/tests/functional/agent/test_dhcp_agent.py
View File

@ -35,6 +35,7 @@ from neutron.agent.linux import utils
from neutron.agent.metadata import driver as metadata_driver
from neutron.common import utils as common_utils
from neutron.conf.agent import common as config
from neutron.plugins.ml2.drivers.openvswitch.agent.common import constants
from neutron.tests.common import net_helpers
from neutron.tests.functional.agent.linux import helpers
from neutron.tests.functional import base
@ -73,8 +74,9 @@ class DHCPAgentOVSTestFramework(base.BaseSudoTestCase):
'interface_driver',
'neutron.agent.linux.interface.OVSInterfaceDriver')
self.conf.set_override('report_interval', 0, 'AGENT')
br_int = self.useFixture(net_helpers.OVSBridgeFixture()).bridge
self.conf.set_override('integration_bridge', br_int.br_name, 'OVS')
self.br_int = self.useFixture(net_helpers.OVSBridgeFixture()).bridge
self.conf.set_override(
'integration_bridge', self.br_int.br_name, 'OVS')
self.mock_plugin_api = mock.patch(
'neutron.agent.dhcp.agent.DhcpPluginApi').start().return_value
@ -291,13 +293,6 @@ class DHCPAgentOVSTestCase(DHCPAgentOVSTestFramework):
dev = ip_lib.IPDevice(iface_name, network.namespace)
self.assertEqual(789, dev.link.mtu)
def test_good_address_allocation(self):
network, port = self._get_network_port_for_allocation_test()
network.ports.append(port)
self.configure_dhcp_for_network(network=network)
self._plug_port_for_dhcp_request(network, port)
self.assert_good_allocation_for_port(network, port)
def test_bad_address_allocation(self):
network, port = self._get_network_port_for_allocation_test()
network.ports.append(port)
@ -448,3 +443,25 @@ class DHCPAgentOVSTestCase(DHCPAgentOVSTestFramework):
else:
self.assertEqual(agent.DHCP_PROCESS_GREENLET_MAX,
self.agent._pool.size)
class DHCPAgentOVSTestCaseOwnBridge(DHCPAgentOVSTestFramework):
"""Class for a single test that needs its own bridge.
This way the ofport numbers are predictable and we can fake ovs-agent's
flow deletions with hardcoded ofport numbers.
"""
def test_good_address_allocation(self):
network, port = self._get_network_port_for_allocation_test()
network.ports.append(port)
self.configure_dhcp_for_network(network=network)
self._plug_port_for_dhcp_request(network, port)
for ofport in (1, 2):
self.br_int.delete_flows(
cookie=ovs_lib.COOKIE_ANY,
table=constants.LOCAL_SWITCHING,
priority=constants.OPENFLOW_MAX_PRIORITY - 1,
in_port=ofport,
strict=True)
self.assert_good_allocation_for_port(network, port)

2
neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_int.py
View File

@ -111,7 +111,7 @@ class OVSIntegrationBridgeTest(ovs_bridge_test_base.OVSBridgeTestBase):
call._send_msg(ofpp.OFPFlowMod(dp,
cookie=self.stamp,
instructions=[],
match=ofpp.OFPMatch(vlan_vid=4095),
match=ofpp.OFPMatch(vlan_vid=ofp.OFPVID_PRESENT | 4095),
priority=65535,
table_id=0),
active_bundle=None),