Commit [0] introduced centralized routing as an opt-in option. Later,
commit [1] made this behavior the default, but it did not cover all
use cases.

If a router is attached to a private subnet with no
localnet port (no direct path to a physical network), and the
external network is instead reached through that subnet via a ramp
switch or EVPN, the corresponding router port is normally turned into
a DGP -- to advertise a default route, answer ARP/ND requests, and
advertise EVPN routes. If the same router also has another DGP port
for unrelated purposes, the private subnet loses access to L3
services/routing entirely: centralized routing currently only works
for a single DGP port per router, so packets on this second DGP get
dropped in lr_in_admission by the is_chassis_resident check.
That check was removed for ramp switches by patch [2], but even with
that fix, the controller on the chassis hosting a VM from this private
subnet will not add the router to its local_datapaths -- as of patch [3],
that logic also relies on the peer switch having its own chassisredirect port.

Instead, add a "no-centralized-routing" option on the DGP that lets a
specific DGP be explicitly excluded from centralized routing. With
this option enabled, the CR port on the switch side is still created,
so that ovn-controller can pick it up and add the router to its local
datapaths, but this CR port is not used for routing.

[0] 
https://github.com/ovn-org/ovn/commit/8d13579bf5b390c1dcf1e737f918e05407f8692c
[1] 
https://github.com/ovn-org/ovn/commit/c71383f858cc80617d50aa8b1fcdb573f3930f4d
[2] 
https://github.com/ovn-org/ovn/commit/9c79ee4d8dd6a3aa31d0f94b1384044daf61cbfb
[3] 
https://github.com/ovn-org/ovn/commit/f1c391c18969d829beb99738fe7f2287ef0f6a14

