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.
In particular: 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. Making centralized routing work for several DGP ports on the same router at once does not look like a good way to fix this: 1. Load balancers would not work correctly in every case. 2. NAT would misbehave in some cases. 3. It would break the old behavior for the ramp/EVPN-attached private network scenario described above: before [0], such traffic was processed locally on the chassis hosting the VM and went straight into a tunnel to the destination node, without going through a gateway port at all -- this is the behavior we want to preserve. 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. Note that this is only expected to work correctly when the router does not perform stateful traffic processing for that port -- i.e. plain routing and stateless NAT only. Stateful NAT and load balancing rely on the traffic being centralized on the gateway chassis and will not work correctly for a port excluded from centralized routing. Documentation for the new option is added to ovn-nb(5) and ovn-sb(5). [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 | 58 +++++++++++++++++++++++------------ northd/northd.h | 19 ++++++++++++ ovn-nb.xml | 37 ++++++++++++++++++++++ ovn-sb.xml | 11 +++++++ tests/ovn-northd.at | 62 +++++++++++++++++++++++++++++++++++++ tests/system-ovn.at | 68 +++++++++++++++++++++++++++++++++++++++++ 8 files changed, 250 insertions(+), 24 deletions(-) diff --git a/NEWS b/NEWS index c007767d0..07b1e7584 100644 --- a/NEWS +++ b/NEWS @@ -96,6 +96,12 @@ Post v26.03.0 egress pipelines. - The support for co-hosting multiple controller instances is now considered stable. Its "experimental" tag has been removed. + - 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 9f33b50cf..5c8e7e145 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -1820,22 +1820,30 @@ 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) - && vector_len(&op->od->l3dgw_ports) == 1 && 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 vector_len(&op->od->l3dgw_ports) == 1 && + !ls_has_localnet_port(op->peer->od); } static void @@ -4210,7 +4218,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"); @@ -4237,6 +4246,9 @@ sync_pb_for_lrp(struct ovn_port *op, op->nbrp->n_gateway_chassis)) { smap_add(&new, "chassis-redirect-port", op->cr_port->key); } + if (lrp_has_no_centralized_routing_option(op)) { + smap_add(&new, "no-centralized-routing", "true"); + } } if (chassis_name) { smap_add(&new, "l3gateway-chassis", chassis_name); @@ -9933,7 +9945,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 @@ -11709,7 +11721,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 @@ -15014,6 +15026,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; } @@ -15022,13 +15038,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 @@ -16681,8 +16695,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). @@ -16725,8 +16741,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..0609aca7c 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -1212,6 +1212,25 @@ 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); diff --git a/ovn-nb.xml b/ovn-nb.xml index 8a9d19fa9..4c70df63a 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -4971,6 +4971,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 037954800..2f8fe5ad5 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -3972,6 +3972,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 85f001003..e2a3e813d 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -23636,3 +23636,65 @@ 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 0 logical_port=cr-sw0-lr0 +check_row_count Port_Binding 1 logical_port=cr-sw1-lr0 + +check_row_count Port_Binding 1 logical_port=cr-lr0-sw0 options:always-redirect=true +check_row_count Port_Binding 0 logical_port=cr-lr0-sw1 options:always-redirect=true + +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'], [1], [dnl +0 +]) +AT_CHECK([ovn-sbctl dump-flows sw1 | grep "ls_in_l2_lkup" | grep -c 'cr-sw1-lr0'], [1], [dnl +0 +]) + +check ovn-nbctl lrp-add lr0 lr0-noswitch 00:00:00:00:ff:03 10.0.3.1/24 +check ovn-nbctl lrp-set-gateway-chassis lr0-noswitch gw1 +check ovn-nbctl set logical_router_port lr0-noswitch \ + options:no-centralized-routing=true +check ovn-nbctl --wait=sb sync +check_row_count Port_Binding 1 logical_port=cr-lr0-noswitch + +OVN_CLEANUP_NORTHD +AT_CLEANUP +]) diff --git a/tests/system-ovn.at b/tests/system-ovn.at index dc1bb1d74..26c6ea1cd 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -23762,3 +23762,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
