Add the incremental change handler for the en_tr node and scope its incremental processing to the affected transit-router datapaths, so an IC-NB transit-router (or its port) change reconciles only that router instead of forcing a full recompute.
Add a tests/ovn-ic.at test exercising the transit switch and router incremental handlers. Assisted-by: Claude Opus 4.8, Claude Code Co-authored-by: Tiago Matos <[email protected]> Signed-off-by: Tiago Matos <[email protected]> Signed-off-by: Paulo Guilherme Silva <[email protected]> --- ic/en-tr.c | 72 ++++++++++++++++++++++++++++ ic/en-tr.h | 5 ++ ic/inc-proc-ic.c | 14 ++++-- tests/ovn-ic.at | 121 +++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 205 insertions(+), 7 deletions(-) diff --git a/ic/en-tr.c b/ic/en-tr.c index 1b5f1db6d..78b84eaf6 100644 --- a/ic/en-tr.c +++ b/ic/en-tr.c @@ -120,6 +120,78 @@ en_tr_run(struct engine_node *node, void *data OVS_UNUSED) return EN_UPDATED; } +/* Only transit-router mirror logical routers (options:interconn-tr) affect + * en_tr. A change to such a logical router is left to a full recompute + * (returns EN_UNHANDLED); any other logical router is irrelevant to en_tr, so + * its change is handled as a no-op, avoiding a recompute on unrelated NB + * Logical_Router updates. */ +enum engine_input_handler_result +en_tr_nb_logical_router_handler(struct engine_node *node, + void *data OVS_UNUSED) +{ + const struct ed_type_az *az = engine_get_input_data("az", node); + + if (!az->runned_az) { + return EN_HANDLED_UNCHANGED; + } + + const struct nbrec_logical_router_table *tbl = + EN_OVSDB_GET(engine_get_input("NB_logical_router", node)); + const struct nbrec_logical_router *lr; + NBREC_LOGICAL_ROUTER_TABLE_FOR_EACH_TRACKED (lr, tbl) { + if (smap_get(&lr->options, "interconn-tr")) { + return EN_UNHANDLED; + } + } + + return EN_HANDLED_UNCHANGED; +} + +/* IC-SB Datapath_Binding: tr_run() reads only transit-router datapath bindings + * (it mirrors their tunnel_key into the NB logical router as + * requested-tnl-key). A new/deleted/modified transit-router binding - created + * by en_tunnel_key - forces a recompute so tr_run() publishes the committed + * key. Transit-switch datapath bindings - the bulk of the churn here - do not + * affect en_tr and are a no-op. A change to the type column cannot be + * classified (the old type is gone), so recompute to be safe. */ +enum engine_input_handler_result +en_tr_icsb_datapath_binding_handler(struct engine_node *node, + void *data OVS_UNUSED) +{ + const struct ed_type_az *az = engine_get_input_data("az", node); + + if (!az->runned_az) { + return EN_HANDLED_UNCHANGED; + } + + const struct icsbrec_datapath_binding_table *tbl = + EN_OVSDB_GET(engine_get_input("ICSB_datapath_binding", node)); + const struct icsbrec_datapath_binding *isb_dp; + ICSBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (isb_dp, tbl) { + if (icsbrec_datapath_binding_is_new(isb_dp) || + icsbrec_datapath_binding_is_deleted(isb_dp)) { + /* A new/deleted transit-router binding is the trigger en_tr needs; + * a transit-switch binding (the common churn) is a no-op. Classify + * by the row's own type, which is available on tracked deleted + * rows too - unlike track_is_updated(), which reports every + * column as updated on insert. */ + if (ic_dp_get_type(isb_dp) == IC_ROUTER) { + return EN_UNHANDLED; + } + } else if (ic_dp_get_type(isb_dp) == IC_ROUTER || + ovsdb_idl_track_is_updated( + &isb_dp->header_, + &icsbrec_datapath_binding_col_type)) { + /* A modified transit-router binding (e.g. tunnel_key/nb_ic_uuid), + * or a type change that can no longer be classified, forces a + * recompute. */ + return EN_UNHANDLED; + } + } + + return EN_HANDLED_UNCHANGED; +} + void * en_tr_init(struct engine_node *node OVS_UNUSED, struct engine_arg *arg OVS_UNUSED) diff --git a/ic/en-tr.h b/ic/en-tr.h index 90c8c8c4a..1920dcdf6 100644 --- a/ic/en-tr.h +++ b/ic/en-tr.h @@ -7,4 +7,9 @@ enum engine_node_state en_tr_run(struct engine_node *node, void *data); void *en_tr_init(struct engine_node *node, struct engine_arg *arg); void en_tr_cleanup(void *data); +enum engine_input_handler_result +en_tr_nb_logical_router_handler(struct engine_node *node, void *data); +enum engine_input_handler_result +en_tr_icsb_datapath_binding_handler(struct engine_node *node, void *data); + #endif /* EN_IC_TR_H */ diff --git a/ic/inc-proc-ic.c b/ic/inc-proc-ic.c index 89135fc5d..88c866f4b 100644 --- a/ic/inc-proc-ic.c +++ b/ic/inc-proc-ic.c @@ -238,12 +238,18 @@ void inc_proc_ic_init(struct ovsdb_idl_loop *nb, engine_add_input(&en_ts, &en_icsb_encap, NULL); /* en_tr: sync transit routers to their AZ NB Logical_Router mirrors. - * Like en_ts, IC-SB Datapath_Binding creation/keying is owned by - * en_tunnel_key. */ + * + * Like en_ts, en_tr builds its own transit-router IC-SB Datapath_Binding + * map each run and only maintains the NB mirror; IC-SB Datapath_Binding + * creation/keying is owned by en_tunnel_key. A transit-router binding + * change (created by en_tunnel_key) forces a recompute so en_tr publishes + * the committed key to requested-tnl-key. */ engine_add_input(&en_tr, &en_az, NULL); - engine_add_input(&en_tr, &en_icsb_datapath_binding, NULL); + engine_add_input(&en_tr, &en_icsb_datapath_binding, + en_tr_icsb_datapath_binding_handler); engine_add_input(&en_tr, &en_icnb_transit_router, NULL); - engine_add_input(&en_tr, &en_nb_logical_router, NULL); + engine_add_input(&en_tr, &en_nb_logical_router, + en_tr_nb_logical_router_handler); /* en_tunnel_key: the single owner of IC-SB Datapath_Binding creation, * tunnel-key allocation, VXLAN-range refresh and GC, for both transit diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at index 49172951c..b63fe614e 100644 --- a/tests/ovn-ic.at +++ b/tests/ovn-ic.at @@ -818,9 +818,9 @@ AT_CLEANUP ]) OVN_FOR_EACH_NORTHD([ -AT_SETUP([ovn-ic -- incremental processing - dp_enum and gateway]) +AT_SETUP([ovn-ic -- incremental processing - tunnel_key and gateway]) -# Validates the incremental-processing change handlers of the en_dp_enum and +# Validates the incremental-processing change handlers of the en_tunnel_key and # en_gateway engine nodes in two ways: # - the change handler of the node that reads the affected table fires # (its compute stat is non-zero), and @@ -854,7 +854,7 @@ OVS_WAIT_UNTIL([test "$(ovn-ic-sbctl --bare --columns=hostname \ AT_CHECK([test "$(ovn-appctl -t ic/ovn-ic \ inc-engine/show-stats gateway compute)" -gt 0]) -# Exercise the dp_enum handler too: a transit switch creates an IC-SB +# Exercise the tunnel_key handler too: a transit switch creates an IC-SB # datapath binding, and adding a port binding drives further IC-SB updates. check ovn-ic-nbctl --wait=sb ts-add ts1 OVS_WAIT_UNTIL([test "$(ovn-nbctl --bare --columns=name \ @@ -881,6 +881,121 @@ OVN_CLEANUP_IC([az1]) AT_CLEANUP ]) +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ovn-ic -- incremental processing - transit switch and router]) + +# The en_ts/en_tr NB Logical_Switch/Logical_Router change handlers must not +# trigger a full recompute for logical switches/routers that are not transit +# switch/router mirrors. + +ovn_init_ic_db +net_add n1 +ovn_start az1 +as az1 +check ovn-ic-nbctl --wait=sb ts-add ts1 +OVS_WAIT_UNTIL([test "$(ovn-nbctl --bare --columns=name \ + find logical_switch name=ts1)" = ts1]) + +# Quiesce the engine and reset stats so only the change under test counts. +check ovn-ic-nbctl --wait=sb sync +check ovn-appctl -t ic/ovn-ic inc-engine/recompute +check ovn-ic-nbctl --wait=sb sync +check ovn-appctl -t ic/ovn-ic inc-engine/clear-stats + +# Adding a regular logical switch and a regular logical router must be handled +# by the en_ts/en_tr handlers without a full recompute of those nodes. +check ovn-nbctl --wait=sb ls-add sw0 +check ovn-nbctl --wait=sb lr-add lr0 +OVS_WAIT_UNTIL([test "$(ovn-nbctl --bare --columns=name \ + find logical_router name=lr0)" = lr0]) +check ovn-ic-nbctl --wait=sb sync + +AT_CHECK([test "$(ovn-appctl -t ic/ovn-ic \ + inc-engine/show-stats ts recompute)" = 0]) +AT_CHECK([test "$(ovn-appctl -t ic/ovn-ic \ + inc-engine/show-stats tr recompute)" = 0]) + +# The transit switch mirror is still intact after the incremental update. +AT_CHECK([test "$(ovn-nbctl --bare --columns=name \ + find logical_switch name=ts1)" = ts1]) + +OVN_CLEANUP_IC([az1]) +AT_CLEANUP +]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ovn-ic -- incremental processing - transit switch handler scoping]) + +# A transit-switch add/delete in IC-NB must be applied by the en_ts +# en_icnb_transit_switch change handler without a full recompute of en_ts, +# keeping the NB Logical_Switch mirror and the IC-SB Datapath_Binding in sync. + +ovn_init_ic_db +net_add n1 +ovn_start az1 +as az1 +check ovn-ic-nbctl --wait=sb ts-add ts1 +OVS_WAIT_UNTIL([test "$(ovn-nbctl --bare --columns=name \ + find logical_switch name=ts1)" = ts1]) + +# Quiesce the engine and reset stats so only the change under test counts. +check ovn-ic-nbctl --wait=sb sync +check ovn-appctl -t ic/ovn-ic inc-engine/recompute +check ovn-ic-nbctl --wait=sb sync +check ovn-appctl -t ic/ovn-ic inc-engine/clear-stats + +# Adding a second transit switch is handled incrementally (no en_ts recompute) +# and creates its NB mirror and IC-SB datapath binding. +check ovn-ic-nbctl --wait=sb ts-add ts2 +OVS_WAIT_UNTIL([test "$(ovn-nbctl --bare --columns=name \ + find logical_switch name=ts2)" = ts2]) +check ovn-ic-nbctl --wait=sb sync + +AT_CHECK([test "$(ovn-appctl -t ic/ovn-ic \ + inc-engine/show-stats ts recompute)" = 0]) +# The transit-switch IC-SB datapath binding en_tunnel_key creates must not +# churn en_tr: en_tr only consumes transit-router datapath bindings, so its +# en_icsb_datapath_binding handler treats a transit-switch binding as a no-op. +AT_CHECK([test "$(ovn-appctl -t ic/ovn-ic \ + inc-engine/show-stats tr recompute)" = 0]) +# en_tunnel_key owns IC-SB datapath binding creation: a transit-switch add is +# handled by its scoped en_icnb_transit_switch handler, so it must not full +# recompute. +AT_CHECK([test "$(ovn-appctl -t ic/ovn-ic \ + inc-engine/show-stats tunnel_key recompute)" = 0]) +AT_CHECK([test "$(ovn-ic-sbctl --bare --columns=type \ + find datapath_binding transit_switch=ts2)" = transit-switch]) + +# Deleting a transit switch is also handled incrementally: its NB mirror and +# IC-SB datapath binding are garbage-collected without a full recompute. +check ovn-ic-nbctl --wait=sb ts-del ts1 +OVS_WAIT_UNTIL([test -z "$(ovn-nbctl --bare --columns=name \ + find logical_switch name=ts1)"]) +check ovn-ic-nbctl --wait=sb sync + +AT_CHECK([test "$(ovn-appctl -t ic/ovn-ic \ + inc-engine/show-stats ts recompute)" = 0]) +# A transit-switch delete is likewise reconciled incrementally in en_tunnel_key +# (its scoped en_icnb_transit_switch handler GCs the binding), not by a full +# recompute. +AT_CHECK([test "$(ovn-appctl -t ic/ovn-ic \ + inc-engine/show-stats tunnel_key recompute)" = 0]) +AT_CHECK([test -z "$(ovn-ic-sbctl --bare --columns=type \ + find datapath_binding transit_switch=ts1)"]) + +# Deleting the NB mirror of a live transit switch out-of-band recreates it via +# the en_ts NB Logical_Switch handler, still without a full recompute. +check ovn-nbctl --wait=sb ls-del ts2 +check ovn-ic-nbctl --wait=sb sync +OVS_WAIT_UNTIL([test "$(ovn-nbctl --bare --columns=name \ + find logical_switch name=ts2)" = ts2]) +AT_CHECK([test "$(ovn-appctl -t ic/ovn-ic \ + inc-engine/show-stats ts recompute)" = 0]) + +OVN_CLEANUP_IC([az1]) +AT_CLEANUP +]) + OVN_FOR_EACH_NORTHD([ AT_SETUP([ovn-ic -- port sync]) -- 2.34.1 -- _'Esta mensagem é direcionada apenas para os endereços constantes no cabeçalho inicial. Se você não está listado nos endereços constantes no cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão imediatamente anuladas e proibidas'._ * **'Apesar do Magazine Luiza tomar todas as precauções razoáveis para assegurar que nenhum vírus esteja presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por quaisquer perdas ou danos causados por esse e-mail ou por seus anexos'.* _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