Fixes: f1c391c18969 ("controller: Optimize adding 'dps' to the local 
datapaths.")
Fixes: 8d13579bf5b3 ("Add support for centralize routing for distributed gw 
ports.")

Signed-off-by: Alexandra Rukomoinikova <[email protected]>
---
 NEWS                    |  6 ++++
 controller/local_data.c | 13 +++++---
 northd/northd.c         | 59 +++++++++++++++++++++++------------
 northd/northd.h         | 20 ++++++++++++
 ovn-nb.xml              | 37 ++++++++++++++++++++++
 ovn-sb.xml              | 11 +++++++
 tests/ovn-northd.at     | 52 +++++++++++++++++++++++++++++++
 tests/system-ovn.at     | 68 +++++++++++++++++++++++++++++++++++++++++
 8 files changed, 242 insertions(+), 24 deletions(-)

diff --git a/NEWS b/NEWS
index 40a1b9867..874f4c96b 100644
--- a/NEWS
+++ b/NEWS
@@ -120,6 +120,12 @@ OVN v26.09.0 - xxx xx xxxx
      filter the routes learned through the port by route tag.  This supersedes
      "ic-route-filter-tag", which is now deprecated and is ignored when
      "ic-route-learn-tag-rules" is set.
+   - Added the logical router port option "no-centralized-routing", which
+     excludes a distributed gateway port from centralized routing.  It is
+     intended for routers with more than one distributed gateway port,
+     where routing for such a port has to stay distributed. Note that the
+     router must not perform stateful processing (stateful NAT or load
+     balancing) for that port.  See ovn-nb(5) for more information.
 
 OVN v26.03.0 - xxx xx xxxx
 --------------------------
diff --git a/controller/local_data.c b/controller/local_data.c
index ad4b92f1f..72d1a01b9 100644
--- a/controller/local_data.c
+++ b/controller/local_data.c
@@ -142,7 +142,8 @@ local_datapath_destroy(struct local_datapath *ld)
  * Note that if 'pb' belongs to a logical switch and 'peer' to a
  * logical router datapath and if 'peer' has a chassis-redirect port,
  * then we add the 'peer' to the local datapaths only if the
- * chassis-redirect port is local.
+ * chassis-redirect port is local or if 'peer' is excluded from
+ * centralized routing.
  * */
 bool
 need_add_peer_to_local(
@@ -189,9 +190,13 @@ need_add_peer_to_local(
     if (cr_peer && datapath_is_switch(pb->datapath) &&
         !datapath_is_switch(peer->datapath)) {
         /* pb belongs to logical switch and peer  belongs to logical router.
-         * Add the peer to local datapaths only if its chassis-redirect-port
-         * is local. */
-        return ha_chassis_group_contains(cr_peer->ha_chassis_group, chassis);
+         * Add the peer to local datapaths if its chassis-redirect-port is
+         * local, or if the peer is excluded from centralized routing, in
+         * which case its chassis-redirect-port is not used for routing and
+         * the router pipeline has to run locally. */
+        return ha_chassis_group_contains(cr_peer->ha_chassis_group, chassis)
+               || smap_get_bool(&peer->options, "no-centralized-routing",
+                                false);
     }
 
     /* Check if cr-pb is configured as "always-redirect". If not, then we will
diff --git a/northd/northd.c b/northd/northd.c
index a352a810b..fac2d313f 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -1825,22 +1825,29 @@ create_cr_port(struct ovn_port *op, struct hmap *ports,
  * op's peer logical switch.  False otherwise.
  *
  * Chassis resident port needs to be created if the following
- * conditionsd are met:
+ * conditions are met:
  *   - op is a distributed gateway port
- *   - op is the only distributed gateway port attached to its
- *     router
- *   - op's peer logical switch has no localnet ports.
+ *   - op's peer is a logical switch port
+ *   - either
+ *     - op is excluded from centralized routing, in which case the
+ *       chassis resident port is not used for routing, but ovn-controller
+ *       still needs it to add the router to its local datapaths, or
+ *     - op is the only distributed gateway port attached to its router
+ *       and op's peer logical switch has no localnet ports.
  */
 static bool
 peer_needs_cr_port_creation(struct ovn_port *op)
 {
-    if ((op->nbrp->n_gateway_chassis || op->nbrp->ha_chassis_group)
-        && op->peer && op->peer->nbsp
-        && !ls_has_localnet_port(op->peer->od)) {
+    if (!op->peer || !op->peer->nbsp ||
+        (!op->nbrp->n_gateway_chassis && !op->nbrp->ha_chassis_group)) {
+        return false;
+    }
+
+    if (lrp_has_no_centralized_routing_option(op)) {
         return true;
     }
 
-    return false;
+    return !ls_has_localnet_port(op->peer->od);
 }
 
 static void
@@ -4215,7 +4222,8 @@ sync_pb_for_lrp(struct ovn_port *op,
         bool always_redirect =
             !lr_stateful_rec->has_distributed_lb &&
             !lr_stateful_rec->lrnat_rec->has_distributed_nat &&
-            !l3dgw_port_has_associated_vtep_lports(op->primary_port);
+            !l3dgw_port_has_associated_vtep_lports(op->primary_port) &&
+            !lrp_has_no_centralized_routing_option(op->primary_port);
 
         const char *redirect_type = smap_get(&op->nbrp->options,
                                             "redirect-type");
@@ -4299,6 +4307,11 @@ sync_pb_for_lrp(struct ovn_port *op,
         smap_add(&new, "ipv6_ra_pd_list", ipv6_pd_list);
     }
 
+    if (!is_cr_port(op) &&
+        lrp_has_no_centralized_routing_option(op)) {
+        smap_add(&new, "no-centralized-routing", "true");
+    }
+
     sbrec_port_binding_set_options(op->sb, &new);
     smap_destroy(&new);
 }
@@ -9938,7 +9951,7 @@ build_lswitch_rport_arp_req_flow(
     arp_nd_ns_match(ips, addr_family, &m);
     ds_clone(&match, &m);
 
-    bool has_cr_port = patch_op->cr_port;
+    bool has_cr_port = lsp_has_centralized_routing(patch_op);
 
     /* If the patch_op has a chassis resident port, it means
      *    - its peer is a distributed gateway port (DGP) and
@@ -11714,7 +11727,7 @@ build_lswitch_ip_unicast_lookup(struct ovn_port *op,
             !vector_is_empty(&op->peer->od->l3dgw_ports) &&
             ls_has_localnet_port(op->od)) {
             add_lrp_chassis_resident_check(op->peer, match);
-        } else if (op->cr_port) {
+        } else if (lsp_has_centralized_routing(op)) {
             /* If the op has a chassis resident port, it means
              *   - its peer is a distributed gateway port (DGP) and
              *   - routing is centralized for the DGP's networks on
@@ -15019,6 +15032,10 @@ build_gateway_mtu_flow(struct lflow_table *lflows, 
struct ovn_port *op,
 static bool
 consider_l3dgw_port_is_centralized(struct ovn_port *op)
 {
+    if (!lrp_is_l3dgw(op)) {
+        return false;
+    }
+
     if (!od_is_centralized(op->od)) {
         return false;
     }
@@ -15027,13 +15044,11 @@ consider_l3dgw_port_is_centralized(struct ovn_port 
*op)
         return false;
     }
 
-    if (lrp_is_l3dgw(op)) {
-        /* Traffic with eth.dst = l3dgw_port->lrp_networks.ea_s
-         * should only be received on the gateway chassis. */
-        return true;
+    if (lrp_has_no_centralized_routing_option(op)) {
+        return false;
     }
 
-    return false;
+    return true;
 }
 
 /* Logical router ingress Table 0: L2 Admission Control
@@ -16686,8 +16701,10 @@ build_gateway_redirect_flows_for_lrouter(
     ovs_assert(od->nbr);
     const struct ovn_port *dgp;
     VECTOR_FOR_EACH (&od->l3dgw_ports, dgp) {
-        if (l3dgw_port_has_associated_vtep_lports(dgp)) {
-            /* Skip adding redirect lflow for vtep-enabled l3dgw ports.
+        if (l3dgw_port_has_associated_vtep_lports(dgp) ||
+            lrp_has_no_centralized_routing_option(dgp)) {
+            /* Skip adding redirect lflow for vtep-enabled l3dgw ports and
+             * for l3dgw ports excluded from centralized routing.
              * Traffic from hypervisor to VTEP (ramp) switch should go in
              * distributed manner. Only returning routed traffic must go
              * through centralized gateway (or ha-chassis-group).
@@ -16730,8 +16747,10 @@ build_lr_gateway_redirect_flows_for_nats(
     ovs_assert(od->nbr);
     const struct ovn_port *dgp;
     VECTOR_FOR_EACH (&od->l3dgw_ports, dgp) {
-        if (l3dgw_port_has_associated_vtep_lports(dgp)) {
-            /* Skip adding redirect lflow for vtep-enabled l3dgw ports.
+        if (l3dgw_port_has_associated_vtep_lports(dgp) ||
+            lrp_has_no_centralized_routing_option(dgp)) {
+            /* Skip adding redirect lflow for vtep-enabled l3dgw ports and
+             * for l3dgw ports excluded from centralized routing.
              * Traffic from hypervisor to VTEP (ramp) switch should go in
              * distributed manner. Only returning routed traffic must go
              * through centralized gateway (or ha-chassis-group).
diff --git a/northd/northd.h b/northd/northd.h
index 2e3a9e00d..98397ac89 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -1213,6 +1213,26 @@ od_is_centralized(const struct ovn_datapath *od)
     return !od->is_distributed;
 }
 
+/* Returns true if the logical router port 'router_op' is excluded from
+ * centralized routing by the "no-centralized-routing" option. */
+static inline bool
+lrp_has_no_centralized_routing_option(const struct ovn_port *router_op)
+{
+    return router_op && router_op->nbrp &&
+           smap_get_bool(&router_op->nbrp->options,
+                         "no-centralized-routing", false);
+}
+
+/* Returns true if routing is centralized on the gateway chassis for the
+ * networks of the distributed gateway port peered with the logical switch
+ * port 'switch_op'. */
+static inline bool
+lsp_has_centralized_routing(struct ovn_port *switch_op)
+{
+    return switch_op->cr_port && switch_op->peer &&
+           !lrp_has_no_centralized_routing_option(switch_op->peer);
+}
+
 struct ovn_port *ovn_port_find(const struct hmap *ports, const char *name);
 
 void build_igmp_lflows(struct hmap *igmp_groups,
diff --git a/ovn-nb.xml b/ovn-nb.xml
index c741a3b32..4d27edaa2 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -5014,6 +5014,43 @@ or
           router version of this option.
         </p>
       </column>
+
+      <column name="options" key="no-centralized-routing"
+              type='{"type": "boolean"}'>
+        <p>
+          By default, when a logical router has a single distributed gateway
+          port (DGP) and the switch attached to it has no
+          <code>localnet</code> port, OVN centralizes routing for that
+          router port on the gateway chassis: traffic destined to the DGP
+          is redirected to the gateway chassis already in the logical
+          switch pipeline.
+        </p>
+
+        <p>
+          Set this option to <code>true</code> on a DGP to exclude it from
+          centralized routing.  Traffic destined to this router port is then
+          processed locally on the chassis where it originates, instead of
+          being redirected to the gateway chassis, and ARP/ND for its
+          addresses is no longer restricted to the gateway chassis. The
+          chassis redirect port of the attached logical switch is still
+          created, because <code>ovn-controller</code> needs it to add the
+          router to its local datapaths, but it is not used for routing.
+        </p>
+
+        <p>
+          This is useful, for example, for a router that has a DGP for a
+          ramp switch or an EVPN-attached private subnet -- which has no
+          <code>localnet</code> port of its own -- alongside another DGP
+          used for unrelated purposes. Without this option, the private
+          subnet would lose L3 routing entirely.
+        </p>
+
+        <p>
+          This option is only expected to work correctly if the logical
+          router does not perform stateful processing for this port, that
+          is, plain routing and stateless NAT only.
+        </p>
+      </column>
     </group>
 
     <group title="Attachment">
diff --git a/ovn-sb.xml b/ovn-sb.xml
index b41b79662..af6492dc6 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -4145,6 +4145,17 @@ tcp.flags = RST;
         The name of the chassis redirect port derived from this port if this
         port is a distributed parent of a chassis redirect port.
       </column>
+
+      <column name="options" key="no-centralized-routing"
+              type='{"type": "boolean"}'>
+        A boolean option that is set to true if this distributed gateway port
+        is excluded from centralized routing.  The value is derived from
+        <ref table="Logical_Router_Port" column="options"
+        key="no-centralized-routing" db="OVN_Northbound"/>.  If set,
+        <code>ovn-controller</code> adds the logical router datapath to its
+        local datapaths even if the chassis redirect port of the peer logical
+        switch port is not local.
+      </column>
     </group>
 
     <group title="Chassis Redirect Options">
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index d0fa17d08..a1cb284af 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -23843,3 +23843,55 @@ AT_CHECK([as northd ovn-appctl -t ovn-northd 
inc-engine/enable-stopwatch nonexis
 OVN_CLEANUP_NORTHD
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([DGP "no-centralized-routing" option])
+ovn_start
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl ls-add sw1
+check ovn-nbctl lsp-add sw0 sw0-port1
+check ovn-nbctl lsp-add sw1 sw1-port1
+
+check ovn-nbctl lr-add lr0
+check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.1.1/24
+check ovn-nbctl lsp-add-router-port sw0 sw0-lr0 lr0-sw0
+check ovn-nbctl lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 10.0.2.1/24
+check ovn-nbctl lsp-add-router-port sw1 sw1-lr0 lr0-sw1
+
+check ovn-nbctl lrp-set-gateway-chassis lr0-sw0 gw1
+check ovn-nbctl lrp-set-gateway-chassis lr0-sw1 gw1
+check ovn-nbctl set logical_router_port lr0-sw1 
options:no-centralized-routing=true
+
+check ovn-nbctl --wait=sb sync
+
+check_row_count Port_Binding 1 logical_port=cr-sw0-lr0
+check_row_count Port_Binding 1 logical_port=cr-sw1-lr0
+
+check_row_count Port_Binding 0 logical_port=lr0-sw0 
options:no-centralized-routing=true
+check_row_count Port_Binding 1 logical_port=lr0-sw1 
options:no-centralized-routing=true
+
+AT_CHECK([ovn-sbctl dump-flows lr0 | grep "lr_in_gw_redirect" | 
ovn_strip_lflows], [0], [dnl
+  table=??(lr_in_gw_redirect  ), priority=0    , match=(1), action=(next;)
+  table=??(lr_in_gw_redirect  ), priority=50   , match=(outport == "lr0-sw0"), 
action=(outport = "cr-lr0-sw0"; next;)
+])
+
+AT_CHECK([ovn-sbctl dump-flows lr0 | grep "lr_in_admission" | grep "lr0-sw0" | 
grep 'priority=50' | ovn_strip_lflows], [0], [dnl
+  table=??(lr_in_admission    ), priority=50   , match=(eth.dst == 
00:00:00:00:ff:01 && inport == "lr0-sw0" && is_chassis_resident("cr-lr0-sw0")), 
action=(xreg0[[0..47]] = 00:00:00:00:ff:01; next;)
+  table=??(lr_in_admission    ), priority=50   , match=(eth.mcast && inport == 
"lr0-sw0"), action=(xreg0[[0..47]] = 00:00:00:00:ff:01; next;)
+])
+AT_CHECK([ovn-sbctl dump-flows lr0 | grep "lr_in_admission" | grep "lr0-sw1" | 
grep 'priority=50' | ovn_strip_lflows], [0], [dnl
+  table=??(lr_in_admission    ), priority=50   , match=(eth.dst == 
00:00:00:00:ff:02 && inport == "lr0-sw1"), action=(xreg0[[0..47]] = 
00:00:00:00:ff:02; next;)
+  table=??(lr_in_admission    ), priority=50   , match=(eth.mcast && inport == 
"lr0-sw1"), action=(xreg0[[0..47]] = 00:00:00:00:ff:02; next;)
+])
+
+AT_CHECK([ovn-sbctl dump-flows sw0 | grep "ls_in_l2_lkup" | grep -c 
'cr-sw0-lr0'], [0], [dnl
+6
+])
+AT_CHECK([ovn-sbctl dump-flows sw1 | grep "ls_in_l2_lkup" | grep -c 
'cr-sw1-lr0'], [1], [dnl
+0
+])
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
+])
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 973c46728..f0ba739de 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -23741,3 +23741,71 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port 
patch-.*/d
 
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([DGP "no-centralized-routing" option])
+ovn_start
+OVS_TRAFFIC_VSWITCHD_START()
+ADD_BR([br-int])
+
+check ovs-vsctl \
+        -- set Open_vSwitch . external-ids:system-id=hv1 \
+        -- set Open_vSwitch . 
external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
+        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
+        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
+        -- set bridge br-int fail-mode=secure other-config:disable-in-band=true
+
+start_daemon ovn-controller
+
+check ovn-nbctl lr-add lr0
+check ovn-nbctl lrp-add lr0 lr0-sw0 f0:00:00:00:01:01 172.31.0.1/24
+check ovn-nbctl lrp-add lr0 lr0-sw1 f0:00:00:00:01:02 172.32.0.1/24
+check ovn-nbctl set logical_router_port lr0-sw0 
options:no-centralized-routing=true
+check ovn-nbctl set logical_router_port lr0-sw1 
options:no-centralized-routing=true
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl ls-add sw1
+check ovn-nbctl lsp-add-router-port sw0 sw0-lr0 lr0-sw0
+check ovn-nbctl lsp-add-router-port sw1 sw1-lr0 lr0-sw1
+
+check ovn-nbctl lrp-set-gateway-chassis lr0-sw0 hv2
+check ovn-nbctl lrp-set-gateway-chassis lr0-sw1 hv2
+
+check ovn-nbctl lsp-add sw0 sw0_vif
+check ovn-nbctl lsp-set-addresses sw0_vif "02:ac:10:02:01:03 172.31.0.3"
+ADD_NAMESPACES(sw0_vif)
+ADD_VETH(sw0_vif, sw0_vif, br-int, "172.31.0.3/24", "02:ac:10:02:01:03", \
+         "172.31.0.1")
+
+check ovn-nbctl lsp-add sw1 sw1_vif
+check ovn-nbctl lsp-set-addresses sw1_vif "02:ac:10:02:02:03 172.32.0.3"
+ADD_NAMESPACES(sw1_vif)
+ADD_VETH(sw1_vif, sw1_vif, br-int, "172.32.0.3/24", "02:ac:10:02:02:03", \
+         "172.32.0.1")
+
+OVN_POPULATE_ARP
+wait_for_ports_up
+check ovn-nbctl --wait=hv sync
+
+# both DGPs are resident on hv2 -- E/W and access to both routers' own IPs 
work.
+NS_CHECK_EXEC([sw0_vif], [ping -q -c 3 -i 0.3 -w 2 172.32.0.3 | FORMAT_PING], 
[0],
+[dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+NS_CHECK_EXEC([sw0_vif], [ping -q -c 3 -i 0.3 -w 2 172.31.0.1 | FORMAT_PING], 
[0],
+[dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+NS_CHECK_EXEC([sw1_vif], [ping -q -c 3 -i 0.3 -w 2 172.32.0.1 | FORMAT_PING], 
[0],
+[dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+OVN_CLEANUP_CONTROLLER([hv1])
+OVN_CLEANUP_NORTHD
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+AT_CLEANUP
+])
-- 
2.48.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to