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, CR port on the switch side will still be created so controller can pick it up, but this CR port is not used for routing. Documentation for the new option is added. NEWS notes that when using VTEP, this option must now be set explicitly for L3 services to work correctly. Note that this is only expected to work correctly when the router does not perform any stateful traffic processing (NAT/LB) -- i.e. for plain routing only. [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 | 4 ++- controller/local_data.c | 12 ++++++-- northd/northd.c | 58 +++++++++++++++++++++++++++-------- northd/northd.h | 1 - ovn-nb.xml | 22 +++++++++++++ tests/ovn-northd.at | 64 ++++++++++++++++++++++++++++++++++++++ tests/system-ovn.at | 68 +++++++++++++++++++++++++++++++++++++++++ 7 files changed, 213 insertions(+), 16 deletions(-) diff --git a/NEWS b/NEWS index 384e30820..3347c97d9 100644 --- a/NEWS +++ b/NEWS @@ -68,7 +68,9 @@ Post v26.03.0 (type 11) and Parameter Problem (type 12) - generated by an external router are un-NATed correctly. This makes Path MTU discovery and traceroute work through stateless NAT. - + - Added "no-centralized-routing" option for logical router port which + is used for fully decentralized routing when a router has more than + one DGP port. See ovn-nb documentation for more information. OVN v26.03.0 - xxx xx xxxx -------------------------- - Added LSP/LRP option "requested-encap-ip" to let CMS request a specific diff --git a/controller/local_data.c b/controller/local_data.c index af6c75b40..eb9bd71a3 100644 --- a/controller/local_data.c +++ b/controller/local_data.c @@ -189,8 +189,16 @@ need_add_peer_to_local( !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); + * is local or if chassis-redirect-port exists but is not local + * but there is no-centralized-routing option on router port */ + if (ha_chassis_group_contains(cr_peer->ha_chassis_group, chassis)) { + return true; + } else if (smap_get_bool(&peer->options, + "no-centralized-routing", false)) { + return true; + } else { + return 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 3a4afa063..f003170cd 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -1813,6 +1813,22 @@ create_cr_port(struct ovn_port *op, struct hmap *ports, return crp; } +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); +} + +static inline bool +ensure_attached_router_has_centralized_routing( + const struct ovn_port *switch_op) +{ + return switch_op->cr_port && switch_op->peer && + !lrp_has_no_centralized_routing_option(switch_op->peer); +} + /* Returns true if chassis resident port needs to be created for * op's peer logical switch. False otherwise. * @@ -1830,6 +1846,8 @@ peer_needs_cr_port_creation(struct ovn_port *op) && vector_len(&op->od->l3dgw_ports) == 1 && op->peer && op->peer->nbsp && !ls_has_localnet_port(op->peer->od)) { return true; + } else if (lrp_has_no_centralized_routing_option(op)) { + return true; } return false; @@ -2738,6 +2756,13 @@ l3dgw_port_has_associated_vtep_lports(const struct ovn_port *op) return op->peer && op->peer->od->has_vtep_lports; } +/*static inline bool +ensure_router_with_centralized_routing(const struct ovn_port ) +{ + +} +*/ + static void ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn, struct ovsdb_idl_index *sbrec_chassis_by_name, @@ -4146,7 +4171,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"); @@ -4173,6 +4199,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); @@ -9681,7 +9710,8 @@ 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 centralized_routing = + ensure_attached_router_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 @@ -9692,7 +9722,7 @@ build_lswitch_rport_arp_req_flow( * the DGP's MAC are sent to the chassis where the DGP resides. * */ - if (has_cr_port) { + if (centralized_routing) { ds_put_format(&match, " && is_chassis_resident(%s)", patch_op->cr_port->json_key); } @@ -9725,7 +9755,7 @@ build_lswitch_rport_arp_req_flow( WITH_HINT(stage_hint)); } - if (has_cr_port) { + if (centralized_routing) { ds_clear(&match); ds_put_format(&match, "%s && !is_chassis_resident(%s)", ds_cstr(&m), patch_op->cr_port->json_key); @@ -11497,7 +11527,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 (ensure_attached_router_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 @@ -14795,6 +14825,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; } @@ -14803,13 +14837,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 @@ -16400,7 +16432,8 @@ 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)) { + if (l3dgw_port_has_associated_vtep_lports(dgp) || + lrp_has_no_centralized_routing_option(dgp)) { /* Skip adding redirect lflow for vtep-enabled l3dgw ports. * Traffic from hypervisor to VTEP (ramp) switch should go in * distributed manner. Only returning routed traffic must go @@ -16444,7 +16477,8 @@ 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)) { + if (l3dgw_port_has_associated_vtep_lports(dgp) || + lrp_has_no_centralized_routing_option(dgp)) { /* Skip adding redirect lflow for vtep-enabled l3dgw ports. * Traffic from hypervisor to VTEP (ramp) switch should go in * distributed manner. Only returning routed traffic must go diff --git a/northd/northd.h b/northd/northd.h index d27f519d6..9bd0f9d9b 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -1211,7 +1211,6 @@ od_is_centralized(const struct ovn_datapath *od) } struct ovn_port *ovn_port_find(const struct hmap *ports, const char *name); - void build_igmp_lflows(struct hmap *igmp_groups, const struct hmap *ls_datapaths, struct lflow_table *lflows, diff --git a/ovn-nb.xml b/ovn-nb.xml index 33a6dc676..1dca6d2d3 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -4843,6 +4843,28 @@ 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 attached switch has no localnet port, + OVN centralizes routing for that router port on the gateway chassis: + traffic destined to router port with DGP is redirected gateway + chassis right on a switch pipeline. + Set this option to <code>true</code> on router port to exclude + it from centralized routing. Traffic destined to this router port + with DGP is then processed locally, instead of being redirected + to the gateway chassis, and ARP/ND for its addresses is no longer + chassis-restricted. + This is useful, for example, for a router that has a DGP for + a ramp switch or EVPN-attached private subnet -- which + has no <code>localnet</code> port of its own -- alongside + another DGP used for unrelated purposes, such as NAT on a + provider network. Without this option, the private subnet + would lose L3 routing entirely. + </p> + </column> </group> <group title="Attachment"> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 86cab3d5b..6a2e4975b 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -21410,3 +21410,67 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE OVN_CLEANUP_NORTHD AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD_NO_HV([ +AT_SETUP([DGP "no-centralized-routing" option]) +ovn_start + +# lr0 has two DGPs, each facing its own localnet-less subnet: +# - lr0-sw0: no "no-centralized-routing" -- the only DGP that actually +# contends for centralizing anything, so it keeps the whole +# "centralized routing on an internal switch" behavior. +# - lr0-sw1: "no-centralized-routing" set -- opts out of it entirely. +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 + +# build_gateway_redirect_flows_for_lrouter(): lr0-sw1 must not get a +# GW_REDIRECT flow at all -- traffic destined to it is processed +# locally instead of being redirected to its cr-port. +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;) +]) + +# consider_l3dgw_port_is_centralized(): lr_in_admission for lr0-sw0 +# (centralized) must require residency on its own cr-port; lr0-sw1 +# (opted out) must not. +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 +]) + +OVN_CLEANUP_NORTHD +AT_CLEANUP +]) diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 17b4dcb3d..83cac4f76 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -21998,3 +21998,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 +]) \ No newline at end of file -- 2.48.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
