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, and 'route_exchange' now recomputes when it changes. 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 | 106 ++++++++++++++--- controller/route-exchange-netlink.c | 75 ++++++++++++ controller/route-exchange-netlink.h | 5 + controller/route-exchange.c | 2 +- controller/route-exchange.h | 5 + tests/ovn-inc-proc-graph-dump.at | 1 + tests/system-ovn-netlink.at | 112 +++++++++++++++++- tests/system-ovn.at | 55 +++++++++ tests/test-ovn-netlink.c | 9 +- 13 files changed, 415 insertions(+), 33 deletions(-) diff --git a/Documentation/topics/dynamic-routing/architecture.rst b/Documentation/topics/dynamic-routing/architecture.rst index e4be2ccf874d..4a363b80a87c 100644 --- a/Documentation/topics/dynamic-routing/architecture.rst +++ b/Documentation/topics/dynamic-routing/architecture.rst @@ -364,6 +364,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 40a1b9867be5..cbc425d61ee4 100644 --- a/NEWS +++ b/NEWS @@ -49,6 +49,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 the same VRF table. * Add support for hub-and-spoke propagation via the "hub-spoke" option in dynamic-routing-redistribute settings. 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 31cb30cfd3cf..7df828be454e 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -5714,6 +5714,33 @@ struct ed_type_route_table_notify { struct vector watches; }; +/* 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; @@ -5739,6 +5766,8 @@ en_route_exchange_run(struct engine_node *node, void *data) engine_get_input_data("route", node); 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); /* There can not actually be any routes to advertise unless we also have * the Learned_Route table, since they where introduced in the same @@ -5761,12 +5790,18 @@ en_route_exchange_run(struct engine_node *node, void *data) = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); ovs_assert(chassis); + /* 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 = { .ovnsb_idl_txn = engine_get_context()->ovnsb_idl_txn, .sbrec_learned_route_by_datapath = sbrec_learned_route_by_datapath, .sbrec_port_binding_by_name = sbrec_port_binding_by_name, .chassis = chassis, .announce_routes = &route_data->announce_routes, + .nexthops = &nhe_data->nexthops, }; struct route_exchange_ctx_out r_ctx_out = { .sb_changes_pending = false, @@ -5781,6 +5816,23 @@ en_route_exchange_run(struct engine_node *node, void *data) return EN_UPDATED; } +static enum engine_input_handler_result +route_exchange_nexthop_handler(struct engine_node *node, + void *data OVS_UNUSED) +{ + 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; + } + + return EN_UNHANDLED; +} + static enum engine_input_handler_result route_exchange_sb_ro_handler(struct engine_node *node OVS_UNUSED, void *data) { @@ -6480,16 +6532,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) @@ -6529,10 +6571,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); @@ -6541,20 +6594,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 == needed) { return; } - if (nhe_data->enabled && !enabled) { + 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 { @@ -6622,7 +6688,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; } @@ -7243,6 +7310,11 @@ inc_proc_ovn_controller_init( engine_noop_handler); engine_add_input(&en_route_exchange, &en_route_table_notify, NULL); 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 8f1615c4ecaf..afb94157d139 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); @@ -199,11 +200,76 @@ re_nl_delete_route(uint32_t table_id, const struct advertise_route_entry *re) return modify_route(RTM_DELROUTE, 0, table_id, re); } +/* Appends a learned route for the prefix in 'rd' reachable through the leaf + * nexthop object 'nhe' to 'learned_routes'. */ +static void +learn_route_via_nexthop(const struct nexthop_entry *nhe, + const struct route_data *rd, + 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 = rd->rta_dst, + .plen = rd->rtm_dst_len, + .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 'rd' 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 route_data *rd, + 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, rd, 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, rd, learned_routes); + } +} + struct route_msg_handle_data { struct hmapx *routes_to_advertise; struct vector *learned_routes; struct vector *stale_routes; const struct hmap *routes; + + /* Kernel nexthop objects (struct nexthop_entry), used to resolve routes + * that reference their next hop(s) through a nexthop id (RTA_NH_ID). + * Must be set whenever 'learned_routes' is. */ + const struct hmap *nexthops; }; static void @@ -236,6 +302,13 @@ handle_route_msg(const struct route_table_msg *msg, if (prefix_is_link_local(&rd->rta_dst, rd->rtm_dst_len)) { return; } + if (rd->rta_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(handle_data->nexthops, rd->rta_nhid, + rd, handle_data->learned_routes); + return; + } struct route_data_nexthop *nexthop; LIST_FOR_EACH (nexthop, nexthop_node, &rd->nexthops) { if (ipv6_is_zero(&nexthop->addr)) { @@ -320,6 +393,7 @@ re_nl_encode_nexthop(struct ofpbuf *request, bool dst_is_ipv4, int re_nl_sync_routes(uint32_t table_id, const struct hmap *routes, + const struct hmap *nexthops, struct vector *learned_routes) { struct hmapx routes_to_advertise = HMAPX_INITIALIZER(&routes_to_advertise); @@ -339,6 +413,7 @@ re_nl_sync_routes(uint32_t table_id, const struct hmap *routes, .routes_to_advertise = &routes_to_advertise, .learned_routes = learned_routes, .stale_routes = &stale_routes, + .nexthops = nexthops, }; route_table_dump_one_table(table_id, AF_INET, handle_route_msg, &data); route_table_dump_one_table(table_id, AF_INET6, handle_route_msg, &data); diff --git a/controller/route-exchange-netlink.h b/controller/route-exchange-netlink.h index c137b5119b36..cff4a6df83ac 100644 --- a/controller/route-exchange-netlink.h +++ b/controller/route-exchange-netlink.h @@ -57,7 +57,12 @@ void re_route_format(struct ds *, uint32_t table_id, const struct in6_addr *dst, unsigned int plen, const struct in6_addr *nexthop, int err); +/* Syncs the routes OVN advertises in 'table_id' and collects the routes + * learned from it into 'learned_routes'. 'nexthops' contains the kernel + * nexthop objects (struct nexthop_entry) used to resolve learned routes that + * reference their next hop through a nexthop id. */ int re_nl_sync_routes(uint32_t table_id, const struct hmap *routes, + const struct hmap *nexthops, struct vector *learned_routes); int re_nl_cleanup_routes(uint32_t table_id); diff --git a/controller/route-exchange.c b/controller/route-exchange.c index 027375071b46..e2bf51914ba7 100644 --- a/controller/route-exchange.c +++ b/controller/route-exchange.c @@ -396,7 +396,7 @@ route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in, struct vector received_routes = VECTOR_EMPTY_INITIALIZER(struct re_nl_received_route_node); error = re_nl_sync_routes(arte->table_id, arte->routes, - &received_routes); + r_ctx_in->nexthops, &received_routes); SET_ROUTE_EXCHANGE_NL_STATUS(error); struct ovsdb_idl_index *sbrec_learned_route_by_datapath = diff --git a/controller/route-exchange.h b/controller/route-exchange.h index a1ef4a359dfc..64d3e3f08ad3 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 { diff --git a/tests/ovn-inc-proc-graph-dump.at b/tests/ovn-inc-proc-graph-dump.at index dd2e0e9c0f72..49d7e61525d9 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_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 dedfd14f2d6f..1d0f3dab99c6 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() @@ -712,14 +816,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 973c4672824f..27ffddb3a4ed 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -20961,6 +20961,61 @@ ip_prefix : "10.10.3.1" ip_prefix : "10.10.4.1" ]) +# 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 + +# How many times ovn-controller rebuilt the routes it learns. A sync +# completes only after it ran the engine, and it reads the notifications right +# before running it, so a sync after a change to the nexthop table tells us +# what it decided to do about that change. +route_recomputes() { + ovn-appctl -t ovn-controller inc-engine/show-stats route_exchange recompute +} + +# The steps above leave work behind that would otherwise land in the middle of +# the measurements, 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([test $(route_recomputes) -gt $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 2019c18cf29c..072d7ee64085 100644 --- a/tests/test-ovn-netlink.c +++ b/tests/test-ovn-netlink.c @@ -202,6 +202,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); @@ -231,7 +232,11 @@ test_route_sync(struct ovs_cmdl_context *ctx) advertise_route_hash(&ar->addr, &ar->nexthop, ar->plen)); } - ovs_assert(re_nl_sync_routes(table_id, &routes_to_advertise, + /* 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(re_nl_sync_routes(table_id, &routes_to_advertise, &nexthops, &received_routes) == 0); struct ds msg = DS_EMPTY_INITIALIZER; @@ -249,6 +254,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
