diff options
5 files changed, 226 insertions, 19 deletions
diff --git a/neutron/common/ovn/constants.py b/neutron/common/ovn/constants.py index 4b4d94ff24..2a4afbf4a9 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -393,6 +393,8 @@ LSP_OPTIONS_MCAST_FLOOD = 'mcast_flood' LSP_OPTIONS_QOS_MIN_RATE = 'qos_min_rate' LRP_OPTIONS_RESIDE_REDIR_CH = 'reside-on-redirect-chassis' +LRP_OPTIONS_REDIRECT_TYPE = 'redirect-type' +BRIDGE_REDIRECT_TYPE = "bridged" # Port Binding types PB_TYPE_VIRTUAL = 'virtual' diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py index b05e22ab12..29a3848b28 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -25,6 +25,7 @@ from neutron_lib import constants as n_const from neutron_lib import context as n_context from neutron_lib.db import api as db_api from neutron_lib import exceptions as n_exc +from neutron_lib.exceptions import l3 as l3_exc from oslo_config import cfg from oslo_log import log from oslo_serialization import jsonutils @@ -677,6 +678,66 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): # A static spacing value is used here, but this method will only run # once per lock due to the use of periodics.NeverAgain(). @periodics.periodic(spacing=600, run_immediately=True) + def check_redirect_type_router_gateway_ports(self): + """Check OVN router gateway ports + Check for the option "redirect-type=bridged" value for + router gateway ports. + """ + if not self.has_lock: + return + context = n_context.get_admin_context() + cmds = [] + gw_ports = self._ovn_client._plugin.get_ports( + context, {'device_owner': [n_const.DEVICE_OWNER_ROUTER_GW]}) + for gw_port in gw_ports: + enable_redirect = False + if ovn_conf.is_ovn_distributed_floating_ip(): + try: + r_ports = self._ovn_client._get_router_ports( + context, gw_port['device_id']) + except l3_exc.RouterNotFound: + LOG.debug("No Router %s not found", gw_port['device_id']) + continue + else: + network_ids = {port['network_id'] for port in r_ports} + networks = self._ovn_client._plugin.get_networks( + context, filters={'id': network_ids}) + # NOTE(ltomasbo): For VLAN type networks connected through + # the gateway port there is a need to set the redirect-type + # option to bridge to ensure traffic is not centralized + # through the controller. + # If there are no VLAN type networks attached we need to + # still make it centralized. + if networks: + enable_redirect = all( + net.get(pnet.NETWORK_TYPE) in [n_const.TYPE_VLAN, + n_const.TYPE_FLAT] + for net in networks) + + lrp_name = utils.ovn_lrouter_port_name(gw_port['id']) + lrp = self._nb_idl.get_lrouter_port(lrp_name) + redirect_value = lrp.options.get( + ovn_const.LRP_OPTIONS_REDIRECT_TYPE) + if enable_redirect: + if redirect_value != ovn_const.BRIDGE_REDIRECT_TYPE: + opt = {ovn_const.LRP_OPTIONS_REDIRECT_TYPE: + ovn_const.BRIDGE_REDIRECT_TYPE} + cmds.append(self._nb_idl.db_set( + 'Logical_Router_Port', lrp_name, ('options', opt))) + else: + if redirect_value == ovn_const.BRIDGE_REDIRECT_TYPE: + cmds.append(self._nb_idl.db_remove( + 'Logical_Router_Port', lrp_name, 'options', + (ovn_const.LRP_OPTIONS_REDIRECT_TYPE))) + if cmds: + with self._nb_idl.transaction(check_error=True) as txn: + for cmd in cmds: + txn.add(cmd) + raise periodics.NeverAgain() + + # A static spacing value is used here, but this method will only run + # once per lock due to the use of periodics.NeverAgain(). + @periodics.periodic(spacing=600, run_immediately=True) def check_vlan_distributed_ports(self): """Check VLAN distributed ports Check for the option "reside-on-redirect-chassis" value for @@ -696,9 +757,11 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): router_ports = self._ovn_client._plugin.get_ports( context, {'network_id': vlan_net_ids, 'device_owner': n_const.ROUTER_PORT_OWNERS}) - expected_value = ('false' if ovn_conf.is_ovn_distributed_floating_ip() - else 'true') + for rp in router_ports: + expected_value = ( + self._ovn_client._get_reside_redir_for_gateway_port( + rp['device_id'])) lrp_name = utils.ovn_lrouter_port_name(rp['id']) lrp = self._nb_idl.get_lrouter_port(lrp_name) if lrp.options.get( 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 8ae0f16d36..f5607e2849 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 @@ -1324,6 +1324,11 @@ class OVNClient(object): nexthop=route['nexthop'])) self._transaction(commands, txn=txn) + def _get_router_gw_ports(self, context, router_id): + return self._plugin.get_ports(context, filters={ + 'device_owner': [const.DEVICE_OWNER_ROUTER_GW], + 'device_id': [router_id]}) + def _get_router_ports(self, context, router_id, get_gw_port=False): # _get_router() will raise a RouterNotFound error if there's no router # with the router_id @@ -1567,6 +1572,31 @@ class OVNClient(object): return ext_ids + def _get_reside_redir_for_gateway_port(self, device_id): + admin_context = n_context.get_admin_context() + reside_redir_ch = 'true' + if ovn_conf.is_ovn_distributed_floating_ip(): + reside_redir_ch = 'false' + try: + router_ports = self._get_router_ports(admin_context, device_id) + except l3_exc.RouterNotFound: + LOG.debug("No Router %s not found", device_id) + else: + network_ids = {port['network_id'] for port in router_ports} + networks = self._plugin.get_networks( + admin_context, filters={'id': network_ids}) + + # NOTE(ltomasbo): not all the networks connected to the router + # are of vlan type, so we won't set the redirect-type=bridged + # on the router gateway port, therefore we need to centralized + # the vlan traffic to avoid tunneling + if networks: + reside_redir_ch = 'true' if any( + net.get(pnet.NETWORK_TYPE) not in [const.TYPE_VLAN, + const.TYPE_FLAT] + for net in networks) else 'false' + return reside_redir_ch + def _gen_router_port_options(self, port, network=None): options = {} admin_context = n_context.get_admin_context() @@ -1581,14 +1611,15 @@ class OVNClient(object): # FIXME(ltomasbo): Once Bugzilla 2162756 is fixed the # is_provider_network check should be removed if network.get(pnet.NETWORK_TYPE) == const.TYPE_VLAN: - options[ovn_const.LRP_OPTIONS_RESIDE_REDIR_CH] = ( - 'false' if (ovn_conf.is_ovn_distributed_floating_ip() and - not utils.is_provider_network(network)) - else 'true') + reside_redir_ch = self._get_reside_redir_for_gateway_port( + port['device_id']) + options[ovn_const.LRP_OPTIONS_RESIDE_REDIR_CH] = reside_redir_ch is_gw_port = const.DEVICE_OWNER_ROUTER_GW == port.get( 'device_owner') - if is_gw_port and ovn_conf.is_ovn_emit_need_to_frag_enabled(): + + if is_gw_port and (ovn_conf.is_ovn_distributed_floating_ip() or + ovn_conf.is_ovn_emit_need_to_frag_enabled()): try: router_ports = self._get_router_ports(admin_context, port['device_id']) @@ -1597,12 +1628,32 @@ class OVNClient(object): LOG.debug("Router %s not found", port['device_id']) else: network_ids = {port['network_id'] for port in router_ports} - for net in self._plugin.get_networks( - admin_context, filters={'id': network_ids}): - if net['mtu'] > network['mtu']: - options[ovn_const.OVN_ROUTER_PORT_GW_MTU_OPTION] = str( - network['mtu']) - break + networks = self._plugin.get_networks( + admin_context, filters={'id': network_ids}) + if ovn_conf.is_ovn_emit_need_to_frag_enabled(): + for net in networks: + if net['mtu'] > network['mtu']: + options[ + ovn_const.OVN_ROUTER_PORT_GW_MTU_OPTION] = str( + network['mtu']) + break + if ovn_conf.is_ovn_distributed_floating_ip(): + # NOTE(ltomasbo): For VLAN type networks connected through + # the gateway port there is a need to set the redirect-type + # option to bridge to ensure traffic is not centralized + # through the controller. + # If there are no VLAN type networks attached we need to + # still make it centralized. + enable_redirect = False + if networks: + enable_redirect = all( + net.get(pnet.NETWORK_TYPE) in [const.TYPE_VLAN, + const.TYPE_FLAT] + for net in networks) + if enable_redirect: + options[ovn_const.LRP_OPTIONS_REDIRECT_TYPE] = ( + ovn_const.BRIDGE_REDIRECT_TYPE) + return options def _create_lrouter_port(self, context, router, port, txn=None): @@ -1683,6 +1734,12 @@ class OVNClient(object): if utils.is_snat_enabled(router) and cidr: self.update_nat_rules(router, networks=[cidr], enable_snat=True, txn=txn) + if ovn_conf.is_ovn_distributed_floating_ip(): + router_gw_ports = self._get_router_gw_ports(context, + router_id) + for router_port in router_gw_ports: + self._update_lrouter_port(context, router_port, + txn=txn) db_rev.bump_revision(context, port, ovn_const.TYPE_ROUTER_PORTS) @@ -1823,6 +1880,11 @@ class OVNClient(object): self.update_nat_rules( router, networks=[cidr], enable_snat=False, txn=txn) + if ovn_conf.is_ovn_distributed_floating_ip(): + router_gw_ports = self._get_router_gw_ports(context, router_id) + for router_port in router_gw_ports: + self._update_lrouter_port(context, router_port, txn=txn) + # NOTE(mangelajo): If the port doesn't exist anymore, we # delete the router port as the last operation and update the # revision database to ensure consistency diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index 4391102074..b22e0a5470 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -614,16 +614,76 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, 'dynamic_neigh_routers': 'true'})] nb_idl.update_lrouter.assert_has_calls(expected_calls) + def _test_check_redirect_type_router_gateway_ports(self, networks, + redirect_value): + self.fake_ovn_client._plugin.get_ports.return_value = [{ + 'device_owner': n_const.DEVICE_OWNER_ROUTER_GW, + 'id': 'fake-id', + 'device_id': 'fake-device-id'}] + self.fake_ovn_client._get_router_ports.return_value = [] + self.fake_ovn_client._plugin.get_networks.return_value = networks + + lrp_redirect = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={ + 'options': {constants.LRP_OPTIONS_REDIRECT_TYPE: "bridged"}}) + lrp_no_redirect = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={ + 'options': {}}) + + # set the opossite so that the value is changed + if redirect_value: + self.fake_ovn_client._nb_idl.get_lrouter_port.return_value = ( + lrp_no_redirect) + else: + self.fake_ovn_client._nb_idl.get_lrouter_port.return_value = ( + lrp_redirect) + + self.assertRaises( + periodics.NeverAgain, + self.periodic.check_redirect_type_router_gateway_ports) + + if redirect_value: + expected_calls = [ + mock.call.db_set('Logical_Router_Port', + mock.ANY, + ('options', {'redirect-type': 'bridged'})) + ] + self.fake_ovn_client._nb_idl.db_set.assert_has_calls( + expected_calls) + else: + expected_calls = [ + mock.call.db_remove('Logical_Router_Port', mock.ANY, + 'options', 'redirect-type') + ] + self.fake_ovn_client._nb_idl.db_remove.assert_has_calls( + expected_calls) + + def test_check_redirect_type_router_gateway_ports_enable_redirect(self): + cfg.CONF.set_override('enable_distributed_floating_ip', 'True', + group='ovn') + networks = [{'network_id': 'foo', + 'provider:network_type': n_const.TYPE_VLAN}] + self._test_check_redirect_type_router_gateway_ports(networks, True) + + def test_check_redirect_type_router_gateway_ports_disable_redirect(self): + cfg.CONF.set_override('enable_distributed_floating_ip', 'True', + group='ovn') + networks = [{'network_id': 'foo', + 'provider:network_type': n_const.TYPE_GENEVE}] + self._test_check_redirect_type_router_gateway_ports(networks, False) + def _test_check_vlan_distributed_ports(self, opt_value=None): fake_net0 = {'id': 'net0'} fake_net1 = {'id': 'net1'} - fake_port0 = {'id': 'port0'} - fake_port1 = {'id': 'port1'} + fake_port0 = {'id': 'port0', 'device_id': 'device0'} + fake_port1 = {'id': 'port1', 'device_id': 'device1'} self.fake_ovn_client._plugin.get_networks.return_value = [ fake_net0, fake_net1] self.fake_ovn_client._plugin.get_ports.return_value = [ fake_port0, fake_port1] + (self.fake_ovn_client._get_reside_redir_for_gateway_port + .return_value) = 'true' fake_lrp = fakes.FakeOvsdbRow.create_one_ovsdb_row( attrs={ @@ -637,8 +697,6 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, self.periodic.check_vlan_distributed_ports) def test_check_vlan_distributed_ports_expected_value(self): - cfg.CONF.set_override('enable_distributed_floating_ip', 'False', - group='ovn') self._test_check_vlan_distributed_ports(opt_value='true') # If the "reside-on-redirect-chassis" option value do match @@ -647,8 +705,6 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, self.fake_ovn_client._nb_idl.db_set.called) def test_check_vlan_distributed_ports_non_expected_value(self): - cfg.CONF.set_override('enable_distributed_floating_ip', 'False', - group='ovn') self._test_check_vlan_distributed_ports(opt_value='false') # If the "reside-on-redirect-chassis" option value does not match diff --git a/releasenotes/notes/redirect-type-f29e89ca97357fe9.yaml b/releasenotes/notes/redirect-type-f29e89ca97357fe9.yaml new file mode 100644 index 0000000000..3d5661b22c --- /dev/null +++ b/releasenotes/notes/redirect-type-f29e89ca97357fe9.yaml @@ -0,0 +1,24 @@ +--- +issues: + - | + The `redirect-type=bridged` option is only used if all the tenant networks + connected to the router are of type VLAN or FLAT. In this case their + traffic will be distributed. However, if there is a mix of VLAN/FLAT and + geneve networks connected to the same router, the redirect-type option is + not set, and therefore the traffic for the VLAN/FLAT networks will also be + centralized but not tunneled. +fixes: + - | + [`bug 2003455 <https://bugs.launchpad.net/neutron/+bug/2003455>`_] + As part of a previous commit + (https://review.opendev.org/c/openstack/neutron/+/875644) the + `redirect-type=bridged` option was set in all the router gateway ports + (cr-lrp ovn ports). However this was breaking the N/S traffic for geneve + tenant networks connected to the provider networks through those routers + with the redirect-type option enabled. To fix this we ensure that the + redirect-type option is only set if all the networks connected to the + router are of VLAN or FLAT type, otherwise we fall back to the default + option. This also means that if there is a mix of VLAN and geneve tenant + networks connected to the same router, the VLAN traffic will be centralized + (but not tunneled). If the traffic for the VLAN/FLAT needs to be + distributed, then it should use a different router. |