Merge pull request #40 from stackhpc/upstream/yoga-2023-04-10

Synchronise yoga with upstream

Author: mnasiadka

neutron/common/ovn/constants.py

@@ -384,6 +384,8 @@
 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'

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py

@@ -28,6 +28,7 @@
 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
@@ -818,6 +819,66 @@ def update_port_qos_with_external_ids_reference(self):
                     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)
@@ -840,9 +901,11 @@ def check_vlan_distributed_ports(self):
         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(

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py

@@ -1320,6 +1320,11 @@ def update_router_routes(self, context, router_id, add, remove,
                     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
@@ -1551,6 +1556,31 @@ def _gen_router_port_ext_ids(self, port):
 
         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()
@@ -1565,14 +1595,15 @@ def _gen_router_port_options(self, port, network=None):
         # 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'])
@@ -1581,12 +1612,32 @@ def _gen_router_port_options(self, port, network=None):
                 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):
@@ -1667,6 +1718,12 @@ def create_router_port(self, context, router_id, router_interface):
                 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)
 
@@ -1807,6 +1864,11 @@ def delete_router_port(self, context, port_id, router_id=None,
                 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

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

@@ -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 @@ def test_update_port_qos_with_external_ids_reference(self):
                                     ('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 @@ def _test_check_vlan_distributed_ports(self, opt_value=None):
                           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 @@ def test_check_vlan_distributed_ports_expected_value(self):
             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

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.