On restart, ovn-controller monitored the whole cluster's Advertised_Route table at startup, and only ran its kernel route sync once all of it had been read. The whole-cluster pull added measurable startup latency in large deployments, and the guard was coarser than it needed to be: the controller only ever deletes routes that it installed itself, and it only needs the advertised routes of the local route-exchange datapaths to know which ones those are.
Monitor no Advertised_Route rows at startup. Once local datapaths are known the monitor condition is already scoped to them, so the dump the controller waits for is small. Defer the route-exchange kernel sync until the controller can be confident that dump is complete: a local route-exchange datapath exists, the current Advertised_Route condition covers those datapaths, and the condition seqno shows the dump was delivered. The kernel routes persist across the deferral, so none are lost. The datapath set changing or a southbound reconnect re-arms the gate. The condition covering the local datapaths is what keeps this safe. Waiting only on the condition seqno could arm the sync against the initial empty condition, and the first sync would then delete the routes installed before the restart. The dynamic-routing system test now checks that the deferred sync is enabled after a restart and that the installed OVN routes survive a --restart of the controller. Reported-at: https://redhat.atlassian.net/browse/FDP-3992 Assisted-by: Qwen3.8-27B-FP8, opencode Signed-off-by: Jacob Tanenbaum <[email protected]> --- NEWS | 7 +++ TODO.rst | 5 -- controller/ovn-controller.c | 104 ++++++++++++++++++++++++++++++++++-- tests/system-ovn.at | 11 +++- 4 files changed, 116 insertions(+), 11 deletions(-) diff --git a/NEWS b/NEWS index f1c56dd14..fec24583d 100644 --- a/NEWS +++ b/NEWS @@ -16,6 +16,13 @@ Post v26.09.0 now written to the SB MAC_Binding table and consumed at the same priority as dynamic entries, making the preference option obsolete. + * ovn-controller no longer monitors the whole cluster's + Advertised_Route table at startup; it defers its kernel route + updates until the initial dump of the advertised routes of the + local route-exchange datapaths is complete. This avoids + deleting routes installed before a restart and reduces + ovn-controller restart time for deployments with many + advertised routes. OVN v26.09.0 - xxx xx xxxx -------------------------- diff --git a/TODO.rst b/TODO.rst index 023eb27f6..dc55c8895 100644 --- a/TODO.rst +++ b/TODO.rst @@ -145,11 +145,6 @@ OVN To-do List * Add incremental processing of en_dynamic_routes for stateful configuration changes. - * The ovn-controller currently loads all Advertised_Route entries on startup. - This is to prevent deleting our routes on restart. If we defer updating - routes until we are sure to have loaded all necessary Advertised_Routes - this could be changed. - * Improve handling of the Learned_Route table in ovn-controller conditional monitoring; once a new local datapath is added we need to wait for monitoring conditions to update before we actually try to learn routes. diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index c601f89dc..32acaabe9 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -176,6 +176,16 @@ struct controller_engine_ctx { struct if_status_mgr *if_mgr; const unsigned int *ovnsb_expected_cond_seqno; const bool *sb_monitor_all; + /* True once every table that the current monitor conditions request has + * been fully received (the SB condition seqno has caught up). The + * route-exchange node uses this to defer its destructive kernel sync + * until the data it relies on is complete. */ + bool sb_all_data_loaded; + /* True once the Advertised_Route monitor condition covers the local + * route-exchange datapaths (or, in monitor-all mode, everything). It is + * false for the initial empty condition that is used before any local + * datapath is known. */ + bool sb_ar_condition_scoped; }; /* Pending packet to be injected into connected OVS. */ @@ -360,11 +370,13 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, sbrec_port_binding_add_clause_type(&pb, OVSDB_F_EQ, "l2gateway"); sbrec_port_binding_add_clause_type(&pb, OVSDB_F_EQ, "l3gateway"); - /* Monitor all advertised routes during startup. - * Otherwise, once we claim a port on startup we do not yet know the - * routes to advertise and might wrongly delete already installed - * ones. */ - ovsdb_idl_condition_add_clause_true(&ar); + /* Do not monitor any Advertised_Route rows during startup. The + * route-exchange node defers its destructive kernel sync until it has + * seen the initial dump of the advertised routes of the local + * route-exchange datapaths, so there is no risk of deleting routes + * we installed previously. Once local datapaths are known the + * condition is scoped to them (below), which avoids pulling the whole + * cluster's advertised routes on startup. */ } if (local_ifaces) { const char *name; @@ -773,6 +785,11 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl, if (sb_cond_seqno) { *sb_cond_seqno = next_cond_seqno; } + /* In monitor-all mode the Advertised_Route condition matches + * everything, so it already covers the local datapaths. */ + if (ctx) { + ctx->sb_ar_condition_scoped = true; + } } if (monitor_all_p) { *monitor_all_p = monitor_all; @@ -5738,6 +5755,12 @@ struct ed_type_route_exchange { bool sb_changes_pending; /* What the last run learned from the kernel routing tables. */ struct route_exchange_state *state; + /* True once the initial Advertised_Route dump for the local + * route-exchange datapaths is complete. Until then the destructive + * kernel sync is deferred so that we cannot delete routes that we + * installed previously but that are not yet present in our (partial) + * view of the SB database. */ + bool initial_routes_loaded; }; static void @@ -5788,6 +5811,42 @@ en_route_exchange_run(struct engine_node *node, void *data) return EN_STALE; } + struct ed_type_route *route_data = + engine_get_input_data("route", node); + struct controller_engine_ctx *ctrl_ctx = + engine_get_context()->client_ctx; + + /* The set of local route-exchange datapaths selects the Advertised_Route + * rows that are relevant to this chassis. Until the initial dump of + * exactly those rows is complete, we must not run the destructive kernel + * sync: it deletes every OVN route it does not see, so a partial view + * would delete routes we installed before a restart that have not + * reached our view yet. */ + bool have_local_re_datapaths = + !hmap_is_empty(&route_data->announce_routes); + bool ready = ctrl_ctx->sb_all_data_loaded + && ctrl_ctx->sb_ar_condition_scoped + && have_local_re_datapaths; + + if (re->initial_routes_loaded && !ready) { + /* The set of local route-exchange datapaths changed or the SB + * connection reset; defer the destructive sync again until the new + * scoped dump is complete. */ + re->initial_routes_loaded = false; + VLOG_INFO("Route-exchange datapath set changed; deferring route " + "sync until the Advertised_Route dump completes."); + return EN_UNCHANGED; + } + if (!re->initial_routes_loaded) { + if (!ready) { + return EN_UNCHANGED; /* still loading; kernel untouched */ + } + re->initial_routes_loaded = true; + VLOG_INFO("Advertised_Route dump complete for local route-exchange " + "datapaths; enabling route sync."); + /* Fall through: run once now to reconcile the kernel. */ + } + vector_clear(&rt_notify->watches); struct route_exchange_ctx_in r_ctx_in; @@ -5810,6 +5869,13 @@ route_exchange_route_table_handler(struct engine_node *node, void *data) struct ed_type_route_table_notify *rt_notify = engine_get_input_data("route_table_notify", node); + /* While the initial dump is still pending the full run has not happened + * yet, so no kernel route watches are registered and nothing was lost. + * The first armed run reconciles the kernel from scratch. */ + if (!re->initial_routes_loaded) { + return EN_HANDLED_UNCHANGED; + } + /* 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) { @@ -7393,6 +7459,10 @@ inc_proc_ovn_controller_init( 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); + /* Re-evaluate the initial-dump readiness gate whenever the SB monitor + * conditions change (including the final ack of the scoped + * Advertised_Route dump). */ + engine_add_input(&en_route_exchange, &en_sb_cond_seqno, NULL); engine_add_input(&en_addr_sets, &en_sb_address_set, addr_sets_sb_address_set_handler); @@ -8170,6 +8240,8 @@ main(int argc, char *argv[]) .if_mgr = if_status_mgr_create(), .ovnsb_expected_cond_seqno = &ovnsb_expected_cond_seqno, .sb_monitor_all = &sb_monitor_all, + .sb_all_data_loaded = false, + .sb_ar_condition_scoped = false, }; struct if_status_mgr *if_mgr = ctrl_engine_ctx.if_mgr; @@ -8239,6 +8311,10 @@ main(int argc, char *argv[]) if (!new_ovnsb_cond_seqno) { VLOG_INFO("OVNSB IDL reconnected, force recompute."); engine_set_force_recompute(); + /* The monitor conditions are re-applied on reconnect, so the + * previously scoped Advertised_Route condition no longer + * holds. */ + ctrl_engine_ctx.sb_ar_condition_scoped = false; } ovnsb_cond_seqno = new_ovnsb_cond_seqno; } @@ -8253,6 +8329,15 @@ main(int argc, char *argv[]) daemon_started_recently_ignore(); } + /* Whether the rows selected by the current monitor conditions are all + * present in the IDL. This is recomputed every loop iteration so + * that engine nodes can rely on it being up to date for the + * conditions that were applied so far. */ + ctrl_engine_ctx.sb_all_data_loaded = + ovnsb_cond_seqno == ovnsb_expected_cond_seqno + && ovnsb_expected_cond_seqno != UINT_MAX + && ovnsb_cond_seqno != 0; + struct engine_context eng_ctx = { .ovs_idl_txn = ovs_idl_txn, .ovnsb_idl_txn = ovnsb_idl_txn, @@ -8575,6 +8660,15 @@ main(int argc, char *argv[]) &runtime_data->lbinding_data.bindings, &runtime_data->local_datapaths, sb_monitor_all); + /* The Advertised_Route condition now covers the + * local datapaths (or everything, in + * monitor-all mode). If there are no local + * datapaths yet, there is nothing to cover and + * the empty condition is sufficient. */ + ctrl_engine_ctx.sb_ar_condition_scoped = + sb_monitor_all || + !hmap_is_empty( + &runtime_data->local_datapaths); bool condition_changed = ovnsb_cond_seqno != ovnsb_expected_cond_seqno; if (had_all_data && condition_changed) { diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 5b6ba3731..2c90088ea 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -15739,6 +15739,12 @@ blackhole 198.51.100.0/24 proto ovn metric 1000 233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 30 onlink 233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 40 onlink]) +# Route exchange defers its kernel sync until the initial Advertised_Route +# dump for the local route-exchange datapaths is complete. The routes above +# can only be installed once that happens, so verify the deferred sync was +# actually enabled. +OVS_WAIT_UNTIL([test "$(grep -c 'Advertised_Route dump complete for local route-exchange datapaths' ovn-controller.log)" -ge 1]) + # Changing the vrf name will switch to the new one. # The old vrf will be removed. check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \ @@ -15757,7 +15763,10 @@ blackhole 198.51.100.0/24 proto ovn metric 1000 233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 30 onlink 233.253.0.0/24 via 192.168.20.20 dev hv1-mll proto zebra metric 40 onlink]) -# Stopping with --restart will not touch the routes. +# Stopping with --restart will not touch the routes. While the restarted +# controller loads its Advertised_Routes the kernel sync is deferred, so a +# controller that synced from a partial view would have deleted the OVN routes +# below; verify they all survive the restart. OVN_CONTROLLER_EXIT([],[--restart]) OVN_ROUTE_EQUAL([ovnvrf1338], [dnl blackhole 192.0.2.1 proto ovn metric 100 -- 2.55.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
