Ensure redirect-type=bridged not used for geneve networks

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

Change-Id: I25e5ee2cf8daee52221a640faa7ac09679742707

2 files changed, 138 insertions, 13 deletions

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