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]> --- .../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. */ 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); + 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
