openstack/neutron
Code Issues Proposed changes

Ensure redirect-type=bridged not used for geneve networks

Browse Source 
As part of [1] the redirect-type=bridged flag was enabled by default.
However this have the side effect of also decentralizing N/S traffic
for geneve tenant networks, breaking the VM connectivity on them when
it must be centralized, i.e., when no FIPs are associated to the VMs.

This patch differentiates and only enable that flag when the networks
conected through that router gateway port are of VLAN/FLAT type.

In addition, to avoid MTU issues for the VLAN networks if there are
also geneve networks connected to the same router, we re-take the
approach on [2] to ensure the traffic is centralized but not tunneled

[1] https://review.opendev.org/c/openstack/neutron/+/875644
[2] https://review.opendev.org/c/openstack/neutron/+/875676

Closes-Bug: #2012712

Conflicts:
    neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py
    neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py

Change-Id: I25e5ee2cf8daee52221a640faa7ac09679742707
(cherry picked from commit 0ec04dd638)
This commit is contained in:
Luis Tomas Bolivar 2023-03-23 16:45:18 +01:00
parent 9bbb97f635
commit c246913b2f
5 changed files with 227 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
neutron/common/ovn/constants.py
View File

@ -372,6 +372,8 @@ LSP_OPTIONS_MCAST_FLOOD_REPORTS = 'mcast_flood_reports'
LSP_OPTIONS_MCAST_FLOOD = 'mcast_flood'
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'

67
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py
View File

@ -27,6 +27,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_utils import timeutils
@ -816,6 +817,66 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
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_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)
@ -838,9 +899,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(

78
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py
View File

@ -1317,6 +1317,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
@ -1548,6 +1553,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()
@ -1562,14 +1592,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'])
@ -1578,12 +1609,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}):
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(
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):
@ -1664,6 +1715,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)
@ -1804,6 +1861,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

69
neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py
View File

@ -16,6 +16,7 @@
from unittest import mock
from futurist import periodics
from neutron_lib import constants as n_const
from neutron_lib import context
from neutron_lib.db import api as db_api
from oslo_config import cfg
@ -622,16 +623,76 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight,
('external_ids', external_ids))]
nb_idl.db_set.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={
@ -645,8 +706,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
@ -655,8 +714,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

24
releasenotes/notes/redirect-type-f29e89ca97357fe9.yaml Normal file
View File

@ -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.