[OVN] Set always the GW LRP "gateway_mtu" option
NOTE: stable/2024.2 only, as this is a combination of two commits in one to not introduce a regression by doing them separately. There were small changes required to unit tests as the code had changed in 2025.1 where this was originally merged. First commit: If the configuration option "ovn.ovn_emit_need_to_frag" is set, write always the "LSP.options.gateway_mtu" value, using the minimum MTU value of all networks connected to the router. That will limit the N/S traffic MTU regardless if the gateway network MTU is greater or less than the internal network MTU. Conflicts: neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py neutron/tests/unit/services/ovn_l3/test_plugin.py (cherry picked from commitd080b35782) Second commit: Revert the network IDs filter in ``_gen_router_port_options`` In [1], a change in the ``network_ids`` filter was introduced. This filter is used to retrieve the networks belonging to a router. However this is used in two places: * To set the gateway MTU. Here all the networks attached to the router must be present. * To calculate the "redirect-type" value in the Logical_Router_Port. Only the internal interfaces must be present; the gateway port should be discarded. NOTE: patch [1] is present since 2025.1 (Epoxy). Conflicts: neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py [1]https://review.opendev.org/c/openstack/neutron/+/937026 Closes-Bug: #2105383 Related-Bug: #2032817 Change-Id: I9cd6e77e47ce16dcf2fc46fc076f1d7ee7a4f9d3 (cherry picked from commitdb03074927) (cherry picked from commit2489d8009e) Signed-off-by: Brian Haley <haleyb.dev@gmail.com>
This commit is contained in:
committed by
Brian Haley
parent
41b9dfa5d1
commit
197c920cdb
@@ -180,3 +180,21 @@ encapsulation) it's been included for completeness.
|
||||
Traffic goes directly from instance to instance through br-int in the case
|
||||
of both instances living in the same host (VM1 and VM2), or via
|
||||
encapsulation when living on different hosts (VM3 and VM4).
|
||||
|
||||
|
||||
Packet fragmentation
|
||||
~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
The Neutron configuration variable ``[ovn]ovn_emit_need_to_frag`` configures
|
||||
OVN to emit the "need to frag" packets in case of MTU mismatches. This
|
||||
configuration option allows Neutron to set, in the router gateway
|
||||
``Logical_Router_Port``, the option "gateway_mtu". If a packet from any
|
||||
network reaches the gateway ``Logical_Router_Port``, OVN will send the "need
|
||||
for frag" message.
|
||||
|
||||
In order to allow any E/W or N/S traffic to cross the router, the value of
|
||||
"gateway_mtu" will have the lowest MTU value off all networks connected to the
|
||||
router. This could impact the performance of the traffic using the networks
|
||||
connected to the router if the MTU defined is low. But the user can unset the
|
||||
Neutron configuration flag in order to avoid the fragmentation, at the cost
|
||||
of limiting the communication between networks with different MTUs.
|
||||
|
||||
@@ -1698,14 +1698,12 @@ class OVNClient(object):
|
||||
for net in networks) else 'false'
|
||||
return reside_redir_ch
|
||||
|
||||
def _gen_router_port_options(self, port, network_mtu=None):
|
||||
def _gen_router_port_options(self, port):
|
||||
options = {}
|
||||
admin_context = n_context.get_admin_context()
|
||||
ls_name = utils.ovn_name(port['network_id'])
|
||||
ls = self._nb_idl.ls_get(ls_name).execute(check_error=True)
|
||||
network_type = ls.external_ids[ovn_const.OVN_NETTYPE_EXT_ID_KEY]
|
||||
network_mtu = network_mtu or int(
|
||||
ls.external_ids[ovn_const.OVN_NETWORK_MTU_EXT_ID_KEY])
|
||||
# For provider networks (VLAN, FLAT types) we need to set the
|
||||
# "reside-on-redirect-chassis" option so the routing for this
|
||||
# logical router port is centralized in the chassis hosting the
|
||||
@@ -1729,15 +1727,16 @@ class OVNClient(object):
|
||||
LOG.debug("Router %s not found", port['device_id'])
|
||||
else:
|
||||
network_ids = {port['network_id'] for port in router_ports}
|
||||
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 this method is called during a port creation, the port
|
||||
# won't be present yet in the router ports list. It is
|
||||
# needed not to modify the ``network_ids`` set.
|
||||
_network_ids = network_ids.union({port['network_id']})
|
||||
networks = self._plugin.get_networks(
|
||||
admin_context, filters={'id': _network_ids})
|
||||
# Set the lower MTU of all networks connected to the router
|
||||
min_mtu = str(min(net['mtu'] for net in networks))
|
||||
options[ovn_const.OVN_ROUTER_PORT_GW_MTU_OPTION] = min_mtu
|
||||
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
|
||||
@@ -1746,6 +1745,8 @@ class OVNClient(object):
|
||||
# If there are no VLAN type networks attached we need to
|
||||
# still make it centralized.
|
||||
enable_redirect = False
|
||||
networks = self._plugin.get_networks(
|
||||
admin_context, filters={'id': network_ids})
|
||||
if networks:
|
||||
enable_redirect = all(
|
||||
net.get(pnet.NETWORK_TYPE) in [const.TYPE_VLAN,
|
||||
@@ -2144,8 +2145,7 @@ class OVNClient(object):
|
||||
commands = []
|
||||
for port in ports:
|
||||
lrp_name = utils.ovn_lrouter_port_name(port['id'])
|
||||
options = self._gen_router_port_options(
|
||||
port, network_mtu=prov_net['mtu'])
|
||||
options = self._gen_router_port_options(port)
|
||||
# Do not fail for cases where logical router port get deleted
|
||||
commands.append(self._nb_idl.lrp_set_options(lrp_name,
|
||||
if_exists=True,
|
||||
|
||||
@@ -186,21 +186,21 @@ class TestOVNClient(testlib_api.MySQLTestCaseMixin,
|
||||
def test_router_reside_chassis_redirect_non_dvr_geneve_net(self):
|
||||
self._test_router_reside_chassis_redirect(False, 'geneve')
|
||||
|
||||
def _check_gw_lrp_mtu(self, router_id, mtu):
|
||||
# Find gateway LRP and check the MTU value.
|
||||
lr = self.nb_api.lookup('Logical_Router',
|
||||
ovn_utils.ovn_name(router_id))
|
||||
for lrp in lr.ports:
|
||||
if strutils.bool_from_string(
|
||||
lrp.external_ids[
|
||||
ovn_const.OVN_ROUTER_IS_EXT_GW]):
|
||||
self.assertEqual(mtu, int(lrp.options['gateway_mtu']))
|
||||
return
|
||||
|
||||
self.fail('Gateway Logical_Router_Port not found for '
|
||||
'router %s' % router_id)
|
||||
|
||||
def test_update_network_lrp_mtu_updated(self):
|
||||
def check_gw_lrp_mtu(router_id, mtu):
|
||||
# Find gateway LRP and check the MTU value.
|
||||
lr = self.nb_api.lookup('Logical_Router',
|
||||
ovn_utils.ovn_name(router_id))
|
||||
for lrp in lr.ports:
|
||||
if strutils.bool_from_string(
|
||||
lrp.external_ids[
|
||||
ovn_const.OVN_ROUTER_IS_EXT_GW]):
|
||||
self.assertEqual(mtu, int(lrp.options['gateway_mtu']))
|
||||
return
|
||||
|
||||
self.fail('Gateway Logical_Router_Port not found for '
|
||||
'router %s' % router_id)
|
||||
|
||||
cfg.CONF.set_override('ovn_emit_need_to_frag', True, group='ovn')
|
||||
self.add_fake_chassis('host1', enable_chassis_as_gw=True, azs=[])
|
||||
net_ext_args = {provider_net.NETWORK_TYPE: 'geneve',
|
||||
@@ -224,14 +224,56 @@ class TestOVNClient(testlib_api.MySQLTestCaseMixin,
|
||||
'add', router_id, snet_int['subnet']['id'],
|
||||
None)
|
||||
|
||||
check_gw_lrp_mtu(router_id, 1300)
|
||||
self._check_gw_lrp_mtu(router_id, 1300)
|
||||
|
||||
# Update external network MTU.
|
||||
net_ext_args = {'network': {mtu_def.MTU: 1350}}
|
||||
req = self.new_update_request('networks', net_ext_args,
|
||||
net_ext['network']['id'])
|
||||
req.get_response(self.api)
|
||||
check_gw_lrp_mtu(router_id, 1350)
|
||||
self._check_gw_lrp_mtu(router_id, 1350)
|
||||
|
||||
def test_add_new_router_interfaces_lrp_mtu_updated(self):
|
||||
mtu_ext = 1350
|
||||
internal_interfaces = [{'cidr': '10.2.0.0/24', 'mtu': mtu_ext - 10},
|
||||
{'cidr': '10.3.0.0/24', 'mtu': mtu_ext + 10},
|
||||
{'cidr': '10.4.0.0/24', 'mtu': mtu_ext - 20}]
|
||||
cfg.CONF.set_override('ovn_emit_need_to_frag', True, group='ovn')
|
||||
self.add_fake_chassis('host1', enable_chassis_as_gw=True, azs=[])
|
||||
net_ext_args = {provider_net.NETWORK_TYPE: 'geneve',
|
||||
external_net.EXTERNAL: True,
|
||||
mtu_def.MTU: mtu_ext}
|
||||
# Store the network's MTUs connected to the router.
|
||||
router_attached_net_mtus = {mtu_ext}
|
||||
with self.network(
|
||||
'test-ext-net', as_admin=True,
|
||||
arg_list=tuple(net_ext_args.keys()), **net_ext_args) as \
|
||||
net_ext:
|
||||
ext_gw = {'network_id': net_ext['network']['id']}
|
||||
with self.subnet(net_ext, cidr='10.1.0.0/24'), \
|
||||
self.router(external_gateway_info=ext_gw) as router:
|
||||
router_id = router['router']['id']
|
||||
for _int in internal_interfaces:
|
||||
# Attach a new internal interface to the router. If the new
|
||||
# network MTU is lower than the minimum MTU connected, the
|
||||
# GW LRP.options.gateway_mtu will be updated.
|
||||
net_int_args = {provider_net.NETWORK_TYPE: 'geneve',
|
||||
mtu_def.MTU: _int['mtu']}
|
||||
router_attached_net_mtus.add(_int['mtu'])
|
||||
net_int = self._make_network(
|
||||
self.fmt, name='nettest', admin_state_up=True,
|
||||
**net_int_args)
|
||||
snet_int = self._make_subnet(
|
||||
self.fmt, net_int, constants.ATTR_NOT_SPECIFIED,
|
||||
_int['cidr'])
|
||||
self._router_interface_action(
|
||||
'add', router_id, snet_int['subnet']['id'],
|
||||
None)
|
||||
|
||||
# The GW LRP "gateway_mtu" value should be the minimum
|
||||
# MTU connected to the router.
|
||||
self._check_gw_lrp_mtu(router_id,
|
||||
min(router_attached_net_mtus))
|
||||
|
||||
def test_process_address_group(self):
|
||||
def _find_address_set_for_ag():
|
||||
|
||||
@@ -60,6 +60,7 @@ from neutron.db import ovn_revision_numbers_db
|
||||
from neutron.db import provisioning_blocks
|
||||
from neutron.db import securitygroups_db
|
||||
from neutron.db import segments_db
|
||||
from neutron.objects import network as network_obj
|
||||
from neutron.plugins.ml2.drivers.ovn.agent import neutron_agent
|
||||
from neutron.plugins.ml2.drivers.ovn.mech_driver import mech_driver
|
||||
from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import impl_idl_ovn
|
||||
@@ -2664,6 +2665,12 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
|
||||
with self.subnet(network=network) as subnet:
|
||||
with self.port(subnet=subnet,
|
||||
device_owner=const.DEVICE_OWNER_ROUTER_GW) as port:
|
||||
# Manually set the network DB register MTU without calling
|
||||
# the network update API.
|
||||
net_obj = network_obj.Network.get_object(
|
||||
self.context, id=network['network']['id'])
|
||||
net_obj.mtu = new_mtu
|
||||
net_obj.update()
|
||||
|
||||
grps.return_value = [{'port_id': port['port']['id'],
|
||||
'network_id':network['network']['id']}]
|
||||
|
||||
@@ -76,7 +76,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
|
||||
revision_plugin.RevisionPlugin()
|
||||
# MTU needs to be 1442 instead of 1500 because GENEVE headers size
|
||||
# must be at least 38 when using OVN
|
||||
network_attrs = {external_net.EXTERNAL: True, 'mtu': 1442}
|
||||
default_mtu = 1442
|
||||
network_attrs = {external_net.EXTERNAL: True, 'mtu': default_mtu}
|
||||
self.fake_network = \
|
||||
fake_resources.FakeNetwork.create_one_network(
|
||||
attrs=network_attrs).info()
|
||||
@@ -175,7 +176,10 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
|
||||
ovn_const.OVN_ROUTER_IS_EXT_GW: 'True',
|
||||
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'router-id',
|
||||
},
|
||||
'options': {}}
|
||||
'options': {
|
||||
ovn_const.OVN_ROUTER_PORT_GW_MTU_OPTION: str(default_mtu)
|
||||
}
|
||||
}
|
||||
self.fake_floating_ip_attrs = {'floating_ip_address': '192.168.0.10',
|
||||
'fixed_ip_address': '10.0.0.10'}
|
||||
self.fake_floating_ip = fake_resources.FakeFloatingIp.create_one_fip(
|
||||
@@ -261,6 +265,9 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
|
||||
self._get_network = self._start_mock(
|
||||
'neutron.plugins.ml2.plugin.Ml2Plugin.get_network',
|
||||
return_value=self.fake_network)
|
||||
self._get_networks = self._start_mock(
|
||||
'neutron.plugins.ml2.plugin.Ml2Plugin.get_networks',
|
||||
return_value=[self.fake_network])
|
||||
self.get_port = self._start_mock(
|
||||
'neutron.db.db_base_plugin_v2.NeutronDbPluginV2.get_port',
|
||||
return_value=self.fake_router_port)
|
||||
@@ -631,6 +638,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
|
||||
@mock.patch('neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.'
|
||||
'ovn_client.OVNClient._get_v4_network_of_all_router_ports')
|
||||
def test_create_router_with_ext_gw(self, get_rps):
|
||||
self.fake_ext_gw_port_assert['options'] = {}
|
||||
self._get_network.return_value = self.fake_ext_network
|
||||
self.l3_inst._nb_ovn.is_col_present.return_value = True
|
||||
self.get_subnet.return_value = self.fake_ext_subnet
|
||||
@@ -817,6 +825,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
|
||||
@mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.'
|
||||
'update_router')
|
||||
def test_update_router_with_ext_gw(self, ur, grps):
|
||||
self.fake_ext_gw_port_assert['options'] = {}
|
||||
self._get_network.return_value = self.fake_ext_network
|
||||
self.l3_inst._nb_ovn.is_col_present.return_value = True
|
||||
ur.return_value = self.fake_router_with_ext_gw
|
||||
@@ -858,6 +867,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
|
||||
'update_router')
|
||||
def test_update_router_ext_gw_change_subnet(self, ur,
|
||||
grps, mock_get_gw):
|
||||
self.fake_ext_gw_port_assert['options'] = {}
|
||||
self._get_network.return_value = self.fake_ext_network
|
||||
self.l3_inst._nb_ovn.is_col_present.return_value = True
|
||||
mock_get_gw.return_value = [mock.sentinel.GwRoute]
|
||||
@@ -933,6 +943,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
|
||||
'update_router')
|
||||
def test_update_router_ext_gw_change_ip_address(self, ur,
|
||||
grps, mock_get_gw):
|
||||
self.fake_ext_gw_port_assert['options'] = {}
|
||||
self._get_network.return_value = self.fake_ext_network
|
||||
self.l3_inst._nb_ovn.is_col_present.return_value = True
|
||||
mock_get_gw.return_value = [mock.sentinel.GwRoute]
|
||||
@@ -1935,7 +1946,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
|
||||
self.fake_router_port['device_owner'] = (
|
||||
constants.DEVICE_OWNER_ROUTER_GW)
|
||||
gn.return_value = prov_net
|
||||
gns.return_value = [self.fake_network]
|
||||
gns.return_value = [self.fake_network, prov_net]
|
||||
ext_ids = {
|
||||
ovn_const.OVN_NETTYPE_EXT_ID_KEY: constants.TYPE_GENEVE,
|
||||
ovn_const.OVN_NETWORK_MTU_EXT_ID_KEY: mtu,
|
||||
@@ -1971,6 +1982,63 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
|
||||
mock.ANY, self.fake_router_port,
|
||||
ovn_const.TYPE_ROUTER_PORTS)
|
||||
|
||||
@mock.patch('neutron.plugins.ml2.plugin.Ml2Plugin.get_network')
|
||||
@mock.patch('neutron.plugins.ml2.plugin.Ml2Plugin.get_networks')
|
||||
@mock.patch('neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.'
|
||||
'ovn_client.OVNClient._get_router_ports')
|
||||
@mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin.add_router_interface')
|
||||
def test_add_router_interface_need_to_frag_disabled(
|
||||
self, ari, grps, gns, gn):
|
||||
config.cfg.CONF.set_override(
|
||||
'ovn_emit_need_to_frag', False, group='ovn')
|
||||
router_id = 'router-id'
|
||||
interface_info = {'port_id': 'router-port-id',
|
||||
'network_id': 'priv-net'}
|
||||
ari.return_value = self.fake_router_interface_info
|
||||
grps.return_value = [interface_info]
|
||||
self.get_router.return_value = self.fake_router_with_ext_gw
|
||||
mtu = 1200
|
||||
network_attrs = {'id': 'prov-net', 'mtu': 1200,
|
||||
'provider:network_type': 'vlan',
|
||||
'provider:physical_network': 'physnet1'}
|
||||
prov_net = fake_resources.FakeNetwork.create_one_network(
|
||||
attrs=network_attrs).info()
|
||||
self.fake_router_port['device_owner'] = (
|
||||
constants.DEVICE_OWNER_ROUTER_GW)
|
||||
gn.return_value = prov_net
|
||||
gns.return_value = [self.fake_network, prov_net]
|
||||
ext_ids = {
|
||||
ovn_const.OVN_NETTYPE_EXT_ID_KEY: constants.TYPE_GENEVE,
|
||||
ovn_const.OVN_NETWORK_MTU_EXT_ID_KEY: mtu,
|
||||
}
|
||||
self.l3_inst._nb_ovn.ls_get.return_value.execute.return_value = (
|
||||
mock.Mock(external_ids=ext_ids))
|
||||
|
||||
payload = self._create_payload_for_router_interface(router_id)
|
||||
self.ovn_drv._process_add_router_interface(resources.ROUTER_INTERFACE,
|
||||
events.AFTER_CREATE,
|
||||
self, payload)
|
||||
|
||||
# Make sure that the "gateway_mtu" option was not set to router port
|
||||
fake_router_port_assert = self.fake_router_port_assert
|
||||
fake_router_port_assert['options'] = {}
|
||||
fake_router_port_assert['external_ids'][
|
||||
ovn_const.OVN_ROUTER_IS_EXT_GW] = 'True'
|
||||
|
||||
self.l3_inst._nb_ovn.add_lrouter_port.assert_called_once_with(
|
||||
**fake_router_port_assert)
|
||||
self.l3_inst._nb_ovn.set_lrouter_port_in_lswitch_port.\
|
||||
assert_called_once_with(
|
||||
'router-port-id', 'lrp-router-port-id', is_gw_port=True,
|
||||
lsp_address=ovn_const.DEFAULT_ADDR_FOR_LSP_WITH_PEER)
|
||||
self.l3_inst._nb_ovn.add_nat_rule_in_lrouter.assert_called_once_with(
|
||||
'neutron-router-id', logical_ip='10.0.0.0/24',
|
||||
external_ip='192.168.1.1', type='snat')
|
||||
|
||||
self.bump_rev_p.assert_called_with(
|
||||
mock.ANY, self.fake_router_port,
|
||||
ovn_const.TYPE_ROUTER_PORTS)
|
||||
|
||||
@mock.patch('neutron.plugins.ml2.plugin.Ml2Plugin.get_network')
|
||||
@mock.patch('neutron.plugins.ml2.plugin.Ml2Plugin.get_networks')
|
||||
@mock.patch('neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.'
|
||||
|
||||
@@ -0,0 +1,7 @@
|
||||
---
|
||||
features:
|
||||
- |
|
||||
Now if the configuration option ``[ovn]ovn_emit_need_to_frag`` is set,
|
||||
OVN will always set the "gateway_mtu" option in the gateway
|
||||
``Logical_Router_Port``. The value defined will be the lowest MTU of all
|
||||
networks connected to this router.
|
||||
Reference in New Issue
Block a user