On Thu, Sep 10, 2026 at 9:52 AM Han Zhou <[email protected]> wrote:
> Every notification about a change to a kernel routing table made
> route_exchange recompute, which reads all watched tables, for both address
> families, and rebuilds the Learned_Route rows of every datapath that uses
> them. A routing daemon that reconverges sends a burst of such
> notifications, and none of them says anything about the tables OVN is not
> interested in.
>
> Keep the routes read from a table and let the notifications maintain them.
> A route change is applied to the routes of its table, and only the
> Learned_Route rows of that table are written again. A change to a route
> OVN
> does not learn from, e.g. one a user configured in the VRF, ends there.
>
> Reading the tables again is left to the cases that need it: the routes OVN
> advertises or the datapaths that distribute them changed, notifications
> were
> missed, or the southbound database cannot be written to right now.
>
> With 10000 routes learned from a single table, handling one route
> notification takes around 40ms, where reading that table again takes 94ms.
> What is saved is the netlink dump of both address families. Resolving the
> routes of the table and reconciling the Learned_Route rows of its datapaths
> remain, and both still scale with the size of the table rather than with
> the
> size of the change.
>
> Assisted-by: Claude Opus 5, Cursor
> Signed-off-by: Han Zhou <[email protected]>
> ---
>
Hi Han,
thank you for the patch. I have two small comments down below.
> .../topics/dynamic-routing/architecture.rst | 7 +-
> controller/ovn-controller.c | 112 ++++++---
> controller/route-exchange.c | 219 ++++++++++++++++--
> controller/route-exchange.h | 28 ++-
> tests/ovn-inc-proc-graph-dump.at | 2 +-
> tests/system-ovn-netlink.at | 74 ++++++
> tests/system-ovn.at | 39 ++++
> tests/test-ovn-netlink.c | 76 ++++++
> 8 files changed, 508 insertions(+), 49 deletions(-)
>
> diff --git a/Documentation/topics/dynamic-routing/architecture.rst
> b/Documentation/topics/dynamic-routing/architecture.rst
> index e4be2ccf874d..cf7de2b79d28 100644
> --- a/Documentation/topics/dynamic-routing/architecture.rst
> +++ b/Documentation/topics/dynamic-routing/architecture.rst
> @@ -346,8 +346,11 @@ routing daemons. This monitoring is performed via
> Netlink route
> notifications (``RTNLGRP_IPV4_ROUTE`` and ``RTNLGRP_IPV6_ROUTE``).
>
> When a route change is detected in a watched VRF table,
> -``ovn-controller`` dumps the table contents and processes each route.
> -The following filtering rules apply:
> +``ovn-controller`` applies it to the routes it already knows about in that
> +table and updates the ``Learned_Route`` records that follow from them.
> The
> +table itself is read in full only when ``ovn-controller`` starts watching
> it,
> +when the configuration of the logical routers using it changes, or when a
> +notification was missed. The following filtering rules apply:
>
> - Routes with protocol ``RTPROT_OVN`` are **skipped** because they were
> installed by ``ovn-controller`` itself (advertised routes).
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 244489b18d50..eb7c1c58c040 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -5716,6 +5716,9 @@ struct ed_type_route_table_notify {
> /* Routes ('struct ovn_route_msg *', owned) the last run was told
> about,
> * limited to the tables in 'watches'. */
> struct vector changed_routes;
> + /* Set when notifications were missed, in which case 'changed_routes'
> does
> + * not describe everything that happened to the watched tables. */
> + bool resync;
> };
>
> static void
> @@ -5734,34 +5737,17 @@ struct ed_type_route_exchange {
> /* Set to true when SB is readonly and we have routes that need
> * to be inserted into SB. */
> bool sb_changes_pending;
> + /* What the last run learned from the kernel routing tables. */
> + struct route_exchange_state *state;
> };
>
> -static enum engine_node_state
> -en_route_exchange_run(struct engine_node *node, void *data)
> +static void
> +route_exchange_ctx_init(struct engine_node *node,
> + struct route_exchange_ctx_in *r_ctx_in,
> + struct route_exchange_ctx_out *r_ctx_out)
> {
> - struct ed_type_route_exchange *re = data;
> - struct ovsdb_idl_index *sbrec_learned_route_by_datapath =
> - engine_ovsdb_node_get_index(
> - engine_get_input("SB_learned_route", node),
> - "datapath");
> -
> - struct ovsdb_idl_index *sbrec_port_binding_by_name =
> - engine_ovsdb_node_get_index(
> - engine_get_input("SB_port_binding", node),
> - "name");
> - struct ed_type_route *route_data =
> - engine_get_input_data("route", node);
> - struct ed_type_route_table_notify *rt_notify =
> - engine_get_input_data("route_table_notify", 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
> - * release. */
> - if (!sbrec_server_has_learned_route_table(re->sb_idl)) {
> - return EN_STALE;
> - }
> + struct ed_type_route *route_data = engine_get_input_data("route",
> node);
>
> - vector_clear(&rt_notify->watches);
> const struct ovsrec_open_vswitch_table *ovs_table =
> EN_OVSDB_GET(engine_get_input("OVS_open_vswitch", node));
> const char *chassis_id = get_ovs_chassis_id(ovs_table);
> @@ -5775,19 +5761,42 @@ en_route_exchange_run(struct engine_node *node,
> void *data)
> = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
> ovs_assert(chassis);
>
> - struct route_exchange_ctx_in r_ctx_in = {
> + *r_ctx_in = (struct route_exchange_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,
> + .sbrec_learned_route_by_datapath = engine_ovsdb_node_get_index(
> + engine_get_input("SB_learned_route", node), "datapath"),
> + .sbrec_port_binding_by_name = engine_ovsdb_node_get_index(
> + engine_get_input("SB_port_binding", node), "name"),
> .chassis = chassis,
> .announce_routes = &route_data->announce_routes,
> };
> - struct route_exchange_ctx_out r_ctx_out = {
> + *r_ctx_out = (struct route_exchange_ctx_out) {
> .sb_changes_pending = false,
> - .route_table_watches = &rt_notify->watches,
> };
> +}
> +
> +static enum engine_node_state
> +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);
>
> - route_exchange_run(&r_ctx_in, &r_ctx_out);
> + /* There can not actually be any routes to advertise unless we also
> have
> + * the Learned_Route table, since they where introduced in the same
> + * release. */
> + if (!sbrec_server_has_learned_route_table(re->sb_idl)) {
> + return EN_STALE;
> + }
> +
> + vector_clear(&rt_notify->watches);
> +
> + 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);
> + r_ctx_out.route_table_watches = &rt_notify->watches;
> +
> + route_exchange_run(re->state, &r_ctx_in, &r_ctx_out);
> route_table_notify_update(&rt_notify->watches);
>
> re->sb_changes_pending = r_ctx_out.sb_changes_pending;
> @@ -5795,6 +5804,39 @@ en_route_exchange_run(struct engine_node *node,
> void *data)
> return EN_UPDATED;
> }
>
> +static enum engine_input_handler_result
> +route_exchange_route_table_handler(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);
> +
> + /* We were not told about every change, so the routes we know of are
> not
> + * necessarily the ones the kernel has. */
> + if (rt_notify->resync) {
> + return EN_UNHANDLED;
> + }
> +
> + 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_route_changes(re->state, &r_ctx_in,
> + &r_ctx_out,
> +
> &rt_notify->changed_routes)) {
> + 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)
> {
> @@ -5814,12 +5856,15 @@ en_route_exchange_init(struct engine_node *node
> OVS_UNUSED,
> struct ed_type_route_exchange *re = xzalloc(sizeof *re);
>
> re->sb_idl = arg->sb_idl;
> + re->state = route_exchange_state_create();
> return re;
> }
>
> static void
> -en_route_exchange_cleanup(void *data OVS_UNUSED)
> +en_route_exchange_cleanup(void *data)
> {
> + struct ed_type_route_exchange *re = data;
> + route_exchange_state_destroy(re->state);
> }
>
> /* The route_table_notify node is an input node, but the watches are
> @@ -5837,11 +5882,13 @@ en_route_table_notify_run(struct engine_node *node
> OVS_UNUSED, void *data)
> struct ed_type_route_table_notify *rtn = data;
>
> route_table_notify_clear_changes(rtn);
> + rtn->resync = false;
>
> for (size_t i = 0; i < ARRAY_SIZE(route_notifiers); i++) {
> /* We cannot tell whether a table we watch was among the changes
> we
> * missed, so assume it was. */
> if (ovn_netlink_notifier_lost(route_notifiers[i])) {
> + rtn->resync = true;
> state = EN_UPDATED;
> }
>
> @@ -7346,7 +7393,8 @@ inc_proc_ovn_controller_init(
> engine_noop_handler);
> engine_add_input(&en_route_exchange, &en_sb_port_binding,
> engine_noop_handler);
> - engine_add_input(&en_route_exchange, &en_route_table_notify, NULL);
> + 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);
> engine_add_input(&en_route_exchange, &en_sb_ro,
> route_exchange_sb_ro_handler);
> diff --git a/controller/route-exchange.c b/controller/route-exchange.c
> index 492697503381..20a1ac7cf564 100644
> --- a/controller/route-exchange.c
> +++ b/controller/route-exchange.c
> @@ -31,6 +31,7 @@
> #include "binding.h"
> #include "ha-chassis.h"
> #include "local_data.h"
> +#include "nexthop-exchange.h"
> #include "route.h"
> #include "route-exchange.h"
> #include "route-exchange-netlink.h"
> @@ -43,6 +44,19 @@ struct maintained_route_table_entry {
> uint32_t table_id;
> };
>
> +/* What route_exchange knows about one kernel routing table it syncs. */
> +struct route_table_state {
> + struct hmap_node node;
> + uint32_t table_id;
> + /* Routes of the table OVN learns from (struct re_nl_cached_route). */
> + struct hmap learned_routes;
> +};
> +
> +struct route_exchange_state {
> + /* Contains 'struct route_table_state', by table id. */
> + struct hmap tables;
> +};
> +
> static struct hmap _maintained_route_tables =
> HMAP_INITIALIZER(&_maintained_route_tables);
> static struct sset _maintained_vrfs = SSET_INITIALIZER(&_maintained_vrfs);
> @@ -86,6 +100,69 @@ maintained_route_table_add(uint32_t table_id)
> hmap_insert(&_maintained_route_tables, &mrt->node, hash);
> }
>
> +static struct route_table_state *
> +route_table_state_find(const struct route_exchange_state *state,
> + uint32_t table_id)
> +{
> + struct route_table_state *rt;
> + HMAP_FOR_EACH_WITH_HASH (rt, node,
> maintained_route_table_hash(table_id),
> + &state->tables) {
> + if (rt->table_id == table_id) {
> + return rt;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +static struct route_table_state *
> +route_table_state_get(struct route_exchange_state *state, uint32_t
> table_id)
> +{
> + struct route_table_state *rt = route_table_state_find(state,
> table_id);
> + if (rt) {
> + return rt;
> + }
> +
> + rt = xmalloc(sizeof *rt);
> + rt->table_id = table_id;
> + hmap_init(&rt->learned_routes);
> + hmap_insert(&state->tables, &rt->node,
> + maintained_route_table_hash(table_id));
> +
> + return rt;
> +}
> +
> +static void
> +route_table_state_destroy(struct route_exchange_state *state,
> + struct route_table_state *rt)
> +{
> + hmap_remove(&state->tables, &rt->node);
> + re_nl_cached_routes_clear(&rt->learned_routes);
> + hmap_destroy(&rt->learned_routes);
> + free(rt);
> +}
> +
> +struct route_exchange_state *
> +route_exchange_state_create(void)
> +{
> + struct route_exchange_state *state = xmalloc(sizeof *state);
> +
> + hmap_init(&state->tables);
> + return state;
> +}
> +
> +void
> +route_exchange_state_destroy(struct route_exchange_state *state)
> +{
> + struct route_table_state *rt;
> + HMAP_FOR_EACH_SAFE (rt, node, &state->tables) {
> + route_table_state_destroy(state, rt);
> + }
> +
> + hmap_destroy(&state->tables);
> + free(state);
> +}
> +
> static struct route_entry *
> route_add_entry(struct hmap *routes,
> const struct sbrec_learned_route *sb_route,
> @@ -353,6 +430,37 @@ advertised_routes_tables(const struct hmapx
> *datapaths,
> }
> }
>
> +static struct advertised_routes_entry *
> +advertised_routes_find(const struct hmap *advertised_routes, uint32_t
> table_id)
> +{
> + struct advertised_routes_entry *arte;
> + HMAP_FOR_EACH_WITH_HASH (arte, node,
> + maintained_route_table_hash(table_id),
> + advertised_routes) {
> + if (arte->table_id == table_id) {
> + return arte;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +/* Maps every routing table OVN distributes routes into to the datapaths
> that
> + * do so. */
> +static void
> +advertised_routes_build(struct hmap *advertised_routes,
> + const struct hmap *announce_routes)
> +{
> + const struct advertise_datapath_entry *ad;
> + HMAP_FOR_EACH (ad, node, announce_routes) {
> + uint32_t table_id = route_get_table_id(ad->db);
> +
> + if (TABLE_ID_VALID(table_id)) {
> + advertised_routes_add(advertised_routes, ad, table_id);
> + }
> + }
> +}
> +
> static void
> advertised_routes_destroy(struct hmap *advertised_routes)
> {
> @@ -364,12 +472,11 @@ advertised_routes_destroy(struct hmap
> *advertised_routes)
> hmap_destroy(advertised_routes);
> }
>
> -/* Turns 'learned_routes', the routes of a kernel routing table OVN may
> learn
> - * from, into Learned_Route rows of every datapath in 'datapaths' ('struct
> - * advertise_datapath_entry *'). */
> +/* Turns the routes OVN learns from the table 'rt' into Learned_Route
> rows of
> + * every datapath in 'datapaths' ('struct advertise_datapath_entry *'). */
> static void
> -resolve_and_sync_learned_routes(
> - const struct hmap *learned_routes, const struct hmapx *datapaths,
> +route_table_resolve_and_sync(
> + struct route_table_state *rt, const struct hmapx *datapaths,
> const struct route_exchange_ctx_in *r_ctx_in,
> struct route_exchange_ctx_out *r_ctx_out)
> {
> @@ -377,7 +484,7 @@ resolve_and_sync_learned_routes(
> VECTOR_EMPTY_INITIALIZER(struct re_nl_received_route_node);
>
> const struct re_nl_cached_route *cr;
> - HMAP_FOR_EACH (cr, node, learned_routes) {
> + HMAP_FOR_EACH (cr, node, &rt->learned_routes) {
> re_nl_resolve_route(cr->msg, &received_routes);
> }
>
> @@ -397,8 +504,87 @@ resolve_and_sync_learned_routes(
> vector_destroy(&received_routes);
> }
>
> +/* Redoes the Learned_Route rows of every table in 'changed_tables'
> ('struct
> + * route_table_state'). */
> +static enum route_exchange_handled
> +resync_changed_tables(const struct hmapx *changed_tables,
> + const struct route_exchange_ctx_in *r_ctx_in,
> + struct route_exchange_ctx_out *r_ctx_out)
> +{
> + if (hmapx_is_empty(changed_tables)) {
> + return ROUTE_EXCHANGE_UNCHANGED;
> + }
> +
> + if (!r_ctx_in->ovnsb_idl_txn) {
> + /* Without a transaction we could only record that there are
> changes
> + * left to write, which a full run does better. */
> + return ROUTE_EXCHANGE_UNHANDLED;
> + }
> +
> + struct hmap advertised_routes = HMAP_INITIALIZER(&advertised_routes);
> + enum route_exchange_handled handled = ROUTE_EXCHANGE_UPDATED;
> +
> + advertised_routes_build(&advertised_routes,
> r_ctx_in->announce_routes);
> +
> + struct hmapx_node *hn;
> + HMAPX_FOR_EACH (hn, changed_tables) {
> + struct route_table_state *rt = hn->data;
> + const struct advertised_routes_entry *arte =
> + advertised_routes_find(&advertised_routes, rt->table_id);
> +
> + if (!arte) {
> + /* The datapaths distributing routes into the table are not
> the
> + * ones it was synced for. */
> + handled = ROUTE_EXCHANGE_UNHANDLED;
> + break;
> + }
> +
> + route_table_resolve_and_sync(rt, &arte->datapaths, r_ctx_in,
> + r_ctx_out);
> + }
> +
> + advertised_routes_destroy(&advertised_routes);
> +
> + return handled;
> +}
> +
> +enum route_exchange_handled
> +route_exchange_handle_route_changes(
> + struct route_exchange_state *state,
> + const struct route_exchange_ctx_in *r_ctx_in,
> + struct route_exchange_ctx_out *r_ctx_out,
> + const struct vector *changed_routes)
> +{
> + struct hmapx changed_tables = HMAPX_INITIALIZER(&changed_tables);
> + enum route_exchange_handled handled;
> +
> + const struct ovn_route_msg *msg;
> + VECTOR_FOR_EACH (changed_routes, msg) {
> + struct route_table_state *rt =
> + route_table_state_find(state, msg->table_id);
> + if (!rt) {
> + /* A table we have not read, so we do not know the rest of it
> + * either. */
> + handled = ROUTE_EXCHANGE_UNHANDLED;
> + goto out;
> + }
> +
> + if (re_nl_cached_routes_apply(&rt->learned_routes, msg)) {
> + hmapx_add(&changed_tables, rt);
> + }
> + }
> +
> + handled = resync_changed_tables(&changed_tables, r_ctx_in, r_ctx_out);
> +
> +out:
> + hmapx_destroy(&changed_tables);
> +
> + return handled;
> +}
> +
> void
> -route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in,
> +route_exchange_run(struct route_exchange_state *state,
> + const struct route_exchange_ctx_in *r_ctx_in,
> struct route_exchange_ctx_out *r_ctx_out)
> {
> struct hmap advertised_routes = HMAP_INITIALIZER(&advertised_routes);
> @@ -444,26 +630,33 @@ route_exchange_run(const struct
> route_exchange_ctx_in *r_ctx_in,
> advertised_routes_add(&advertised_routes, ad, table_id);
> }
>
> - struct hmap learned_routes = HMAP_INITIALIZER(&learned_routes);
> struct advertised_routes_entry *arte;
> HMAP_FOR_EACH (arte, node, &advertised_routes) {
> maintained_route_table_add(arte->table_id);
>
> + struct route_table_state *rt = route_table_state_get(state,
> +
> arte->table_id);
> struct vector route_tables =
> VECTOR_EMPTY_INITIALIZER(const struct hmap *);
> advertised_routes_tables(&arte->datapaths, &route_tables);
>
> error = re_nl_sync_routes(arte->table_id, &route_tables,
> - &learned_routes);
> + &rt->learned_routes);
> SET_ROUTE_EXCHANGE_NL_STATUS(error);
> vector_destroy(&route_tables);
>
> - resolve_and_sync_learned_routes(&learned_routes, &arte->datapaths,
> - r_ctx_in, r_ctx_out);
> + route_table_resolve_and_sync(rt, &arte->datapaths, r_ctx_in,
> + r_ctx_out);
> vector_push(r_ctx_out->route_table_watches, &arte->table_id);
> }
> - re_nl_cached_routes_clear(&learned_routes);
> - hmap_destroy(&learned_routes);
> +
> + /* Forget the tables we do not sync anymore. */
> + struct route_table_state *rt;
> + HMAP_FOR_EACH_SAFE (rt, node, &state->tables) {
> + if (!advertised_routes_find(&advertised_routes, rt->table_id)) {
> + route_table_state_destroy(state, rt);
> + }
> + }
>
> /* Remove routes in tables previously maintained by us. */
> struct maintained_route_table_entry *mrt;
> diff --git a/controller/route-exchange.h b/controller/route-exchange.h
> index a1ef4a359dfc..f52ea505585c 100644
> --- a/controller/route-exchange.h
> +++ b/controller/route-exchange.h
> @@ -31,12 +31,38 @@ struct route_exchange_ctx_in {
> };
>
> struct route_exchange_ctx_out {
> + /* Populated by route_exchange_run() only, NULL otherwise. */
>
Nit: This comment seems unnecessary.
> struct vector *route_table_watches;
> bool sb_changes_pending;
> };
>
> -void route_exchange_run(const struct route_exchange_ctx_in *,
> +/* What route_exchange knows about the kernel routing tables it syncs,
> kept
> + * between runs so that a change to one of them can be applied without
> reading
> + * them all again. */
> +struct route_exchange_state;
> +
> +struct route_exchange_state *route_exchange_state_create(void);
> +void route_exchange_state_destroy(struct route_exchange_state *);
> +
> +void route_exchange_run(struct route_exchange_state *,
> + const struct route_exchange_ctx_in *,
> struct route_exchange_ctx_out *);
> +
> +enum route_exchange_handled {
> + /* The change cannot be applied to what the last route_exchange_run()
> left
> + * behind, so it has to run again. */
> + ROUTE_EXCHANGE_UNHANDLED,
> + /* The change does not affect the routes OVN learned. */
> + ROUTE_EXCHANGE_UNCHANGED,
> + ROUTE_EXCHANGE_UPDATED,
> +};
> +
> +/* Updates the routes OVN learned after the kernel reported the route
> changes
> + * in 'changed_routes' ('struct ovn_route_msg *'). */
> +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);
> +
>
All of those updated and new function should have proper implementation in
route-exchange-stub.c.
> 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 44bf5689e853..b0c29d1b5535 100644
> --- a/tests/ovn-inc-proc-graph-dump.at
> +++ b/tests/ovn-inc-proc-graph-dump.at
> @@ -471,7 +471,7 @@ digraph "Incremental-Processing-Engine" {
> route -> route_exchange [[label=""]];
> SB_learned_route -> route_exchange [[label="engine_noop_handler"]];
> SB_port_binding -> route_exchange [[label="engine_noop_handler"]];
> - route_table_notify -> route_exchange [[label=""]];
> + route_table_notify -> route_exchange
> [[label="route_exchange_route_table_handler"]];
> route_exchange_status -> route_exchange [[label=""]];
> sb_ro -> route_exchange [[label="route_exchange_sb_ro_handler"]];
> garp_rarp [[style=filled, shape=box, fillcolor=white,
> label="garp_rarp"]];
> diff --git a/tests/system-ovn-netlink.at b/tests/system-ovn-netlink.at
> index d534814a2ff5..69c25d6e9544 100644
> --- a/tests/system-ovn-netlink.at
> +++ b/tests/system-ovn-netlink.at
> @@ -617,6 +617,80 @@ AT_CHECK_UNQUOTED([ovstest test-ovn-netlink
> route-table-notify \
>
> AT_CLEANUP
>
> +AT_SETUP([sync netlink routes - table update])
> +AT_KEYWORDS([netlink-routes])
> +CHECK_VRF()
> +
> +table_id=100
> +
> +check ip link add vrf-$table_id type vrf table $table_id
> +on_exit 'ip link del 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 addr add 20.0.0.10/24 dev lo-test
> +check ip link set up lo-test
> +
> +dnl A route installed by a dynamic routing protocol is there from the
> start.
> +check ip route add 10.10.10.0/24 via 20.0.0.1 vrf vrf-$table_id proto
> zebra
> +
> +dnl A route added to the table joins the ones we know of.
> +AT_CHECK_UNQUOTED([ovstest test-ovn-netlink route-table-update $table_id \
> + "ip route add 10.10.11.0/24 via 20.0.0.2 vrf vrf-$table_id proto
> zebra" \
> + | sort], [0], [dnl
> +Applied route add
> +Route table_id=$table_id dst=10.10.10.0 plen=24 proto=11 priority=0
> nexthop=20.0.0.1,dev=lo-test
> +Route table_id=$table_id dst=10.10.11.0 plen=24 proto=11 priority=0
> nexthop=20.0.0.2,dev=lo-test
> +])
> +
> +dnl A route removed from the table leaves them.
> +AT_CHECK_UNQUOTED([ovstest test-ovn-netlink route-table-update $table_id \
> + "ip route del 10.10.11.0/24 vrf vrf-$table_id" | sort], [0], [dnl
> +Applied route delete
> +Route table_id=$table_id dst=10.10.10.0 plen=24 proto=11 priority=0
> nexthop=20.0.0.1,dev=lo-test
> +])
> +
> +dnl Sending a route somewhere else replaces the one we know of.
> +AT_CHECK_UNQUOTED([ovstest test-ovn-netlink route-table-update $table_id \
> + "ip route replace 10.10.10.0/24 via 20.0.0.3 vrf vrf-$table_id proto
> zebra" \
> + | sort], [0], [dnl
> +Applied route add
> +Route table_id=$table_id dst=10.10.10.0 plen=24 proto=11 priority=0
> nexthop=20.0.0.3,dev=lo-test
> +])
> +
> +dnl A route OVN does not learn, here one configured by a user, is left
> out.
> +AT_CHECK_UNQUOTED([ovstest test-ovn-netlink route-table-update $table_id \
> + "ip route add 10.10.12.0/24 via 20.0.0.4 vrf vrf-$table_id proto
> static" \
> + | sort], [0], [dnl
> +Ignored route add
> +Route table_id=$table_id dst=10.10.10.0 plen=24 proto=11 priority=0
> nexthop=20.0.0.3,dev=lo-test
> +])
> +
> +dnl The kernel keeps several routes to one prefix that differ only by
> their
> +dnl metric, so the metric is part of what tells them apart.
> +check ip route add 10.10.13.0/24 via 20.0.0.5 metric 100 vrf
> vrf-$table_id \
> + proto zebra
> +AT_CHECK_UNQUOTED([ovstest test-ovn-netlink route-table-update $table_id \
> + "ip route add 10.10.13.0/24 via 20.0.0.6 metric 200 vrf
> vrf-$table_id proto zebra" \
> + | sort], [0], [dnl
> +Applied route add
> +Route table_id=$table_id dst=10.10.10.0 plen=24 proto=11 priority=0
> nexthop=20.0.0.3,dev=lo-test
> +Route table_id=$table_id dst=10.10.13.0 plen=24 proto=11 priority=100
> nexthop=20.0.0.5,dev=lo-test
> +Route table_id=$table_id dst=10.10.13.0 plen=24 proto=11 priority=200
> nexthop=20.0.0.6,dev=lo-test
> +])
> +
> +dnl Removing one of them keeps the other.
> +AT_CHECK_UNQUOTED([ovstest test-ovn-netlink route-table-update $table_id \
> + "ip route del 10.10.13.0/24 metric 100 vrf vrf-$table_id" | sort],
> [0], [dnl
> +Applied route delete
> +Route table_id=$table_id dst=10.10.10.0 plen=24 proto=11 priority=0
> nexthop=20.0.0.3,dev=lo-test
> +Route table_id=$table_id dst=10.10.13.0 plen=24 proto=11 priority=200
> nexthop=20.0.0.6,dev=lo-test
> +])
> +
> +AT_CLEANUP
> +
> AT_SETUP([sync netlink nexthops - learn nexthops])
> AT_KEYWORDS([netlink-nexthops])
>
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 799779276de6..766d838190c9 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -21045,6 +21045,45 @@ ip_prefix : "10.10.3.1"
> ip_prefix : "10.10.4.1"
> ])
>
> +AS_BOX([$(date +%H:%M:%S.%03N) Learned routes follow the notifications])
> +
> +# 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 a routing 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 checks 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)"])
> +
> +# A route appearing in the table, or leaving it, is picked up from the
> +# notification about it rather than by reading the table again.
> +re_recompute=$(route_recomputes)
> +check ip route add 10.10.6.1 via 20.0.0.25 vrf vrf-$vni proto zebra
> +wait_row_count Learned_Route 1 ip_prefix=10.10.6.1 nexthop=20.0.0.25
> +AT_CHECK_UNQUOTED([route_recomputes], [0], [$re_recompute
> +])
> +
> +re_recompute=$(route_recomputes)
> +check ip route del 10.10.6.1 vrf vrf-$vni
> +wait_row_count Learned_Route 0 ip_prefix=10.10.6.1
> +AT_CHECK_UNQUOTED([route_recomputes], [0], [$re_recompute
> +])
> +
> +# A route OVN does not learn from does not even reach the tables it keeps.
> +re_recompute=$(route_recomputes)
> +check ip route add 10.10.7.1 via 20.0.0.25 vrf vrf-$vni
> +check ovn-nbctl --wait=hv sync
> +check_row_count Learned_Route 0 ip_prefix=10.10.7.1
> +AT_CHECK_UNQUOTED([route_recomputes], [0], [$re_recompute
> +])
> +check ip route del 10.10.7.1 vrf vrf-$vni
> +
> # 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 3c2c14dc5066..f75a6f295cab 100644
> --- a/tests/test-ovn-netlink.c
> +++ b/tests/test-ovn-netlink.c
> @@ -369,6 +369,80 @@ test_nexthop_table_notify(struct ovs_cmdl_context
> *ctx)
> ovn_netlink_notifiers_destroy();
> }
>
> +/* Reports the routes of 'table_id' OVN learns from after applying the
> changes
> + * caused by running 'shell_command' to them. Unlike "route-sync", which
> reads
> + * the whole table, this goes through the incremental update path. */
> +static void
> +test_route_table_update(struct ovs_cmdl_context *ctx)
> +{
> + static const enum ovn_netlink_notifier_type types[] = {
> + OVN_NL_NOTIFIER_ROUTE_V4, OVN_NL_NOTIFIER_ROUTE_V6,
> + };
> + unsigned int shift = 1;
> +
> + unsigned int table_id;
> + if (!test_read_uint_value(ctx, shift++, "table id", &table_id)) {
> + return;
> + }
> +
> + const char *cmd = test_read_value(ctx, shift++, "shell_command");
> + if (!cmd) {
> + return;
> + }
> +
> + struct hmap routes_to_advertise =
> HMAP_INITIALIZER(&routes_to_advertise);
> + struct hmap learned_routes = HMAP_INITIALIZER(&learned_routes);
> + struct vector route_tables =
> + VECTOR_EMPTY_INITIALIZER(const struct hmap *);
> + struct ds ds = DS_EMPTY_INITIALIZER;
> +
> + const struct hmap *routes = &routes_to_advertise;
> + vector_push(&route_tables, &routes);
> +
> + for (size_t i = 0; i < ARRAY_SIZE(types); i++) {
> + ovn_netlink_update_notifier(types[i], true);
> + }
> + ovs_assert(re_nl_sync_routes(table_id, &route_tables,
> + &learned_routes) == 0);
> + vector_destroy(&route_tables);
> + /* The routes are up to date, anything reported so far is among them.
> */
> + for (size_t i = 0; i < ARRAY_SIZE(types); i++) {
> + ovn_netlink_notifier_flush(types[i]);
> + }
> +
> + run_command_under_notifier(cmd);
> +
> + for (size_t i = 0; i < ARRAY_SIZE(types); i++) {
> + struct vector *msgs = ovn_netlink_get_msgs(types[i]);
> + struct ovn_route_msg *msg;
> +
> + VECTOR_FOR_EACH (msgs, msg) {
> + if (msg->table_id != table_id) {
> + continue;
> + }
> +
> + printf("%s route %s\n",
> + re_nl_cached_routes_apply(&learned_routes, msg)
> + ? "Applied" : "Ignored",
> + msg->nlmsg_type == RTM_NEWROUTE ? "add" : "delete");
> + }
> + ovn_netlink_notifier_flush(types[i]);
> + }
> +
> + const struct re_nl_cached_route *cr;
> + HMAP_FOR_EACH (cr, node, &learned_routes) {
> + ds_clear(&ds);
> + ovn_route_msg_format(&ds, cr->msg);
> + printf("Route %s\n", ds_cstr(&ds));
> + }
> +
> + ds_destroy(&ds);
> + re_nl_cached_routes_clear(&learned_routes);
> + hmap_destroy(&learned_routes);
> + hmap_destroy(&routes_to_advertise);
> + ovn_netlink_notifiers_destroy();
> +}
> +
> /* Dumps the nexthop table after applying the changes caused by running
> * 'shell_command' to it. Unlike "nexthop-sync", which builds the table
> from
> * scratch, this goes through the incremental update path. */
> @@ -421,6 +495,8 @@ test_ovn_netlink(int argc, char *argv[])
> {"route-sync", NULL, 1, INT_MAX, test_route_sync, OVS_RO},
> {"route-table-notify", NULL, 1, 1,
> test_route_table_notify, OVS_RO},
> + {"route-table-update", NULL, 2, 2,
> + test_route_table_update, OVS_RO},
> {"nexthop-sync", NULL, 0, 0, test_nexthop_sync, OVS_RO},
> {"nexthop-table-notify", NULL, 1, 1,
> test_nexthop_table_notify, OVS_RO},
> --
> 2.38.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Other than that it looks good.
Acked-by: Ales Musil <[email protected]>
Regards,
Ales
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
