From 8cf622d83dceedc20a0fae66d790380823e43676 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Thu, 12 Dec 2024 15:56:36 +0100 Subject: [PATCH] QinQ implementation for the ML2/OVN backend This patch implements support for the 'vlan_qinq' network parameter in the ML2/OVN backend. It is done in almost similar way to the 'vlan_transparent' parameter, the difference is in the 'ethtype' set for the provnet port for the network. For QinQ it is set to '802.1ad'. It also adds functional tests for the 'vlan_transparent' setting for the OVN mechanism driver. The reason why those 2 are tested together is that both are using the same options on the OVN side and are mutually exclusive so we have to make sure we set those options as expected in each case. Related-Bug: #1915151 Change-Id: I110c366a37a65d625083a7112f1adb9a3dc5e7cc --- neutron/common/ovn/constants.py | 15 +++- neutron/common/ovn/extensions.py | 2 + .../drivers/ovn/mech_driver/mech_driver.py | 6 +- .../ovn/mech_driver/ovsdb/ovn_client.py | 29 +++++-- .../ovn/mech_driver/ovsdb/ovn_db_sync.py | 2 +- .../ovn/mech_driver/test_mech_driver.py | 86 +++++++++++++++++++ .../ovn/mech_driver/ovsdb/test_ovn_db_sync.py | 3 +- .../ovn/mech_driver/test_mech_driver.py | 41 ++++++--- ...the-network-resource-4a1ab8497dfd16b8.yaml | 15 ++++ 9 files changed, 179 insertions(+), 20 deletions(-) create mode 100644 releasenotes/notes/add-qinq-attribute-to-the-network-resource-4a1ab8497dfd16b8.yaml diff --git a/neutron/common/ovn/constants.py b/neutron/common/ovn/constants.py index f58b398916c..cf468e6e3f9 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -447,8 +447,21 @@ CMS_OPT_CHASSIS_AS_EXTPORT_HOST = 'enable-chassis-as-extport-host' CMS_OPT_AVAILABILITY_ZONES = 'availability-zones' CMS_OPT_CARD_SERIAL_NUMBER = 'card-serial-number' -# OVN vlan transparency option +# OVN vlan transparency and QinQ options VLAN_PASSTHRU = 'vlan-passthru' +VLAN_ETHTYPE = 'ethtype' +# TODO(slaweq): handle somehow differences between version < 24.03 where +# 802.11ad was wrongly used and >= 24.03 where it was fixed and works with +# correct name 802.1ad +# according to +# https://github.com/ovn-org/ovn/commit/58dcbef8aace526d1ca57769ff1c38eff8db83be +# it seems that those old, wrong values are still accepted for now. Maybe they +# will not be in the future so we will need to update it then, +# I need to: +# - test locally if 802.11ad and 802.11q will be working fine in newer ovn, if +# yes, just add todo explaining that this may have to be changed in the future +ETHTYPE_8021q = '802.11q' +ETHTYPE_8021ad = '802.11ad' # OVN mechanism driver constants. OVN_RP_UUID = uuid.UUID('5533233b-800c-11eb-b1f4-000056b2f5b8') diff --git a/neutron/common/ovn/extensions.py b/neutron/common/ovn/extensions.py index 62197648bb1..884185cea69 100644 --- a/neutron/common/ovn/extensions.py +++ b/neutron/common/ovn/extensions.py @@ -66,6 +66,7 @@ from neutron_lib.api.definitions import portbindings from neutron_lib.api.definitions import portbindings_extended as pbe_ext from neutron_lib.api.definitions import project_id from neutron_lib.api.definitions import provider_net +from neutron_lib.api.definitions import qinq from neutron_lib.api.definitions import qos from neutron_lib.api.definitions import qos_bw_limit_direction from neutron_lib.api.definitions import qos_default @@ -165,6 +166,7 @@ ML2_SUPPORTED_API_EXTENSIONS = [ port_trusted_vif.ALIAS, provider_net.ALIAS, port_resource_request.ALIAS, + qinq.ALIAS, qos.ALIAS, qos_bw_limit_direction.ALIAS, qos_default.ALIAS, diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py index eac8d984f5b..a5830feef8b 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -227,7 +227,11 @@ class OVNMechanismDriver(api.MechanismDriver): def check_vlan_qinq(self, context): """OVN driver vlan QinQ support.""" - return False + vlan_qinq_network_types = [ + const.TYPE_VLAN, + ] + return (context.current.get(provider_net.NETWORK_TYPE) + in vlan_qinq_network_types) def _setup_vif_port_bindings(self): self.supported_vnic_types = ovn_const.OVN_SUPPORTED_VNIC_TYPES diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index a7e49d39037..aa1c989a63a 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -25,6 +25,7 @@ from neutron_lib.api.definitions import l3_ext_gw_multihoming from neutron_lib.api.definitions import port_security as psec from neutron_lib.api.definitions import portbindings from neutron_lib.api.definitions import provider_net as pnet +from neutron_lib.api.definitions import qinq as qinq_apidef from neutron_lib.api.definitions import segment as segment_def from neutron_lib import constants as const from neutron_lib import context as n_context @@ -2043,7 +2044,8 @@ class OVNClient: self._transaction(commands, txn=txn) - def create_provnet_port(self, network_id, segment, txn=None): + def create_provnet_port(self, network_id, segment, txn=None, + network=None): tag = segment.get(segment_def.SEGMENTATION_ID, []) physnet = segment.get(segment_def.PHYSICAL_NETWORK) fdb_enabled = ('true' if ovn_conf.is_learn_fdb_enabled() @@ -2055,6 +2057,15 @@ class OVNClient: ovn_const.LSP_OPTIONS_MCAST_FLOOD: ovs_conf.get_igmp_flood(), ovn_const.LSP_OPTIONS_LOCALNET_LEARN_FDB: fdb_enabled} + network = network or self._plugin.get_network( + n_context.get_admin_context(), network_id) + if self._get_vlan_passthru(network): + vlan_ethtype = self._get_vlan_ethtype(network) + if vlan_ethtype == ovn_const.ETHTYPE_8021ad: + # 802.1q ethtype is default so it needs to be set in the OVN + # db only if required value is 802.1ad + options[ovn_const.VLAN_ETHTYPE] = vlan_ethtype + cmd = self._nb_idl.create_lswitch_port( lport_name=utils.ovn_provnet_port_name(segment['id']), lswitch_name=utils.ovn_name(network_id), @@ -2072,6 +2083,14 @@ class OVNClient: lswitch_name=utils.ovn_name(network_id)) self._transaction([cmd]) + def _get_vlan_passthru(self, network): + return bool(network.get('vlan_transparent') or + network.get(qinq_apidef.QINQ_FIELD)) + + def _get_vlan_ethtype(self, network): + return (ovn_const.ETHTYPE_8021ad if network.get(qinq_apidef.QINQ_FIELD) + else ovn_const.ETHTYPE_8021q) + def _gen_network_parameters(self, network): params = {'external_ids': { ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: network['name'], @@ -2086,14 +2105,13 @@ class OVNClient: }} # Enable IGMP snooping if igmp_snooping_enable is enabled in Neutron - vlan_transparent = ( - 'true' if network.get('vlan_transparent') else 'false') params['other_config'] = { ovn_const.MCAST_SNOOP: ovs_conf.get_igmp_snooping_enabled(), ovn_const.MCAST_FLOOD_UNREGISTERED: ovs_conf.get_igmp_flood_unregistered(), - ovn_const.VLAN_PASSTHRU: vlan_transparent} + ovn_const.VLAN_PASSTHRU: str( + self._get_vlan_passthru(network)).lower()} if utils.is_provider_network(network): params['other_config'][ovn_const.LS_OPTIONS_FDB_AGE_THRESHOLD] = ( ovn_conf.get_fdb_age_threshold()) @@ -2120,7 +2138,8 @@ class OVNClient: may_exist=True)) for segment in segments: if segment.get(segment_def.PHYSICAL_NETWORK): - self.create_provnet_port(network['id'], segment, txn=txn) + self.create_provnet_port(network['id'], segment, txn=txn, + network=network) db_rev.bump_revision(context, network, ovn_const.TYPE_NETWORKS) self.create_metadata_port(context, network) return network diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py index c99b1d7d754..ab13020ef83 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py @@ -1213,7 +1213,7 @@ class OvnNbSynchronizer(OvnDbSynchronizer): 'OVN NB DB', utils.ovn_provnet_port_name(segment['id'])) self._ovn_client.create_provnet_port( - network['id'], segment, txn=txn) + network['id'], segment, txn=txn, network=network) for provnet_port_info in del_provnet_ports_list: network = provnet_port_info['network'] diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 1d8f7e920db..9cf53daca37 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -22,6 +22,8 @@ from unittest import mock import netaddr from neutron_lib.api.definitions import portbindings +from neutron_lib.api.definitions import qinq as qinq_apidef +from neutron_lib.api.definitions import vlantransparent as vlan_apidef from neutron_lib import constants from neutron_lib.db import api as db_api from neutron_lib.exceptions import agent as agent_exc @@ -33,6 +35,7 @@ from ovsdbapp.backend.ovs_idl import event from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import utils from neutron.common import utils as n_utils +from neutron.conf import common as common_conf from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf from neutron.db import ovn_hash_ring_db from neutron.db import ovn_revision_numbers_db as db_rev @@ -1048,6 +1051,89 @@ class TestProvnetPorts(base.TestOVNFunctionalBase): self.assertIsNone(ovn_localnetport) +class TestVlanTransparencyOptions(base.TestOVNFunctionalBase): + + """Tests for the vlan_transparent and vlan_qinq network params. + + This class contains tests which tests both "vlan_transparent" and + "vlan_qinq" options. The reason why those 2 are tested together is that + both options are using the same options on the OVN side and are mutually + exclusive. + """ + + def setUp(self): + common_conf.register_core_common_config_opts() + common_conf.cfg.CONF.set_override('vlan_qinq', True) + common_conf.cfg.CONF.set_override('vlan_transparent', True) + super().setUp() + self._ovn_client = self.mech_driver._ovn_client + + def _find_row_by_name(self, row_name, name): + cmd = self.nb_api.db_find_rows(row_name, ('name', '=', name)) + rows = cmd.execute(check_error=True) + return rows[0] if rows else None + + def _find_port_row_by_name(self, name): + return self._find_row_by_name('Logical_Switch_Port', name) + + def _find_network_row_by_name(self, name): + return self._find_row_by_name('Logical_Switch', name) + + def _test_network_with_qinq_and_vlan_transparent( + self, vlan_qinq, vlan_transparent): + net = self._make_network( + self.fmt, 'n1', True, as_admin=True, + arg_list=('provider:network_type', + 'provider:segmentation_id', + 'provider:physical_network', + qinq_apidef.QINQ_FIELD, + vlan_apidef.VLANTRANSPARENT), + **{'provider:network_type': 'vlan', + 'provider:segmentation_id': 100, + 'provider:physical_network': 'physnet1', + qinq_apidef.QINQ_FIELD: vlan_qinq, + vlan_apidef.VLANTRANSPARENT: vlan_transparent} + )['network'] + + seg_db = self.segments_plugin.get_segments( + self.context, filters={'network_id': [net['id']]}) + ovn_network = self._find_network_row_by_name(utils.ovn_name(net['id'])) + ovn_localnetport = self._find_port_row_by_name( + utils.ovn_provnet_port_name(seg_db[0]['id'])) + + if vlan_qinq: + self.assertEqual(ovn_const.ETHTYPE_8021ad, + ovn_localnetport.options[ovn_const.VLAN_ETHTYPE]) + # This means that "vlan-passthru" should be set to "true" for + # the LS + self.assertEqual('true', + ovn_network.other_config[ovn_const.VLAN_PASSTHRU]) + else: + self.assertNotIn(ovn_const.VLAN_ETHTYPE, ovn_localnetport.options) + if vlan_transparent: + # This means that "vlan-passthru" should be set to "true" for + # the LS, in case of vlan-transparent network there is no need + # to set ethtype on the localnet port as 802.1q is default + # value for this field in the OVN db + self.assertEqual( + 'true', ovn_network.other_config[ovn_const.VLAN_PASSTHRU]) + else: + self.assertEqual( + 'false', ovn_network.other_config[ovn_const.VLAN_PASSTHRU]) + + def test_network_with_qinq_enabled_vlan_transparent_disabled(self): + self._test_network_with_qinq_and_vlan_transparent( + True, False) + + def test_network_with_qinq_disabled_vlan_transparent_enabled(self): + self._test_network_with_qinq_and_vlan_transparent( + False, True) + + def test_network_with_qinq_and_vlan_transparent_disabled(self): + self._test_network_with_qinq_and_vlan_transparent( + False, False) + + class TestMetadataPorts(base.TestOVNFunctionalBase): def setUp(self, *args, **kwargs): diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py index 93b923b1388..bc253bca283 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py @@ -630,7 +630,8 @@ class TestOvnNbSyncML2(test_mech_driver.OVNMechanismDriverTestCase): mock.call( network['id'], self.segments_map[network['id']], - txn=mock.ANY) + txn=mock.ANY, + network=network) for network in create_provnet_port_list if network.get('provider:physical_network')] self.assertEqual( diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index ed7ab5c54aa..18eee5b2ed6 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -26,6 +26,7 @@ from neutron_lib.api.definitions import external_net from neutron_lib.api.definitions import extra_dhcp_opt as edo_ext from neutron_lib.api.definitions import portbindings from neutron_lib.api.definitions import provider_net as pnet +from neutron_lib.api.definitions import qinq as qinq_apidef from neutron_lib.callbacks import events from neutron_lib.callbacks import registry from neutron_lib.callbacks import resources @@ -178,6 +179,7 @@ class TestOVNMechanismDriverBase(MechDriverSetupBase, # setting up test_plugin config.register_common_config_options() cfg.CONF.set_override('vlan_transparent', True) + cfg.CONF.set_override('vlan_qinq', True) cfg.CONF.set_override('ovsdb_connection_timeout', 30, group='ovn') mock.patch.object(impl_idl_ovn.Backend, 'schema_helper').start() super().setUp() @@ -896,24 +898,41 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase): def test_create_network_igmp_snoop_disabled(self): self._create_network_igmp_snoop(enabled=False) - def _create_network_vlan_passthru(self, enabled): + def _create_network_vlan_passthru(self, vlan_transparent, qinq): nb_idl = self.mech_driver._ovn_client._nb_idl - net = self._make_network(self.fmt, name='net1', - admin_state_up=True, - vlan_transparent=enabled)['network'] - value = 'true' if enabled else 'false' + net = self._make_network( + self.fmt, name='net1', + as_admin=True, + admin_state_up=True, + arg_list=('provider:network_type', + 'provider:segmentation_id', + 'provider:physical_network', + qinq_apidef.QINQ_FIELD), + vlan_transparent=vlan_transparent, + qinq=qinq, + **{'provider:network_type': 'vlan', + 'provider:segmentation_id': 100, + 'provider:physical_network': 'physnet1'})['network'] + value = 'true' if vlan_transparent or qinq else 'false' + expected_fdb_age_treshold = ovn_conf.get_fdb_age_threshold() nb_idl.ls_add.assert_called_once_with( ovn_utils.ovn_name(net['id']), external_ids=mock.ANY, may_exist=True, - other_config={ovn_const.MCAST_SNOOP: 'false', - ovn_const.MCAST_FLOOD_UNREGISTERED: 'false', - ovn_const.VLAN_PASSTHRU: value}) + other_config={ + ovn_const.MCAST_SNOOP: 'false', + ovn_const.MCAST_FLOOD_UNREGISTERED: 'false', + ovn_const.LS_OPTIONS_FDB_AGE_THRESHOLD: + expected_fdb_age_treshold, + ovn_const.VLAN_PASSTHRU: value}) - def test_create_network_vlan_passthru_enabled(self): - self._create_network_vlan_passthru(enabled=True) + def test_create_network_vlan_passthru_vlan_transparent_enabled(self): + self._create_network_vlan_passthru(vlan_transparent=True, qinq=False) + + def test_create_network_vlan_passthru_qinq_enabled(self): + self._create_network_vlan_passthru(vlan_transparent=False, qinq=True) def test_create_network_vlan_passthru_disabled(self): - self._create_network_vlan_passthru(enabled=False) + self._create_network_vlan_passthru(vlan_transparent=False, qinq=False) def test_create_network_create_localnet_port_tunnel_network_type(self): nb_idl = self.mech_driver._ovn_client._nb_idl diff --git a/releasenotes/notes/add-qinq-attribute-to-the-network-resource-4a1ab8497dfd16b8.yaml b/releasenotes/notes/add-qinq-attribute-to-the-network-resource-4a1ab8497dfd16b8.yaml new file mode 100644 index 00000000000..f5ee9b993c3 --- /dev/null +++ b/releasenotes/notes/add-qinq-attribute-to-the-network-resource-4a1ab8497dfd16b8.yaml @@ -0,0 +1,15 @@ +--- +features: + - | + There is a new attribute ``qinq`` added to the ``network`` resource. This + new attribute can be used to enable for the network VLAN transparency using + ``802.1ad`` standard where outer tag (``S-Tag``) frame has ``0x8a88`` as + ethertype and the inner tag (``C-Tag``) has ``0x8100`` ethertype. + The behavior of this is very similar to the existing ``vlan_transparent`` + extension, with the difference in the standard and ethtypes used for the + outer tag. + The ``qinq`` parameter can be set only for the ``vlan`` networks and is + mutually exclusive with the ``vlan_transparent`` attribute as same network + can't have both of them enabled. + The ``qinq`` extension is currently supported only by the ``ML2/OVN`` + backend.