Previously we just assumed that if a LR had multiple LRPs bound to the local chassis that all routes where valid for all of these LRPs. This commit handles a previous option in ovn-nb that allows the user to specify the interface name that a route needs to use to be acceptable for a given LRP. The user specifies a port name in the northbound database. If this port is bound locally we use the interface name associated with it for filtering routes. Additionally users can overwrite that interface name to also support cases where there is no referenceable port name in the northbound.
This can e.g. be used if a chassis has multiple uplinks. Each of these uplinks could be assigned to a different LRP of the same LR connected to the same localnet. The option allows then to map e.g. a learned default route to each individual LRP. If one of the links would go down just one of the learned routes would be removed, causing ECMP across multiple chassis to be correctly weighted. Signed-off-by: Felix Huettner <[email protected]> --- v7->v8: * Rework implementation to only parse the port-mapping setting once per run. v6->v7: * Reworked implementation based on review comments. We now have a layer of indirection to allow different chassis to have different interface names. v5->v6: * addressed review comments v3->v4: - addressed review comments. - updated commit message to be more descriptive. controller/ovn-controller.8.xml | 8 +++ controller/ovn-controller.c | 7 +++ controller/route-exchange-netlink.c | 2 + controller/route-exchange-netlink.h | 3 + controller/route-exchange.c | 20 ++++-- controller/route.c | 81 ++++++++++++++++++++++--- controller/route.h | 11 +++- tests/system-ovn.at | 94 ++++++++++++++++++++++++++++- 8 files changed, 209 insertions(+), 17 deletions(-) diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml index f27b32118..a6a6d6f4e 100644 --- a/controller/ovn-controller.8.xml +++ b/controller/ovn-controller.8.xml @@ -414,6 +414,14 @@ If the value is zero, it disables the inactivity probe. </p> </dd> + <dt><code>external_ids:dynamic-routing-port-mapping</code></dt> + <dd> + <p> + This setting works together with the Northbound + dynamic-routing-port-name option on Logical_Router_Ports. + See the <code>ovn-nb</code>(5) for more details. + </p> + </dd> </dl> <p> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index cf83ea6e3..bc5467c0f 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -5005,13 +5005,20 @@ en_route_run(struct engine_node *node, void *data) const struct sbrec_advertised_route_table *advertised_route_table = EN_OVSDB_GET(engine_get_input("SB_advertised_route", node)); + const struct ovsrec_open_vswitch *cfg + = ovsrec_open_vswitch_table_first(ovs_table); + const char *dynamic_routing_port_mapping = + smap_get(&cfg->external_ids, "dynamic-routing-port-mapping"); + struct route_ctx_in r_ctx_in = { .advertised_route_table = advertised_route_table, .sbrec_port_binding_by_name = sbrec_port_binding_by_name, .chassis = chassis, + .dynamic_routing_port_mapping = dynamic_routing_port_mapping, .active_tunnels = &rt_data->active_tunnels, .local_datapaths = &rt_data->local_datapaths, .local_lports = &rt_data->local_lports, + .local_bindings = &rt_data->lbinding_data.bindings, }; struct route_ctx_out r_ctx_out = { diff --git a/controller/route-exchange-netlink.c b/controller/route-exchange-netlink.c index 07c4ab0d0..bcd48c028 100644 --- a/controller/route-exchange-netlink.c +++ b/controller/route-exchange-netlink.c @@ -227,6 +227,8 @@ handle_route_msg(const struct route_table_msg *msg, void *data) rr->prefix = rd->rta_dst; rr->plen = rd->rtm_dst_len; rr->nexthop = nexthop->addr; + memcpy(rr->ifname, nexthop->ifname, IFNAMSIZ); + rr->ifname[IFNAMSIZ] = 0; ovs_list_push_back(handle_data->learned_routes, &rr->list_node); } return; diff --git a/controller/route-exchange-netlink.h b/controller/route-exchange-netlink.h index 6b26103d5..e46ae048e 100644 --- a/controller/route-exchange-netlink.h +++ b/controller/route-exchange-netlink.h @@ -21,6 +21,7 @@ #include <stdint.h> #include "openvswitch/list.h" #include <netinet/in.h> +#include <net/if.h> /* This value is arbitrary but currently unused. * See the kernel rtnetlink UAPI at @@ -37,6 +38,8 @@ struct re_nl_received_route_node { struct in6_addr prefix; unsigned int plen; struct in6_addr nexthop; + /* Adding 1 to this to be sure we actually have a terminating '\0' */ + char ifname[IFNAMSIZ + 1]; }; int re_nl_create_vrf(const char *ifname, uint32_t table_id); diff --git a/controller/route-exchange.c b/controller/route-exchange.c index b0ebc4fda..7b91d3adb 100644 --- a/controller/route-exchange.c +++ b/controller/route-exchange.c @@ -94,7 +94,7 @@ route_lookup(struct hmap *route_map, static void sb_sync_learned_routes(const struct ovs_list *learned_routes, const struct sbrec_datapath_binding *datapath, - const struct sset *bound_ports, + const struct smap *bound_ports, struct ovsdb_idl_txn *ovnsb_idl_txn, struct ovsdb_idl_index *sbrec_port_binding_by_name, struct ovsdb_idl_index *sbrec_learned_route_by_datapath) @@ -109,8 +109,9 @@ sb_sync_learned_routes(const struct ovs_list *learned_routes, SBREC_LEARNED_ROUTE_FOR_EACH_EQUAL (sb_route, filter, sbrec_learned_route_by_datapath) { /* If the port is not local we don't care about it. - * Some other ovn-controller will handle it. */ - if (!sset_contains(bound_ports, + * Some other ovn-controller will handle it. + * We may not use smap_get since the value might be validly NULL. */ + if (!smap_get_node(bound_ports, sb_route->logical_port->logical_port)) { continue; } @@ -124,11 +125,18 @@ sb_sync_learned_routes(const struct ovs_list *learned_routes, learned_route->plen); char *nexthop = normalize_v46(&learned_route->nexthop); - const char *logical_port_name; - SSET_FOR_EACH (logical_port_name, bound_ports) { + struct smap_node *port_node; + SMAP_FOR_EACH (port_node, bound_ports) { + /* The user specified an ifname, but we learned it on a different + * port. */ + if (port_node->value && strcmp(port_node->value, + learned_route->ifname)) { + continue; + } + const struct sbrec_port_binding *logical_port = lport_lookup_by_name(sbrec_port_binding_by_name, - logical_port_name); + port_node->key); if (!logical_port) { continue; } diff --git a/controller/route.c b/controller/route.c index 108435bad..4f3f26838 100644 --- a/controller/route.c +++ b/controller/route.c @@ -19,8 +19,10 @@ #include <net/if.h> +#include "vswitch-idl.h" #include "openvswitch/hmap.h" #include "openvswitch/vlog.h" +#include "openvswitch/ofp-parse.h" #include "lib/ovn-sb-idl.h" @@ -30,7 +32,6 @@ #include "route.h" VLOG_DEFINE_THIS_MODULE(exchange); -static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); bool route_exchange_relevant_port(const struct sbrec_port_binding *pb) @@ -73,6 +74,49 @@ find_route_exchange_pb(struct ovsdb_idl_index *sbrec_port_binding_by_name, return NULL; } +static void +build_port_mapping(struct smap *mapping, const char *port_mapping) +{ + if (!port_mapping) { + return; + } + + char *tokstr, *orig, *key, *value; + orig = tokstr = xstrdup(port_mapping); + + while (ofputil_parse_key_value(&tokstr, &key, &value)) { + if (!*value) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); + VLOG_WARN_RL(&rl, "dynamic-routing-port-mapping setting '%s' is " + "not valid.", tokstr); + free(orig); + break; + } + smap_add(mapping, key, value); + } + free(orig); +} + +static const char * +ifname_from_port_name(const struct smap *port_mapping, + struct shash *local_bindings, + const struct sbrec_chassis *chassis, + const char *port_name) +{ + const char *iface = smap_get(port_mapping, port_name); + if (iface) { + return iface; + } + + if (!local_binding_is_up(local_bindings, port_name, chassis)) { + return NULL; + } + + struct local_binding *binding = local_binding_find(local_bindings, + port_name); + return binding->iface->name; +} + static void advertise_datapath_cleanup(struct advertise_datapath_entry *ad) { @@ -82,7 +126,7 @@ advertise_datapath_cleanup(struct advertise_datapath_entry *ad) free(ar); } hmap_destroy(&ad->routes); - sset_destroy(&ad->bound_ports); + smap_destroy(&ad->bound_ports); free(ad); } @@ -105,6 +149,9 @@ route_run(struct route_ctx_in *r_ctx_in, { struct advertise_datapath_entry *ad; const struct local_datapath *ld; + struct smap port_mapping = SMAP_INITIALIZER(&port_mapping); + + build_port_mapping(&port_mapping, r_ctx_in->dynamic_routing_port_mapping); HMAP_FOR_EACH (ld, hmap_node, r_ctx_in->local_datapaths) { if (!ld->n_peer_ports || ld->is_switch) { @@ -114,7 +161,7 @@ route_run(struct route_ctx_in *r_ctx_in, ad = xzalloc(sizeof(*ad)); ad->db = ld->datapath; hmap_init(&ad->routes); - sset_init(&ad->bound_ports); + smap_init(&ad->bound_ports); /* This is a LR datapath, find LRPs with route exchange options * that are bound locally. */ @@ -132,13 +179,28 @@ route_run(struct route_ctx_in *r_ctx_in, ad->maintain_vrf |= smap_get_bool( &repb->options, "dynamic-routing-maintain-vrf", false); - sset_add(&ad->bound_ports, local_peer->logical_port); - } - if (sset_is_empty(&ad->bound_ports)) { - advertise_datapath_cleanup(ad); - continue; + const char *port_name = smap_get(&repb->options, + "dynamic-routing-port-name"); + if (!port_name) { + /* No port-name set, so we learn routes from all ports. */ + smap_add_nocopy(&ad->bound_ports, + xstrdup(local_peer->logical_port), NULL); + } else { + /* If a port_name is set the we filter for the name as set in + * the port-mapping or the interface name of the local + * binding. If the port is not in the port_mappings and not + * bound locally we will not learn routes for this port. */ + const char *ifname = ifname_from_port_name( + &port_mapping, r_ctx_in->local_bindings, + r_ctx_in->chassis, port_name); + if (ifname) { + smap_add(&ad->bound_ports, local_peer->logical_port, + ifname); + } + } } + tracked_datapath_add(ld->datapath, TRACKED_RESOURCE_NEW, r_ctx_out->tracked_re_datapaths); @@ -157,6 +219,7 @@ route_run(struct route_ctx_in *r_ctx_in, struct in6_addr prefix; unsigned int plen; if (!ip46_parse_cidr(route->ip_prefix, &prefix, &plen)) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); VLOG_WARN_RL(&rl, "bad 'ip_prefix' %s in route " UUID_FMT, route->ip_prefix, UUID_ARGS(&route->header_.uuid)); @@ -169,6 +232,8 @@ route_run(struct route_ctx_in *r_ctx_in, hmap_insert(&ad->routes, &ar->node, advertise_route_hash(&prefix, plen)); } + + smap_destroy(&port_mapping); } void diff --git a/controller/route.h b/controller/route.h index 448643ead..d3e5b3eff 100644 --- a/controller/route.h +++ b/controller/route.h @@ -22,6 +22,7 @@ #include <netinet/in.h> #include "openvswitch/hmap.h" #include "sset.h" +#include "smap.h" struct hmap; struct ovsdb_idl_index; @@ -32,9 +33,11 @@ struct route_ctx_in { const struct sbrec_advertised_route_table *advertised_route_table; struct ovsdb_idl_index *sbrec_port_binding_by_name; const struct sbrec_chassis *chassis; + const char *dynamic_routing_port_mapping; const struct sset *active_tunnels; const struct hmap *local_datapaths; const struct sset *local_lports; + struct shash *local_bindings; }; struct route_ctx_out { @@ -50,8 +53,12 @@ struct advertise_datapath_entry { struct hmap routes; /* The name of the port bindings locally bound for this datapath and - * running route exchange logic. */ - struct sset bound_ports; + * running route exchange logic. + * The key is the port name and the value is the ifname if set. This may + * be empty if the all ports specified dynamic-routing-port-name, but no + * such referenced port is local. In this case we should only advertise + * routes but not learn them. */ + struct smap bound_ports; }; struct advertise_route_entry { diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 88e37613e..bd15e0fa5 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -16101,6 +16101,11 @@ blackhole 192.0.2.3 proto 84 blackhole 192.0.2.10 proto 84 blackhole 198.51.100.0/24 proto 84]) +# Disable maintaining the vrf as some tests below would cause the vrfs to be +# removed. +check ovn-nbctl set Logical_Router_Port internet-phys \ + options:dynamic-routing-maintain-vrf=false + # Now we test route learning. check_row_count Learned_Route 0 check ip route add 233.252.0.0/24 via 192.168.10.10 dev lo onlink vrf ovnvrf1337 @@ -16111,6 +16116,46 @@ check_row_count Learned_Route 1 lp=$(fetch_column port_binding _uuid logical_port=internet-phys) check_row_count Learned_Route 1 logical_port=$lp ip_prefix=233.252.0.0/24 nexthop=192.168.10.10 +# If we remove the route it is also gone. +check ip route del 233.252.0.0/24 via 192.168.10.10 dev lo onlink vrf ovnvrf1337 +# For now we trigger a recompute as route watching is not yet implemented. +check ovn-appctl -t ovn-controller inc-engine/recompute +check ovn-nbctl --wait=hv sync +check_row_count Learned_Route 0 + +# By setting a learning interface filter will prevent us from learning the +# route again. The Port referenced by the name does not even exist. +check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \ + options:dynamic-routing-port-name=thisportdoesnotexist +check ip route add 233.252.0.0/24 via 192.168.10.10 dev lo onlink vrf ovnvrf1337 +# For now we trigger a recompute as route watching is not yet implemented. +check ovn-appctl -t ovn-controller inc-engine/recompute +check ovn-nbctl --wait=hv sync +check_row_count Learned_Route 0 + +# Setting the local ovsdb to map this port to "lo" will make route learning +# work again. +check ovs-vsctl set Open_vSwitch . \ + external-ids:dynamic-routing-port-mapping="thisisirrelevant=andjustfortesting,thisportdoesnotexist=lo" +check ovn-nbctl --wait=hv sync +check_row_count Learned_Route 1 + +# Now we try the interface filter with an existing port. +check ovn-nbctl lsp-add phys mylearninglsp +check ovs-vsctl -- add-port br-int hv1-mll -- \ + set interface hv1-mll type=internal external-ids:iface-id=mylearninglsp +check ip link set hv1-mll up +wait_for_ports_up mylearninglsp + +check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \ + options:dynamic-routing-port-name=mylearninglsp + +check ip route add 233.253.0.0/24 via 192.168.20.20 dev hv1-mll onlink vrf ovnvrf1337 +# For now we trigger a recompute as route watching is not yet implemented. +check ovn-appctl -t ovn-controller inc-engine/recompute +check ovn-nbctl --wait=hv sync +check_row_count Learned_Route 1 ip_prefix=233.253.0.0/24 nexthop=192.168.20.20 + OVS_APP_EXIT_AND_WAIT([ovn-controller]) as ovn-sb @@ -16314,16 +16359,63 @@ blackhole 192.0.2.3 proto 84 blackhole 192.0.2.10 proto 84 blackhole 198.51.100.0/24 proto 84]) +# Disable maintaining the vrf as some tests below would cause the vrfs to be +# removed. +check ovn-nbctl set Logical_Router_Port internet-phys \ + options:dynamic-routing-maintain-vrf=false + # Now we test route learning. check_row_count Learned_Route 0 check ip route add 233.252.0.0/24 via 192.168.10.10 dev lo onlink vrf ovnvrf1337 # For now we trigger a recompute as route watching is not yet implemented. check ovn-appctl -t ovn-controller inc-engine/recompute check ovn-nbctl --wait=hv sync -check_row_count Learned_Route 2 +check_row_count Learned_Route 1 lp=$(fetch_column port_binding _uuid logical_port=internet-phys) check_row_count Learned_Route 1 logical_port=$lp ip_prefix=233.252.0.0/24 nexthop=192.168.10.10 +# If we remove the route it is also gone. +check ip route del 233.252.0.0/24 via 192.168.10.10 dev lo onlink vrf ovnvrf1337 +# For now we trigger a recompute as route watching is not yet implemented. +check ovn-appctl -t ovn-controller inc-engine/recompute +check ovn-nbctl --wait=hv sync +check_row_count Learned_Route 0 + +# By setting a learning interface filter will prevent us from learning the +# route again. The Port referenced by the name does not even exist. +check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \ + options:dynamic-routing-port-name=thisportdoesnotexist +check ip route add 233.252.0.0/24 via 192.168.10.10 dev lo onlink vrf ovnvrf1337 +# For now we trigger a recompute as route watching is not yet implemented. +check ovn-appctl -t ovn-controller inc-engine/recompute +check ovn-nbctl --wait=hv sync +check_row_count Learned_Route 0 + +# Setting the local ovsdb to map this port to "lo" will make route learning +# work again. +check ovs-vsctl set Open_vSwitch . \ + external-ids:dynamic-routing-port-mapping="thisisirrelevant=andjustfortesting,thisportdoesnotexist=lo" +check ovn-nbctl --wait=hv sync +check_row_count Learned_Route 1 + +# Now we try the interface filter with an existing port. +check ovn-nbctl lsp-add phys mylearninglsp +check ovs-vsctl -- add-port br-int hv1-mll -- \ + set interface hv1-mll type=internal external-ids:iface-id=mylearninglsp +check ip link set hv1-mll up +wait_for_ports_up mylearninglsp + +check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \ + options:dynamic-routing-port-name=mylearninglsp + +check ip route add 233.253.0.0/24 via 192.168.20.20 dev hv1-mll onlink vrf ovnvrf1337 +# For now we trigger a recompute as route watching is not yet implemented. +check ovn-appctl -t ovn-controller inc-engine/recompute +check ovn-nbctl --wait=hv sync +check_row_count Learned_Route 1 ip_prefix=233.253.0.0/24 nexthop=192.168.20.20 + +OVS_APP_EXIT_AND_WAIT([ovn-controller]) + as ovn-sb OVS_APP_EXIT_AND_WAIT([ovsdb-server]) -- 2.47.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
