Routing daemons commonly describe the next hop of a route with a separate nexthop object (see 'ip nexthop') that the route refers to through a nexthop id in the RTA_NH_ID attribute, instead of encoding the next hop inline. With the net.ipv4.nexthop_compat_mode sysctl turned off, which is what such a daemon typically asks for, the kernel reports nothing but that id, so the route carried neither a gateway nor an output interface and was never learned.
Resolve the nexthop id against the kernel nexthop table instead. A nexthop group yields one learned route per usable member. The id is used whenever the route carries one, including in compatibility mode where the kernel also reports the resolved next hop inline, so that both settings learn the same routes. The nexthop table is therefore taken from the 'nexthop_exchange' node, which already maintains it incrementally for EVPN. A change to it does not say which routes resolve differently now, so 'route_exchange' relearns the routes of every table it syncs, which it can do without reading the tables again. The RTNLGRP_NEXTHOP notifier is reference counted between the two users, and changes that only touch FDB nexthops are filtered out so that EVPN activity does not needlessly relearn routes. Resolving RTA_NH_ID depends on the matching route table parsing on the OVS side, added by "route-table: Support routes resolving through a nexthop object.". The submodule has to be updated to a version containing it before this can be used. Assisted-by: Claude Opus 5, Cursor Signed-off-by: Han Zhou <[email protected]> --- .../topics/dynamic-routing/architecture.rst | 20 +++ NEWS | 5 + controller/nexthop-exchange.c | 43 ++++-- controller/nexthop-exchange.h | 10 ++ controller/ovn-controller.c | 125 +++++++++++++++--- controller/route-exchange-netlink.c | 75 +++++++++++ controller/route-exchange-netlink.h | 5 +- controller/route-exchange.c | 25 +++- controller/route-exchange.h | 11 ++ tests/ovn-inc-proc-graph-dump.at | 1 + tests/system-ovn-netlink.at | 112 +++++++++++++++- tests/system-ovn.at | 48 +++++++ tests/test-ovn-netlink.c | 12 +- 13 files changed, 457 insertions(+), 35 deletions(-) diff --git a/Documentation/topics/dynamic-routing/architecture.rst b/Documentation/topics/dynamic-routing/architecture.rst index cf7de2b79d28..df0059e74d1c 100644 --- a/Documentation/topics/dynamic-routing/architecture.rst +++ b/Documentation/topics/dynamic-routing/architecture.rst @@ -367,6 +367,26 @@ For each qualifying route, ``ovn-controller`` creates a ``Learned_Route`` record in the Southbound database containing the datapath, logical port, IP prefix, and nexthop. +Routes that reference a next hop indirectly are also supported. Modern +routing daemons frequently install routes that point at a *nexthop object* +(see ``ip nexthop``) using a nexthop id (the ``RTA_NH_ID`` route attribute) +instead of encoding the next hop inline in the route. When such a route is +encountered, ``ovn-controller`` resolves the referenced nexthop object +against the kernel nexthop table (``RTM_GETNEXTHOP``) to obtain the actual +next hop address(es) and output interface(s). Nexthop groups are expanded +so that a single route backed by a group results in one ``Learned_Route`` +record per usable group member. With the ``net.ipv4.nexthop_compat_mode`` +sysctl enabled, which is the default, the kernel reports the resolved next +hop inline as well, but ``ovn-controller`` resolves through the nexthop id +whenever the route carries one so that both settings behave the same. + +Because several routes can share one nexthop object, changing that object +redirects all of them at once without the kernel reporting any route change. +``ovn-controller`` therefore also watches the kernel nexthop table +(``RTNLGRP_NEXTHOP``) and re-learns the affected routes whenever a nexthop +object is added, replaced or removed. The same nexthop table is used to +resolve the EVPN nexthop groups described below. + Flow Generation by ovn-northd ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/NEWS b/NEWS index aadf6416d2ff..793ffcc5ebd7 100644 --- a/NEWS +++ b/NEWS @@ -51,6 +51,11 @@ OVN v26.09.0 - xxx xx xxxx "lb-add", "meter-add", "lr-policy-add", and "lr-policy-del", and fixed the "nfg-list" signature. - Dynamic Routing: + * Learn routes whose next hop is described by a kernel nexthop object + referenced through a nexthop id (RTA_NH_ID) instead of being encoded + inline in the route. Nexthop groups are expanded into one learned + route per group member, and the learned routes are updated when the + nexthop object they reference changes. * Allow multiple routers to read from and advertise their routes to the same VRF table. * Add support for hub-and-spoke propagation via the "hub-spoke" option diff --git a/controller/nexthop-exchange.c b/controller/nexthop-exchange.c index 8075dd4937a8..ff2766e7bf40 100644 --- a/controller/nexthop-exchange.c +++ b/controller/nexthop-exchange.c @@ -54,8 +54,10 @@ nexthop_grp_weight(const struct nexthop_grp *entry) } #endif -/* Populates 'nexthops' with all nexthop entries - * (struct nexthop_entry) with fdb flag set that exist in the table. */ +/* Populates 'nexthops' with all nexthop entries (struct nexthop_entry) that + * exist in the kernel nexthop table. Both the FDB nexthops used by EVPN and + * the nexthops referenced by routes through a nexthop id are included, use + * 'is_fdb' to tell them apart. */ void nexthops_sync(struct hmap *nexthops) { @@ -96,10 +98,7 @@ void nexthop_entry_format(struct ds *ds, const struct nexthop_entry *nhe) { ds_put_format(ds, "id=%"PRIu32", ", nhe->id); - if (!nhe->n_grps) { - ds_put_cstr(ds, "address="); - ipv6_format_mapped(&nhe->addr, ds); - } else { + if (nhe->n_grps) { ds_put_cstr(ds, "group=["); for (size_t i = 0; i < nhe->n_grps; i++) { const struct nexthop_grp_entry *grp = &nhe->grps[i]; @@ -113,6 +112,15 @@ nexthop_entry_format(struct ds *ds, const struct nexthop_entry *nhe) ds_truncate(ds, ds->length - 2); ds_put_char(ds, ']'); + } else if (nhe->is_blackhole) { + ds_put_cstr(ds, "blackhole"); + } else { + ds_put_cstr(ds, "address="); + ipv6_format_mapped(&nhe->addr, ds); + } + + if (nhe->ifname[0]) { + ds_put_format(ds, ", dev=%s", nhe->ifname); } } @@ -203,6 +211,8 @@ nh_table_parse__(struct ofpbuf *buf, size_t ofs, const struct nlmsghdr *nlmsg, static const struct nl_policy policy[] = { [NHA_ID] = { .type = NL_A_U32 }, [NHA_FDB] = { .type = NL_A_FLAG, .optional = true }, + [NHA_BLACKHOLE] = { .type = NL_A_FLAG, .optional = true }, + [NHA_OIF] = { .type = NL_A_U32, .optional = true }, [NHA_GROUP] = { .type = NL_A_UNSPEC, .optional = true, .min_len = sizeof(struct nexthop_grp) }, [NHA_GATEWAY] = { .type = NL_A_UNSPEC, .optional = true, @@ -218,10 +228,6 @@ nh_table_parse__(struct ofpbuf *buf, size_t ofs, const struct nlmsghdr *nlmsg, return 0; } - if (!nl_attr_get_flag(attrs[NHA_FDB])) { - return 0; - } - const struct nexthop_grp *grps = NULL; struct in6_addr addr = in6addr_any; size_t n_grps = 0; @@ -245,8 +251,9 @@ nh_table_parse__(struct ofpbuf *buf, size_t ofs, const struct nlmsghdr *nlmsg, } else if (attrs[NHA_GROUP]) { n_grps = nl_attr_get_size(attrs[NHA_GROUP]) / sizeof *grps; grps = nl_attr_get(attrs[NHA_GROUP]); - } else { - VLOG_DBG_RL(&rl, "missing group or gateway nexthop attribute"); + } else if (!attrs[NHA_BLACKHOLE] && !attrs[NHA_OIF]) { + VLOG_DBG_RL(&rl, "missing group, gateway, oif or blackhole nexthop " + "attribute"); return 0; } @@ -256,9 +263,21 @@ nh_table_parse__(struct ofpbuf *buf, size_t ofs, const struct nlmsghdr *nlmsg, *change->nhe = (struct nexthop_entry) { .id = nl_attr_get_u32(attrs[NHA_ID]), .addr = addr, + .is_blackhole = nl_attr_get_flag(attrs[NHA_BLACKHOLE]), + .is_fdb = nl_attr_get_flag(attrs[NHA_FDB]), .n_grps = n_grps, }; + if (attrs[NHA_OIF]) { + uint32_t oif = nl_attr_get_u32(attrs[NHA_OIF]); + + if (!if_indextoname(oif, change->nhe->ifname)) { + change->nhe->ifname[0] = '\0'; + VLOG_DBG_RL(&rl, "could not find interface name for nexthop " + "%"PRIu32" if-index %"PRIu32, change->nhe->id, oif); + } + } + for (size_t i = 0; i < n_grps; i++) { const struct nexthop_grp *grp = &grps[i]; change->nhe->grps[i] = (struct nexthop_grp_entry) { diff --git a/controller/nexthop-exchange.h b/controller/nexthop-exchange.h index 73f08c2fe6c6..ff6d39b0d729 100644 --- a/controller/nexthop-exchange.h +++ b/controller/nexthop-exchange.h @@ -16,7 +16,9 @@ #ifndef NEXTHOP_EXCHANGE_H #define NEXTHOP_EXCHANGE_H 1 +#include <net/if.h> #include <netinet/in.h> +#include <stdbool.h> #include <stdint.h> #include "openvswitch/hmap.h" @@ -40,6 +42,14 @@ struct nexthop_entry { uint32_t id; /* Nexthop IP address, zeroed in case of group entry. */ struct in6_addr addr; + /* Output interface, empty string if the nexthop does not have one. + * Adding 1 to this to be sure we actually have a terminating '\0'. */ + char ifname[IFNAMSIZ + 1]; + /* True if the nexthop discards the traffic sent to it. */ + bool is_blackhole; + /* True if the nexthop belongs to a bridge FDB, i.e. it is used by EVPN + * rather than by the routing table. */ + bool is_fdb; /* Number of group entries, "0" in case of gateway entry. */ size_t n_grps; /* Array of group entries. */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index eb7c1c58c040..60c7f91b65a0 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -5731,6 +5731,33 @@ route_table_notify_clear_changes(struct ed_type_route_table_notify *rtn) vector_clear(&rtn->changed_routes); } +/* The kernel nexthop table is shared by the features below, each of them + * independently declares whether it needs it to be tracked. */ +enum nexthop_exchange_user { + NEXTHOP_EXCHANGE_USER_EVPN, + NEXTHOP_EXCHANGE_USER_ROUTE, + NEXTHOP_EXCHANGE_USER_MAX, +}; + +/* The nexthop_exchange node is an input node, but is enabled/disabled based on + * the en_neighbor_exchange and en_route_exchange nodes. The reason being that + * engine periodically runs input nodes to check if there are updates, so it + * could be polled for updates without requiring other nodes to run first. */ +struct ed_type_nexthop_exchange { + /* Contains 'struct nexthop_entry'. */ + struct hmap nexthops; + /* Set for each feature that currently needs 'nexthops'. */ + bool users[NEXTHOP_EXCHANGE_USER_MAX]; + bool enabled; + bool recompute; + /* True if the last run changed any nexthop that is not an EVPN FDB one, + * i.e. any nexthop a route may be referencing. */ + bool routing_changed; +}; + +static void nexthop_exchange_update(struct ed_type_nexthop_exchange *, + enum nexthop_exchange_user, bool enabled); + struct ed_type_route_exchange { /* We need the idl to check if the Learned_Route table exists. */ struct ovsdb_idl *sb_idl; @@ -5747,6 +5774,8 @@ route_exchange_ctx_init(struct engine_node *node, struct route_exchange_ctx_out *r_ctx_out) { struct ed_type_route *route_data = engine_get_input_data("route", node); + struct ed_type_nexthop_exchange *nhe_data = + engine_get_input_data("nexthop_exchange", node); const struct ovsrec_open_vswitch_table *ovs_table = EN_OVSDB_GET(engine_get_input("OVS_open_vswitch", node)); @@ -5769,6 +5798,7 @@ route_exchange_ctx_init(struct engine_node *node, engine_get_input("SB_port_binding", node), "name"), .chassis = chassis, .announce_routes = &route_data->announce_routes, + .nexthops = &nhe_data->nexthops, }; *r_ctx_out = (struct route_exchange_ctx_out) { .sb_changes_pending = false, @@ -5781,6 +5811,9 @@ en_route_exchange_run(struct engine_node *node, void *data) struct ed_type_route_exchange *re = data; struct ed_type_route_table_notify *rt_notify = engine_get_input_data("route_table_notify", node); + struct ed_type_nexthop_exchange *nhe_data = + engine_get_input_data("nexthop_exchange", node); + struct ed_type_route *route_data = engine_get_input_data("route", node); /* There can not actually be any routes to advertise unless we also have * the Learned_Route table, since they where introduced in the same @@ -5791,6 +5824,11 @@ en_route_exchange_run(struct engine_node *node, void *data) vector_clear(&rt_notify->watches); + /* Routes we learn may reference their next hop through a nexthop id, which + * we can only resolve while the kernel nexthop table is tracked. */ + nexthop_exchange_update(nhe_data, NEXTHOP_EXCHANGE_USER_ROUTE, + !hmap_is_empty(&route_data->announce_routes)); + struct route_exchange_ctx_in r_ctx_in; struct route_exchange_ctx_out r_ctx_out; route_exchange_ctx_init(node, &r_ctx_in, &r_ctx_out); @@ -5837,6 +5875,39 @@ route_exchange_route_table_handler(struct engine_node *node, void *data) return EN_HANDLED_UPDATED; } +static enum engine_input_handler_result +route_exchange_nexthop_handler(struct engine_node *node, void *data) +{ + struct ed_type_route_exchange *re = data; + struct ed_type_nexthop_exchange *nhe_data = + engine_get_input_data("nexthop_exchange", node); + + /* Only the nexthops a route can reference through a nexthop id matter + * here, changes limited to the FDB nexthops used by EVPN cannot affect + * the routes we learn. */ + if (!nhe_data->routing_changed) { + return EN_HANDLED_UNCHANGED; + } + + struct route_exchange_ctx_in r_ctx_in; + struct route_exchange_ctx_out r_ctx_out; + route_exchange_ctx_init(node, &r_ctx_in, &r_ctx_out); + + switch (route_exchange_handle_nexthop_changes(re->state, &r_ctx_in, + &r_ctx_out)) { + case ROUTE_EXCHANGE_UNHANDLED: + return EN_UNHANDLED; + case ROUTE_EXCHANGE_UNCHANGED: + return EN_HANDLED_UNCHANGED; + case ROUTE_EXCHANGE_UPDATED: + break; + } + + re->sb_changes_pending |= r_ctx_out.sb_changes_pending; + + return EN_HANDLED_UPDATED; +} + static enum engine_input_handler_result route_exchange_sb_ro_handler(struct engine_node *node OVS_UNUSED, void *data) { @@ -6556,16 +6627,6 @@ en_neighbor_table_notify_run(struct engine_node *node OVS_UNUSED, return state; } -/* The nexthop_exchange node is an input node, but is enabled/disabled - * based on en_neighbor_exchange node. The reason being that engine - * periodically runs input nodes to check if there are updates, so it could - * be polled for updates without requiring other nodes to run first. */ -struct ed_type_nexthop_exchange { - struct hmap nexthops; - bool enabled; - bool recompute; -}; - static void * en_nexthop_exchange_init(struct engine_node *node OVS_UNUSED, struct engine_arg *arg OVS_UNUSED) @@ -6611,10 +6672,21 @@ en_nexthop_exchange_run(struct engine_node *node OVS_UNUSED, void *data) ovn_netlink_notifier_flush(OVN_NL_NOTIFIER_NEXTHOP); nhe_data->recompute = false; + nhe_data->routing_changed = true; return EN_UPDATED; } struct vector *msgs = ovn_netlink_get_msgs(OVN_NL_NOTIFIER_NEXTHOP); + + nhe_data->routing_changed = false; + const struct nh_table_msg *msg; + VECTOR_FOR_EACH_PTR (msgs, msg) { + if (!msg->nhe->is_fdb) { + nhe_data->routing_changed = true; + break; + } + } + bool updated = nexthops_handle_changes(&nhe_data->nexthops, msgs); ovn_netlink_notifier_flush(OVN_NL_NOTIFIER_NEXTHOP); @@ -6623,20 +6695,33 @@ en_nexthop_exchange_run(struct engine_node *node OVS_UNUSED, void *data) static void nexthop_exchange_update(struct ed_type_nexthop_exchange *nhe_data, - bool enabled) + enum nexthop_exchange_user user, bool enabled) { - if (nhe_data->enabled == enabled) { + if (nhe_data->users[user] == enabled) { return; } + nhe_data->users[user] = enabled; + + bool needed = false; + for (size_t i = 0; i < NEXTHOP_EXCHANGE_USER_MAX; i++) { + needed = needed || nhe_data->users[i]; + } - if (nhe_data->enabled && !enabled) { + if (nhe_data->enabled == needed) { + return; + } + + if (!needed) { nexthops_destroy(&nhe_data->nexthops); - } else if (!nhe_data->enabled && enabled) { + } else { nhe_data->recompute = true; + /* The table is only dumped the next time this node runs, make sure + * that happens without waiting for an unrelated event. */ + poll_immediate_wake(); } - nhe_data->enabled = enabled; - ovn_netlink_update_notifier(OVN_NL_NOTIFIER_NEXTHOP, enabled); + nhe_data->enabled = needed; + ovn_netlink_update_notifier(OVN_NL_NOTIFIER_NEXTHOP, needed); } struct ed_type_neighbor_exchange { @@ -6704,7 +6789,8 @@ en_neighbor_exchange_run(struct engine_node *node, void *data_) neighbor_exchange_run(&n_ctx_in, &n_ctx_out); neighbor_table_notify_update(&nt_notify->watches); - nexthop_exchange_update(nhe_data, !vector_is_empty(&nt_notify->watches)); + nexthop_exchange_update(nhe_data, NEXTHOP_EXCHANGE_USER_EVPN, + !vector_is_empty(&nt_notify->watches)); return EN_UPDATED; } @@ -7396,6 +7482,11 @@ inc_proc_ovn_controller_init( engine_add_input(&en_route_exchange, &en_route_table_notify, route_exchange_route_table_handler); engine_add_input(&en_route_exchange, &en_route_exchange_status, NULL); + /* Routes referencing a nexthop id have to be re-learned whenever the + * nexthop object they point at changes, the route itself is not updated + * by the kernel in that case. */ + engine_add_input(&en_route_exchange, &en_nexthop_exchange, + route_exchange_nexthop_handler); engine_add_input(&en_route_exchange, &en_sb_ro, route_exchange_sb_ro_handler); diff --git a/controller/route-exchange-netlink.c b/controller/route-exchange-netlink.c index d2b8fb01e2a3..19883b3c3f67 100644 --- a/controller/route-exchange-netlink.c +++ b/controller/route-exchange-netlink.c @@ -32,6 +32,7 @@ #include "route.h" #include "vec.h" +#include "nexthop-exchange.h" #include "route-exchange-netlink.h" VLOG_DEFINE_THIS_MODULE(route_exchange_netlink); @@ -221,6 +222,7 @@ ovn_route_msg_from_route_data(uint16_t nlmsg_type, msg->plen = rd->rtm_dst_len; msg->protocol = rd->rtm_protocol; msg->priority = rd->rta_priority; + msg->nhid = rd->rta_nhid; msg->n_nexthops = n_nexthops; struct ovn_route_nexthop *nh = msg->nexthops; @@ -248,6 +250,11 @@ ovn_route_msg_format(struct ds *ds, const struct ovn_route_msg *msg) ds_put_format(ds, " plen=%u proto=%u priority=%"PRIu32, msg->plen, msg->protocol, msg->priority); + if (msg->nhid) { + ds_put_format(ds, " nhid=%"PRIu32, msg->nhid); + return; + } + for (size_t i = 0; i < msg->n_nexthops; i++) { const struct ovn_route_nexthop *nh = &msg->nexthops[i]; @@ -351,10 +358,78 @@ re_nl_cached_routes_clear(struct hmap *routes) } } +/* Appends a learned route for the prefix in 'msg' reachable through the leaf + * nexthop object 'nhe' to 'learned_routes'. */ +static void +learn_route_via_nexthop(const struct nexthop_entry *nhe, + const struct ovn_route_msg *msg, + struct vector *learned_routes) +{ + if (ipv6_is_zero(&nhe->addr)) { + /* Blackhole next hop, or an address on the local link. As we just + * want to learn remote routes we do not need it. */ + return; + } + + struct re_nl_received_route_node rr = (struct re_nl_received_route_node) { + .prefix = msg->prefix, + .plen = msg->plen, + .nexthop = nhe->addr, + }; + ovs_strlcpy(rr.ifname, nhe->ifname, sizeof rr.ifname); + + vector_push(learned_routes, &rr); +} + +/* Resolves the kernel nexthop object identified by 'id' against 'nexthops' + * and appends a learned route for the prefix in 'msg' to 'learned_routes' for + * each usable next hop. A nexthop group yields one learned route per + * member. */ +static void +learn_routes_via_nexthop_id(const struct hmap *nexthops, uint32_t id, + const struct ovn_route_msg *msg, + struct vector *learned_routes) +{ + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); + + const struct nexthop_entry *nhe = nexthop_entry_find(nexthops, id); + if (!nhe) { + VLOG_DBG_RL(&rl, "could not resolve nexthop id %"PRIu32, id); + return; + } + + if (!nhe->n_grps) { + learn_route_via_nexthop(nhe, msg, learned_routes); + return; + } + + for (size_t i = 0; i < nhe->n_grps; i++) { + const struct nexthop_grp_entry *grp = &nhe->grps[i]; + + /* The kernel does not allow a nexthop group to contain other groups, + * so a single level of indirection is all we have to follow. */ + if (!grp->gateway) { + VLOG_DBG_RL(&rl, "could not resolve member %"PRIu32" of nexthop " + "group %"PRIu32, grp->id, id); + continue; + } + + learn_route_via_nexthop(grp->gateway, msg, learned_routes); + } +} + void re_nl_resolve_route(const struct ovn_route_msg *msg, + const struct hmap *nexthops, struct vector *learned_routes) { + if (msg->nhid) { + /* The next hop(s) are not encoded in the route itself, they are + * described by a separate kernel nexthop object. */ + learn_routes_via_nexthop_id(nexthops, msg->nhid, msg, learned_routes); + return; + } + for (size_t i = 0; i < msg->n_nexthops; i++) { const struct ovn_route_nexthop *nh = &msg->nexthops[i]; diff --git a/controller/route-exchange-netlink.h b/controller/route-exchange-netlink.h index 736a05c0fc97..1c8e675db44b 100644 --- a/controller/route-exchange-netlink.h +++ b/controller/route-exchange-netlink.h @@ -98,8 +98,11 @@ struct re_nl_received_route_node { }; /* Turns the route 'msg' into the routes OVN learns from it, appending them to - * 'learned_routes'. */ + * 'learned_routes'. 'nexthops' contains the kernel nexthop objects (struct + * nexthop_entry) used to resolve a route that references its next hop through + * a nexthop id. */ void re_nl_resolve_route(const struct ovn_route_msg *, + const struct hmap *nexthops, struct vector *learned_routes); 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 20a1ac7cf564..20e85e83a94c 100644 --- a/controller/route-exchange.c +++ b/controller/route-exchange.c @@ -485,7 +485,7 @@ route_table_resolve_and_sync( const struct re_nl_cached_route *cr; HMAP_FOR_EACH (cr, node, &rt->learned_routes) { - re_nl_resolve_route(cr->msg, &received_routes); + re_nl_resolve_route(cr->msg, r_ctx_in->nexthops, &received_routes); } struct hmapx_node *dp_node; @@ -582,6 +582,29 @@ out: return handled; } +enum route_exchange_handled +route_exchange_handle_nexthop_changes( + struct route_exchange_state *state, + const struct route_exchange_ctx_in *r_ctx_in, + struct route_exchange_ctx_out *r_ctx_out) +{ + struct hmapx changed_tables = HMAPX_INITIALIZER(&changed_tables); + + /* A route names a nexthop object, the object does not name the routes + * using it, so every table we know of may resolve differently now. */ + struct route_table_state *rt; + HMAP_FOR_EACH (rt, node, &state->tables) { + hmapx_add(&changed_tables, rt); + } + + enum route_exchange_handled handled = + resync_changed_tables(&changed_tables, r_ctx_in, r_ctx_out); + + hmapx_destroy(&changed_tables); + + return handled; +} + void route_exchange_run(struct route_exchange_state *state, const struct route_exchange_ctx_in *r_ctx_in, diff --git a/controller/route-exchange.h b/controller/route-exchange.h index f52ea505585c..1845782e0e2c 100644 --- a/controller/route-exchange.h +++ b/controller/route-exchange.h @@ -28,6 +28,11 @@ struct route_exchange_ctx_in { /* Contains struct advertise_datapath_entry */ const struct hmap *announce_routes; + + /* Contains struct nexthop_entry, the kernel nexthop objects used to + * resolve learned routes that reference their next hop through a nexthop + * id. */ + const struct hmap *nexthops; }; struct route_exchange_ctx_out { @@ -63,6 +68,12 @@ enum route_exchange_handled route_exchange_handle_route_changes( struct route_exchange_state *, const struct route_exchange_ctx_in *, struct route_exchange_ctx_out *, const struct vector *changed_routes); +/* Updates the routes OVN learned after the kernel nexthop objects they may + * resolve through changed. */ +enum route_exchange_handled route_exchange_handle_nexthop_changes( + struct route_exchange_state *, const struct route_exchange_ctx_in *, + struct route_exchange_ctx_out *); + void route_exchange_cleanup_vrfs(void); void route_exchange_destroy(void); diff --git a/tests/ovn-inc-proc-graph-dump.at b/tests/ovn-inc-proc-graph-dump.at index b0c29d1b5535..5fd8530fcbaf 100644 --- a/tests/ovn-inc-proc-graph-dump.at +++ b/tests/ovn-inc-proc-graph-dump.at @@ -473,6 +473,7 @@ digraph "Incremental-Processing-Engine" { SB_port_binding -> route_exchange [[label="engine_noop_handler"]]; route_table_notify -> route_exchange [[label="route_exchange_route_table_handler"]]; route_exchange_status -> route_exchange [[label=""]]; + nexthop_exchange -> route_exchange [[label="route_exchange_nexthop_handler"]]; sb_ro -> route_exchange [[label="route_exchange_sb_ro_handler"]]; garp_rarp [[style=filled, shape=box, fillcolor=white, label="garp_rarp"]]; OVS_open_vswitch -> garp_rarp [[label=""]]; diff --git a/tests/system-ovn-netlink.at b/tests/system-ovn-netlink.at index 69c25d6e9544..ee7272394e58 100644 --- a/tests/system-ovn-netlink.at +++ b/tests/system-ovn-netlink.at @@ -569,6 +569,110 @@ fe80::/64 dev lo-test proto kernel metric 256 pref medium]) AT_CLEANUP +AT_SETUP([sync netlink routes - learn routes via nexthop id]) +AT_KEYWORDS([netlink-routes]) +CHECK_VRF() + +dnl Skip if the running kernel or iproute2 has no nexthop object support. +AT_SKIP_IF([! ip nexthop show >/dev/null 2>&1]) + +dnl Make the kernel describe these routes through the nexthop id alone. In +dnl compatibility mode, which is the default, it reports the resolved next +dnl hop inline as well and OVN would not have to resolve anything. +compat_mode=$(sysctl -n net.ipv4.nexthop_compat_mode) +on_exit "sysctl -wq net.ipv4.nexthop_compat_mode=$compat_mode" +check sysctl -wq net.ipv4.nexthop_compat_mode=0 + +table_id=100 + +check ip link add vrf-$table_id type vrf table $table_id +on_exit 'ip link del vrf-$table_id' +dnl Routes can outlive the VRF they were added to, so make sure the ones this +dnl test adds do not leak into the following tests. +on_exit 'ip -6 route flush vrf vrf-$table_id' +on_exit 'ip route flush vrf vrf-$table_id' +check ip link set dev vrf-$table_id up + +check ip link add lo-test type dummy +on_exit 'ip link del lo-test' +check ip link set lo-test master vrf-$table_id +check ip link set lo-test address 00:00:00:00:00:10 +check ip addr add 20.0.0.10/24 dev lo-test +check ip addr add fd20::10/64 dev lo-test +check ip link set up lo-test + +dnl Drop the routes a previous test may have left behind in this table. +check ovstest test-ovn-netlink route-sync $table_id + +dnl Create standalone nexthop objects and a nexthop group referencing them. +dnl These are the "nhid" next hops that OVN has to resolve against the kernel +dnl nexthop table. +nh_v4=$(nexthop_alloc) +check ip nexthop add id $nh_v4 via 20.0.0.1 dev lo-test +on_exit "ip nexthop del id $nh_v4" +nh_v4_b=$(nexthop_alloc) +check ip nexthop add id $nh_v4_b via 20.0.0.2 dev lo-test +on_exit "ip nexthop del id $nh_v4_b" +nh_v6=$(nexthop_alloc) +check ip nexthop add id $nh_v6 via fd20::1 dev lo-test +on_exit "ip nexthop del id $nh_v6" +nh_grp=$(nexthop_alloc) +check ip nexthop add id $nh_grp group $nh_v4/$nh_v4_b +on_exit "ip nexthop del id $nh_grp" + +dnl Add routes that reference their next hop(s) indirectly through a nexthop +dnl id instead of carrying the next hop inline. Use a dynamic routing +dnl protocol so that OVN actually learns them. +check ip route add 10.10.10.0/24 nhid $nh_v4 vrf vrf-$table_id proto bgp +check ip route add 10.10.30.0/24 nhid $nh_grp vrf vrf-$table_id proto bgp +check ip -6 route add fd20:100::/64 nhid $nh_v6 vrf vrf-$table_id proto bgp + +dnl OVN should learn all of them, expanding the nexthop group into one +dnl learned route per member. +OVS_WAIT_FOR_OUTPUT_UNQUOTED([ovstest test-ovn-netlink route-sync \ + $table_id | sort], [0], [dnl +Route table_id=$table_id dst=10.10.10.0 plen=24 nexthop=20.0.0.1 +Route table_id=$table_id dst=10.10.30.0 plen=24 nexthop=20.0.0.1 +Route table_id=$table_id dst=10.10.30.0 plen=24 nexthop=20.0.0.2 +Route table_id=$table_id dst=fd20:100:: plen=64 nexthop=fd20::1 +]) + +dnl Replacing a nexthop object changes where the routes referencing it point, +dnl without the kernel touching the routes themselves. +check ip nexthop replace id $nh_v4 via 20.0.0.9 dev lo-test + +OVS_WAIT_FOR_OUTPUT_UNQUOTED([ovstest test-ovn-netlink route-sync \ + $table_id | sort], [0], [dnl +Route table_id=$table_id dst=10.10.10.0 plen=24 nexthop=20.0.0.9 +Route table_id=$table_id dst=10.10.30.0 plen=24 nexthop=20.0.0.2 +Route table_id=$table_id dst=10.10.30.0 plen=24 nexthop=20.0.0.9 +Route table_id=$table_id dst=fd20:100:: plen=64 nexthop=fd20::1 +]) + +dnl A blackhole nexthop has no usable next hop, so nothing is learned for the +dnl routes pointing at it. +check ip -6 nexthop replace id $nh_v6 blackhole + +OVS_WAIT_FOR_OUTPUT_UNQUOTED([ovstest test-ovn-netlink route-sync \ + $table_id | sort], [0], [dnl +Route table_id=$table_id dst=10.10.10.0 plen=24 nexthop=20.0.0.9 +Route table_id=$table_id dst=10.10.30.0 plen=24 nexthop=20.0.0.2 +Route table_id=$table_id dst=10.10.30.0 plen=24 nexthop=20.0.0.9 +]) + +dnl Leaving the kernel in its default compatibility mode, where the resolved +dnl next hop is reported inline as well, must not change what is learned. +check sysctl -wq net.ipv4.nexthop_compat_mode=1 + +OVS_WAIT_FOR_OUTPUT_UNQUOTED([ovstest test-ovn-netlink route-sync \ + $table_id | sort], [0], [dnl +Route table_id=$table_id dst=10.10.10.0 plen=24 nexthop=20.0.0.9 +Route table_id=$table_id dst=10.10.30.0 plen=24 nexthop=20.0.0.2 +Route table_id=$table_id dst=10.10.30.0 plen=24 nexthop=20.0.0.9 +]) + +AT_CLEANUP + AT_SETUP([sync netlink routes - table notify]) AT_KEYWORDS([netlink-routes]) CHECK_VRF() @@ -792,14 +896,18 @@ Delete nexthop id=11, group=[[2;1]] Delete nexthop id=2, address=fd20::2 ]) -dnl Should NOT notify if a blackhole nexthop is added. +dnl Routes reference nexthop objects that are not FDB ones, so those have to +dnl be reported as well. +dnl Should notify if a blackhole nexthop is added. NS_CHECK_EXEC([nh], [ovstest test-ovn-netlink nexthop-table-notify \ "ip nexthop add id 1 blackhole"], [0], [dnl +Add nexthop id=1, blackhole ]) -dnl Should NOT notify if non-FDB nexthop is added. +dnl Should notify if non-FDB nexthop is added. NS_CHECK_EXEC([nh], [ovstest test-ovn-netlink nexthop-table-notify \ "ip nexthop add id 2 via 127.0.0.2 dev lo"], [0], [dnl +Add nexthop id=2, address=127.0.0.2, dev=lo ]) AT_CLEANUP diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 766d838190c9..fb1e3032882c 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -21084,6 +21084,54 @@ AT_CHECK_UNQUOTED([route_recomputes], [0], [$re_recompute ]) check ip route del 10.10.7.1 vrf vrf-$vni +# A route can name its next hop through a kernel nexthop object instead of +# carrying it inline. The kernel does not report the route again when the +# object it names changes, so ovn-controller has to watch the nexthop table. +# Skip this part where nexthop objects are not available. +if ip nexthop show > /dev/null 2>&1; then + +AS_BOX([$(date +%H:%M:%S.%03N) Learned route through a nexthop object]) + +# Make the kernel describe the route through the nexthop id alone. In +# compatibility mode, which is the default, it reports the resolved next hop +# inline as well and reannounces the route whenever the object changes, so the +# nexthop table would not be what drives the relearning. +compat_mode=$(sysctl -n net.ipv4.nexthop_compat_mode) +on_exit "sysctl -wq net.ipv4.nexthop_compat_mode=$compat_mode" +check sysctl -wq net.ipv4.nexthop_compat_mode=0 + +nh_used=$(nexthop_alloc) +check ip nexthop add id $nh_used via 20.0.0.25 dev local-bgp-port +on_exit "ip nexthop del id $nh_used" +check ip route add 10.10.5.1 nhid $nh_used vrf vrf-$vni proto zebra + +wait_row_count Learned_Route 1 ip_prefix=10.10.5.1 nexthop=20.0.0.25 + +# Adding the route above leaves work behind that would otherwise land in the +# middle of the check below, wait for it to finish first. +check ovn-nbctl --wait=hv sync +OVS_WAIT_UNTIL([settled=$(route_recomputes); sleep 2; + test "$settled" = "$(route_recomputes)"]) + +# Replacing the object the route names moves the route, even though the +# kernel reports nothing about the route itself. Relearning has to happen for +# this change, waiting for the route to move would also accept it happening +# later on for an unrelated reason. +re_recompute=$(route_recomputes) +check ip nexthop replace id $nh_used via 20.0.0.27 dev local-bgp-port +check ovn-nbctl --wait=hv sync +AT_CHECK_UNQUOTED([route_recomputes], [0], [$re_recompute +]) + +wait_row_count Learned_Route 1 ip_prefix=10.10.5.1 nexthop=20.0.0.27 + +# Restore what the rest of the test expects to find. +check ip route del 10.10.5.1 vrf vrf-$vni +wait_row_count Learned_Route 0 ip_prefix=10.10.5.1 +check sysctl -wq net.ipv4.nexthop_compat_mode=$compat_mode + +fi + # Verify that we can have one router read and another write from the same # vrf table. # diff --git a/tests/test-ovn-netlink.c b/tests/test-ovn-netlink.c index f75a6f295cab..c2440ba8ca11 100644 --- a/tests/test-ovn-netlink.c +++ b/tests/test-ovn-netlink.c @@ -195,6 +195,7 @@ test_host_if_monitor(struct ovs_cmdl_context *ctx) static int sync_and_resolve_routes(uint32_t table_id, const struct hmap *routes_to_advertise, + const struct hmap *nexthops, struct vector *received_routes) { struct hmap learned_routes = HMAP_INITIALIZER(&learned_routes); @@ -208,7 +209,7 @@ sync_and_resolve_routes(uint32_t table_id, const struct re_nl_cached_route *cr; HMAP_FOR_EACH (cr, node, &learned_routes) { - re_nl_resolve_route(cr->msg, received_routes); + re_nl_resolve_route(cr->msg, nexthops, received_routes); } re_nl_cached_routes_clear(&learned_routes); @@ -229,6 +230,7 @@ test_route_sync(struct ovs_cmdl_context *ctx) } struct hmap routes_to_advertise = HMAP_INITIALIZER(&routes_to_advertise); + struct hmap nexthops = HMAP_INITIALIZER(&nexthops); struct vector received_routes = VECTOR_EMPTY_INITIALIZER(struct re_nl_received_route_node); @@ -258,8 +260,12 @@ test_route_sync(struct ovs_cmdl_context *ctx) advertise_route_hash(&ar->addr, &ar->nexthop, ar->plen)); } + /* Learned routes may reference their next hop through a nexthop id, which + * can only be resolved against the kernel nexthop table. */ + nexthops_sync(&nexthops); + ovs_assert(sync_and_resolve_routes(table_id, &routes_to_advertise, - &received_routes) == 0); + &nexthops, &received_routes) == 0); struct ds msg = DS_EMPTY_INITIALIZER; @@ -276,6 +282,8 @@ done: free(e); } hmap_destroy(&routes_to_advertise); + nexthops_destroy(&nexthops); + hmap_destroy(&nexthops); vector_destroy(&received_routes); ds_destroy(&msg); } -- 2.38.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
