Hi Lucas, Thanks for the review and for the reproducer.
You're right. The scoped en_tunnel_key handler only sees the new name on the tracked row, so the IC-SB Datapath_Binding of the old name was never visited and leaked. I reproduced it with your test, and the same scenario works on main, so it's a regression introduced by the series. The handler can't scope a rename (the old name is gone by then), so in v5 a Transit_Switch name change falls back to a full recompute. This is the same thing the en_route handler already does for renames. I've also added an "ovn-ic -- transit switch and router rename" test. It renames a TS that has a port and checks that only the new Datapath_Binding and NB Logical_Switch remain and that the old Port_Binding is removed. It also renames a TR and checks that its port binding follows the new name. Regards, Paulo Em sex., 18 de set. de 2026 às 11:43, Lucas Vargas Dias <[email protected]> escreveu: > Hi Paulo, > > Thanks for your patch series. It's a great improvement for ovn-ic. > > I found an issue in this patch. > > When a transit switch is renamed, it'll leak in IC-SB Datapath_Binding. > The scoped handlers key off the tracked row, which only carries the *new* > name, so the > old name is never visited. Just make a test with (it will fail): > check ovn-ic-nbctl set Transit_Switch ts1 name=ts2 > check ovn-ic-nbctl --wait=sb sync > AT_CHECK([test "$(ovn-nbctl --bare --columns=name \ > list logical_switch | grep -c .)" = 1]) > AT_CHECK([test "$(ovn-ic-sbctl --bare --columns=transit_switch \ > list datapath_binding | grep -c .)" = 1]) > > > > Em qua., 2 de set. de 2026 às 18:19, Paulo Guilherme Silva > <[email protected]> escreveu: > >> Replace the en_dp_enum engine node with a new en_tunnel_key node that is >> the >> single owner of IC-SB Datapath_Binding creation, tunnel-key allocation, >> VXLAN-range refresh and garbage collection, for both transit switches and >> transit routers. >> >> en_dp_enum kept a datapath map and tunnel-key allocator that en_ts and >> en_tr >> mutated (cloning it first, and still risking dangling pointers when they >> deleted bindings it referenced). Now en_ts and en_tr build their own >> transit-switch/router datapath maps each run and only maintain the AZ NB >> mirrors; concentrating tunnel-key allocation in en_tunnel_key keeps the >> keys >> globally unique across both datapath types without any node mutating >> another >> node's data. >> >> This addresses the review of the en_dp_enum node. >> >> Signed-off-by: Paulo Guilherme Silva <[email protected]> >> --- >> ic/automake.mk | 4 +- >> ic/en-dp-enum.c | 68 ------- >> ic/en-dp-enum.h | 44 ----- >> ic/en-tr.c | 90 ++++++++- >> ic/en-ts.c | 34 +++- >> ic/en-tunnel-key.c | 449 +++++++++++++++++++++++++++++++++++++++++++++ >> ic/en-tunnel-key.h | 60 ++++++ >> ic/inc-proc-ic.c | 64 ++++--- >> ic/ovn-ic.c | 341 ++++++++++++++-------------------- >> ic/ovn-ic.h | 36 +++- >> 10 files changed, 827 insertions(+), 363 deletions(-) >> delete mode 100644 ic/en-dp-enum.c >> delete mode 100644 ic/en-dp-enum.h >> create mode 100644 ic/en-tunnel-key.c >> create mode 100644 ic/en-tunnel-key.h >> >> diff --git a/ic/automake.mk b/ic/automake.mk >> index b730fc578..82338d5dd 100644 >> --- a/ic/automake.mk >> +++ b/ic/automake.mk >> @@ -6,14 +6,14 @@ ic_ovn_ic_SOURCES = ic/ovn-ic.c \ >> ic/en-ic.h \ >> ic/en-az.c \ >> ic/en-az.h \ >> - ic/en-dp-enum.c \ >> - ic/en-dp-enum.h \ >> ic/en-gateway.c \ >> ic/en-gateway.h \ >> ic/en-ts.c \ >> ic/en-ts.h \ >> ic/en-tr.c \ >> ic/en-tr.h \ >> + ic/en-tunnel-key.c \ >> + ic/en-tunnel-key.h \ >> ic/en-port-binding.c \ >> ic/en-port-binding.h \ >> ic/en-route.c \ >> diff --git a/ic/en-dp-enum.c b/ic/en-dp-enum.c >> deleted file mode 100644 >> index fd447ab52..000000000 >> --- a/ic/en-dp-enum.c >> +++ /dev/null >> @@ -1,68 +0,0 @@ >> -/* >> - * Licensed under the Apache License, Version 2.0 (the "License"); >> - * you may not use this file except in compliance with the License. >> - * You may obtain a copy of the License at: >> - * >> - * http://www.apache.org/licenses/LICENSE-2.0 >> - * >> - * Unless required by applicable law or agreed to in writing, software >> - * distributed under the License is distributed on an "AS IS" BASIS, >> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >> implied. >> - * See the License for the specific language governing permissions and >> - * limitations under the License. >> - */ >> - >> -#include <config.h> >> - >> -#include "en-dp-enum.h" >> -#include "lib/inc-proc-eng.h" >> -#include "lib/ovn-util.h" >> -#include "openvswitch/hmap.h" >> -#include "openvswitch/shash.h" >> -#include "openvswitch/vlog.h" >> -#include "ovn-ic.h" >> - >> -VLOG_DEFINE_THIS_MODULE(en_ic_dp_enum); >> - >> -enum engine_node_state >> -en_dp_enum_run(struct engine_node *node OVS_UNUSED, void *data_) >> -{ >> - const struct engine_context *eng_ctx = engine_get_context(); >> - struct ic_context *ctx = eng_ctx->client_ctx; >> - struct ed_type_dp_enum *data = data_; >> - >> - /* Clear the previous iteration's state and rebuild from the current >> IC-SB >> - * datapath bindings. */ >> - ovn_destroy_tnlids(&data->dp_tnlids); >> - hmap_init(&data->dp_tnlids); >> - shash_clear(&data->isb_ts_dps); >> - shash_clear(&data->isb_tr_dps); >> - >> - enumerate_datapaths(ctx, &data->dp_tnlids, &data->isb_ts_dps, >> - &data->isb_tr_dps); >> - >> - return EN_UPDATED; >> -} >> - >> -void * >> -en_dp_enum_init(struct engine_node *node OVS_UNUSED, >> - struct engine_arg *arg OVS_UNUSED) >> -{ >> - struct ed_type_dp_enum *data = xzalloc(sizeof *data); >> - >> - hmap_init(&data->dp_tnlids); >> - shash_init(&data->isb_ts_dps); >> - shash_init(&data->isb_tr_dps); >> - >> - return data; >> -} >> - >> -void >> -en_dp_enum_cleanup(void *data_) >> -{ >> - struct ed_type_dp_enum *data = data_; >> - >> - ovn_destroy_tnlids(&data->dp_tnlids); >> - shash_destroy(&data->isb_ts_dps); >> - shash_destroy(&data->isb_tr_dps); >> -} >> diff --git a/ic/en-dp-enum.h b/ic/en-dp-enum.h >> deleted file mode 100644 >> index ef925ea68..000000000 >> --- a/ic/en-dp-enum.h >> +++ /dev/null >> @@ -1,44 +0,0 @@ >> -/* >> - * Licensed under the Apache License, Version 2.0 (the "License"); >> - * you may not use this file except in compliance with the License. >> - * You may obtain a copy of the License at: >> - * >> - * http://www.apache.org/licenses/LICENSE-2.0 >> - * >> - * Unless required by applicable law or agreed to in writing, software >> - * distributed under the License is distributed on an "AS IS" BASIS, >> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >> implied. >> - * See the License for the specific language governing permissions and >> - * limitations under the License. >> - */ >> - >> -#ifndef EN_IC_DP_ENUM_H >> -#define EN_IC_DP_ENUM_H 1 >> - >> -#include "lib/inc-proc-eng.h" >> -#include "openvswitch/hmap.h" >> -#include "openvswitch/shash.h" >> - >> -/* Data maintained by the "dp_enum" engine node: the result of >> enumerating the >> - * IC-SB datapath bindings. It is the explicit, engine-visible form of >> the >> - * state that used to be built on the stack by enumerate_datapaths() >> inside >> - * ovn_db_run(). The transit-switch (en_ts) and transit-router (en_tr) >> nodes >> - * consume this data instead of rebuilding it themselves, which also >> keeps the >> - * shared 'dp_tnlids' tunnel-key allocator consistent between them. */ >> -struct ed_type_dp_enum { >> - /* Set of tunnel keys already in use by IC-SB datapath bindings. >> Holds >> - * 'struct tnlid_node *' entries owned by this node. */ >> - struct hmap dp_tnlids; >> - /* Transit-switch datapath bindings, keyed by transit switch name. >> Values >> - * are 'const struct icsbrec_datapath_binding *' owned by the IDL. */ >> - struct shash isb_ts_dps; >> - /* Transit-router datapath bindings, keyed by NB IC UUID string. >> Values >> - * are 'const struct icsbrec_datapath_binding *' owned by the IDL. */ >> - struct shash isb_tr_dps; >> -}; >> - >> -enum engine_node_state en_dp_enum_run(struct engine_node *node, void >> *data); >> -void *en_dp_enum_init(struct engine_node *node, struct engine_arg *arg); >> -void en_dp_enum_cleanup(void *data); >> - >> -#endif /* EN_IC_DP_ENUM_H */ >> diff --git a/ic/en-tr.c b/ic/en-tr.c >> index c3b1b60c6..9c1eed3de 100644 >> --- a/ic/en-tr.c >> +++ b/ic/en-tr.c >> @@ -14,21 +14,100 @@ >> >> #include <config.h> >> >> -#include "en-dp-enum.h" >> -#include "en-tr.h" >> #include "en-az.h" >> +#include "en-tr.h" >> #include "lib/inc-proc-eng.h" >> +#include "lib/ovn-ic-nb-idl.h" >> +#include "lib/ovn-ic-sb-idl.h" >> +#include "lib/ovn-nb-idl.h" >> +#include "openvswitch/shash.h" >> #include "openvswitch/vlog.h" >> #include "ovn-ic.h" >> +#include "ovsdb-idl.h" >> +#include "smap.h" >> +#include "util.h" >> +#include "uuid.h" >> >> VLOG_DEFINE_THIS_MODULE(en_ic_tr); >> >> +/* Builds 'isb_tr_dps': the committed IC-SB transit-router >> Datapath_Bindings >> + * keyed by NB IC UUID string. Read-only; used to copy each committed >> tunnel >> + * key into its NB Logical_Router mirror's options:requested-tnl-key. >> This is >> + * en_tr's own local data, rebuilt each run and never mutated by another >> node. >> + */ >> +static void >> +collect_tr_datapaths(struct ic_context *ctx, struct shash *isb_tr_dps) >> +{ >> + shash_init(isb_tr_dps); >> + >> + const struct icsbrec_datapath_binding *isb_dp; >> + ICSBREC_DATAPATH_BINDING_FOR_EACH (isb_dp, ctx->ovnisb_idl) { >> + if (ic_dp_get_type(isb_dp) == IC_ROUTER) { >> + if (!isb_dp->nb_ic_uuid) { >> + /* Not inserted by ovn-ic: nothing to reconcile. */ >> + continue; >> + } >> + char *uuid_str = uuid_to_string(isb_dp->nb_ic_uuid); >> + shash_add(isb_tr_dps, uuid_str, isb_dp); >> + free(uuid_str); >> + } >> + } >> +} >> + >> +/* Syncs transit routers to their AZ NB Logical_Router mirrors: creates >> the >> + * mirror if missing, and copies each committed IC-SB Datapath_Binding >> tunnel >> + * key into options:requested-tnl-key. IC-SB Datapath_Binding >> creation/keying/ >> + * GC is owned by en_tunnel_key. */ >> +static void >> +tr_run(struct ic_context *ctx, struct shash *isb_tr_dps) >> +{ >> + if (!ctx->ovnnb_txn) { >> + return; >> + } >> + >> + const struct nbrec_logical_router *lr; >> + struct shash nb_tres = SHASH_INITIALIZER(&nb_tres); >> + NBREC_LOGICAL_ROUTER_FOR_EACH (lr, ctx->ovnnb_idl) { >> + const char *tr_name = smap_get(&lr->options, "interconn-tr"); >> + if (tr_name) { >> + shash_add(&nb_tres, tr_name, lr); >> + } >> + } >> + >> + const struct icnbrec_transit_router *tr; >> + ICNBREC_TRANSIT_ROUTER_FOR_EACH (tr, ctx->ovninb_idl) { >> + lr = shash_find_and_delete(&nb_tres, tr->name); >> + if (!lr) { >> + lr = nbrec_logical_router_insert(ctx->ovnnb_txn); >> + nbrec_logical_router_set_name(lr, tr->name); >> + nbrec_logical_router_update_options_setkey( >> + lr, "interconn-tr", tr->name); >> + } >> + char *uuid_str = uuid_to_string(&tr->header_.uuid); >> + const struct icsbrec_datapath_binding *isb_dp = >> + shash_find_data(isb_tr_dps, uuid_str); >> + free(uuid_str); >> + >> + if (isb_dp) { >> + char *tnl_key_str = xasprintf("%"PRId64, isb_dp->tunnel_key); >> + nbrec_logical_router_update_options_setkey( >> + lr, "requested-tnl-key", tnl_key_str); >> + free(tnl_key_str); >> + } >> + } >> + >> + struct shash_node *node; >> + SHASH_FOR_EACH (node, &nb_tres) { >> + nbrec_logical_router_delete(node->data); >> + } >> + shash_destroy(&nb_tres); >> +} >> + >> enum engine_node_state >> en_tr_run(struct engine_node *node, void *data OVS_UNUSED) >> { >> const struct engine_context *eng_ctx = engine_get_context(); >> struct ic_context *ctx = eng_ctx->client_ctx; >> - struct ed_type_dp_enum *dp = engine_get_input_data("dp_enum", node); >> const struct ed_type_az *az = engine_get_input_data("az", node); >> >> /* runned_az is resolved by the upstream en_az node. Without an AZ >> there >> @@ -37,7 +116,10 @@ en_tr_run(struct engine_node *node, void *data >> OVS_UNUSED) >> return EN_UNCHANGED; >> } >> >> - tr_run(ctx, &dp->dp_tnlids, &dp->isb_tr_dps); >> + struct shash isb_tr_dps; >> + collect_tr_datapaths(ctx, &isb_tr_dps); >> + tr_run(ctx, &isb_tr_dps); >> + shash_destroy(&isb_tr_dps); >> >> return EN_UPDATED; >> } >> diff --git a/ic/en-ts.c b/ic/en-ts.c >> index e4ec2a0b7..d983e6bb0 100644 >> --- a/ic/en-ts.c >> +++ b/ic/en-ts.c >> @@ -14,21 +14,44 @@ >> >> #include <config.h> >> >> -#include "en-dp-enum.h" >> -#include "en-ts.h" >> #include "en-az.h" >> +#include "en-ts.h" >> #include "lib/inc-proc-eng.h" >> +#include "lib/ovn-ic-nb-idl.h" >> +#include "lib/ovn-ic-sb-idl.h" >> +#include "lib/ovn-nb-idl.h" >> +#include "openvswitch/shash.h" >> #include "openvswitch/vlog.h" >> +#include "ovsdb-idl.h" >> +#include "smap.h" >> +#include "sset.h" >> #include "ovn-ic.h" >> >> VLOG_DEFINE_THIS_MODULE(en_ic_ts); >> >> +/* Builds 'isb_ts_dps': the committed IC-SB transit-switch >> Datapath_Bindings >> + * keyed by transit-switch name. Read-only; ts_sync_scope() copies each >> + * committed tunnel key into its NB mirror's requested-tnl-key. This is >> + * en_ts's own local data, rebuilt each run and never mutated by another >> + * node. */ >> +static void >> +collect_ts_datapaths(struct ic_context *ctx, struct shash *isb_ts_dps) >> +{ >> + shash_init(isb_ts_dps); >> + >> + const struct icsbrec_datapath_binding *isb_dp; >> + ICSBREC_DATAPATH_BINDING_FOR_EACH (isb_dp, ctx->ovnisb_idl) { >> + if (ic_dp_get_type(isb_dp) == IC_SWITCH) { >> + shash_add(isb_ts_dps, isb_dp->transit_switch, isb_dp); >> + } >> + } >> +} >> + >> enum engine_node_state >> en_ts_run(struct engine_node *node, void *data OVS_UNUSED) >> { >> const struct engine_context *eng_ctx = engine_get_context(); >> struct ic_context *ctx = eng_ctx->client_ctx; >> - struct ed_type_dp_enum *dp = engine_get_input_data("dp_enum", node); >> const struct ed_type_az *az = engine_get_input_data("az", node); >> >> /* runned_az is resolved by the upstream en_az node. Without an AZ >> there >> @@ -37,7 +60,10 @@ en_ts_run(struct engine_node *node, void *data >> OVS_UNUSED) >> return EN_UNCHANGED; >> } >> >> - ts_run(ctx, &dp->dp_tnlids, &dp->isb_ts_dps); >> + struct shash isb_ts_dps; >> + collect_ts_datapaths(ctx, &isb_ts_dps); >> + ts_sync_scope(ctx, &isb_ts_dps, NULL); >> + shash_destroy(&isb_ts_dps); >> >> return EN_UPDATED; >> } >> diff --git a/ic/en-tunnel-key.c b/ic/en-tunnel-key.c >> new file mode 100644 >> index 000000000..6e149554a >> --- /dev/null >> +++ b/ic/en-tunnel-key.c >> @@ -0,0 +1,449 @@ >> +/* >> + * Licensed under the Apache License, Version 2.0 (the "License"); >> + * you may not use this file except in compliance with the License. >> + * You may obtain a copy of the License at: >> + * >> + * http://www.apache.org/licenses/LICENSE-2.0 >> + * >> + * Unless required by applicable law or agreed to in writing, software >> + * distributed under the License is distributed on an "AS IS" BASIS, >> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >> implied. >> + * See the License for the specific language governing permissions and >> + * limitations under the License. >> + */ >> + >> +#include <config.h> >> + >> +#include "en-tunnel-key.h" >> +#include "lib/inc-proc-eng.h" >> +#include "lib/ovn-ic-nb-idl.h" >> +#include "lib/ovn-ic-sb-idl.h" >> +#include "lib/ovn-nb-idl.h" >> +#include "lib/ovn-util.h" >> +#include "openvswitch/hmap.h" >> +#include "openvswitch/shash.h" >> +#include "openvswitch/vlog.h" >> +#include "ovn-ic.h" >> +#include "ovsdb-idl.h" >> +#include "smap.h" >> +#include "sset.h" >> +#include "util.h" >> +#include "uuid.h" >> + >> +VLOG_DEFINE_THIS_MODULE(en_ic_tunnel_key); >> + >> +/* Returns true if 'key' falls in the global VXLAN datapath tunnel-key >> range. >> +*/ >> +static bool >> +dp_key_in_vxlan_range(int64_t key) >> +{ >> + return key >= OVN_MIN_DP_VXLAN_KEY_GLOBAL && >> + key <= OVN_MAX_DP_VXLAN_KEY_GLOBAL; >> +} >> + >> +/* Rebuilds the tunnel-key allocator 'data->dp_tnlids' and, in >> 'isb_ts_dps' >> + * (keyed by transit-switch name) / 'isb_tr_dps' (keyed by NB IC UUID >> string), >> + * every committed IC-SB Datapath_Binding. Read from the locked IC-SB >> IDL, >> + * which is the same IDL this node writes to and reads back from, so no >> + * locked/unlocked bridging is needed: the tracked changes (from the >> unlocked >> + * IDL) are used only to scope which entities to reconcile, never to >> mutate >> + * state directly. */ >> +static void >> +tunnel_key_build_state(struct ic_context *ctx, struct ed_type_tunnel_key >> *data, >> + struct shash *isb_ts_dps, struct shash >> *isb_tr_dps) >> +{ >> + ovn_destroy_tnlids(&data->dp_tnlids); >> + hmap_init(&data->dp_tnlids); >> + shash_init(isb_ts_dps); >> + shash_init(isb_tr_dps); >> + >> + const struct icsbrec_datapath_binding *isb_dp; >> + ICSBREC_DATAPATH_BINDING_FOR_EACH (isb_dp, ctx->ovnisb_idl) { >> + ovn_add_tnlid(&data->dp_tnlids, isb_dp->tunnel_key); >> + if (ic_dp_get_type(isb_dp) == IC_ROUTER) { >> + if (!isb_dp->nb_ic_uuid) { >> + /* Not inserted by ovn-ic: nothing to reconcile. */ >> + continue; >> + } >> + char *uuid_str = uuid_to_string(isb_dp->nb_ic_uuid); >> + shash_add(isb_tr_dps, uuid_str, isb_dp); >> + free(uuid_str); >> + } else { >> + shash_add(isb_ts_dps, isb_dp->transit_switch, isb_dp); >> + } >> + } >> +} >> + >> +/* Publishes 'dp_key' to the AZ NB Logical_Switch mirror of transit >> switch >> + * 'ts_name' ('nb_ts_mirrors', keyed by name), when the mirror exists. */ >> +static void >> +tunnel_key_publish_ts_mirror(struct shash *nb_ts_mirrors, const char >> *ts_name, >> + int64_t dp_key) >> +{ >> + const struct nbrec_logical_switch *ls = >> + shash_find_data(nb_ts_mirrors, ts_name); >> + if (!ls) { >> + return; >> + } >> + >> + char *tnl_key_str = xasprintf("%"PRId64, dp_key); >> + nbrec_logical_switch_update_other_config_setkey(ls, >> "requested-tnl-key", >> + tnl_key_str); >> + free(tnl_key_str); >> +} >> + >> +/* Reconciles one transit switch 'ts's IC-SB Datapath_Binding: creates >> it (with >> + * a fresh globally-unique tunnel key) when 'isb_dp' is NULL, >> reallocates its >> + * key if the encap (VXLAN) mode changed its range, and back-fills the >> type / >> + * nb_ic_uuid columns. Whenever a key is allocated - on creation and on >> + * reallocation alike - it is published to the AZ NB Logical_Switch >> mirror in >> + * this same iteration. Leaving the mirror to a later run is not safe: >> the >> + * committed key only reaches en_ts on a following engine run, and >> nothing >> + * guarantees one happens after the IC-SB transaction lands, so the >> mirror can >> + * keep a key from the wrong range indefinitely. */ >> +static void >> +tunnel_key_reconcile_ts(struct ic_context *ctx, >> + struct ed_type_tunnel_key *data, >> + const struct icnbrec_transit_switch *ts, >> + const struct icsbrec_datapath_binding *isb_dp, >> + struct shash *nb_ts_mirrors, bool vxlan_mode) >> +{ >> + if (!isb_dp) { >> + int64_t dp_key = allocate_dp_key(&data->dp_tnlids, vxlan_mode, >> + "transit switch datapath"); >> + if (!dp_key) { >> + return; >> + } >> + struct icsbrec_datapath_binding *new_dp = >> + icsbrec_datapath_binding_insert(ctx->ovnisb_txn); >> + icsbrec_datapath_binding_set_transit_switch(new_dp, ts->name); >> + icsbrec_datapath_binding_set_tunnel_key(new_dp, dp_key); >> + icsbrec_datapath_binding_set_type(new_dp, "transit-switch"); >> + icsbrec_datapath_binding_set_nb_ic_uuid(new_dp, >> &ts->header_.uuid, 1); >> + >> + tunnel_key_publish_ts_mirror(nb_ts_mirrors, ts->name, dp_key); >> + return; >> + } >> + >> + if (dp_key_in_vxlan_range(isb_dp->tunnel_key) != vxlan_mode) { >> + int64_t dp_key = allocate_dp_key(&data->dp_tnlids, vxlan_mode, >> + "transit switch datapath"); >> + if (dp_key) { >> + icsbrec_datapath_binding_set_tunnel_key(isb_dp, dp_key); >> + tunnel_key_publish_ts_mirror(nb_ts_mirrors, ts->name, >> dp_key); >> + } >> + } >> + if (!isb_dp->type) { >> + icsbrec_datapath_binding_set_type(isb_dp, "transit-switch"); >> + } >> + if (!isb_dp->nb_ic_uuid) { >> + icsbrec_datapath_binding_set_nb_ic_uuid(isb_dp, >> &ts->header_.uuid, 1); >> + } >> +} >> + >> +/* Reconciles one transit router 'tr's IC-SB Datapath_Binding: creates >> it (with >> + * a fresh globally-unique tunnel key, always from the non-VXLAN range) >> when >> + * 'isb_dp' is NULL. Its key reaches the NB Logical_Router mirror on a >> + * follow-up iteration (en_tr copies the committed key back). */ >> +static void >> +tunnel_key_reconcile_tr(struct ic_context *ctx, >> + struct ed_type_tunnel_key *data, >> + const struct icnbrec_transit_router *tr, >> + const struct icsbrec_datapath_binding *isb_dp) >> +{ >> + if (isb_dp) { >> + return; >> + } >> + >> + int64_t dp_key = allocate_dp_key(&data->dp_tnlids, false, >> + "transit router datapath"); >> + if (!dp_key) { >> + return; >> + } >> + struct icsbrec_datapath_binding *new_dp = >> + icsbrec_datapath_binding_insert(ctx->ovnisb_txn); >> + icsbrec_datapath_binding_set_tunnel_key(new_dp, dp_key); >> + icsbrec_datapath_binding_set_nb_ic_uuid(new_dp, &tr->header_.uuid, >> 1); >> + icsbrec_datapath_binding_set_type(new_dp, "transit-router"); >> +} >> + >> +/* Builds 'nb_ts_mirrors' (keyed by transit-switch name) from the AZ NB >> + * Logical_Switch mirrors. A table walk (not the by-name index) is used >> so it >> + * sees the mirrors en_ts just inserted in this transaction, which lets a >> + * brand-new binding publish its key to the mirror in the same >> iteration. */ >> +static void >> +tunnel_key_collect_nb_mirrors(struct ic_context *ctx, >> + struct shash *nb_ts_mirrors) >> +{ >> + shash_init(nb_ts_mirrors); >> + if (!ctx->ovnnb_txn) { >> + return; >> + } >> + const struct nbrec_logical_switch *ls; >> + NBREC_LOGICAL_SWITCH_FOR_EACH (ls, ctx->ovnnb_idl) { >> + const char *ts_name = smap_get(&ls->other_config, >> "interconn-ts"); >> + if (ts_name && !shash_find(nb_ts_mirrors, ts_name)) { >> + shash_add(nb_ts_mirrors, ts_name, >> + CONST_CAST(struct nbrec_logical_switch *, ls)); >> + } >> + } >> +} >> + >> +enum engine_node_state >> +en_tunnel_key_run(struct engine_node *node OVS_UNUSED, void *data_) >> +{ >> + const struct engine_context *eng_ctx = engine_get_context(); >> + struct ic_context *ctx = eng_ctx->client_ctx; >> + struct ed_type_tunnel_key *data = data_; >> + >> + struct shash isb_ts_dps, isb_tr_dps; >> + tunnel_key_build_state(ctx, data, &isb_ts_dps, &isb_tr_dps); >> + >> + /* The IC-SB Datapath_Binding table is the leader's to write; only >> touch it >> + * (create/refresh/delete) while holding the IC-SB lock. */ >> + if (ctx->ovnisb_txn && is_az_leader(ctx->ovnisb_txn)) { >> + bool vxlan_mode = ic_ts_compute_vxlan_mode(ctx); >> + struct shash nb_ts_mirrors; >> + tunnel_key_collect_nb_mirrors(ctx, &nb_ts_mirrors); >> + >> + const struct icnbrec_transit_switch *ts; >> + ICNBREC_TRANSIT_SWITCH_FOR_EACH (ts, ctx->ovninb_idl) { >> + const struct icsbrec_datapath_binding *isb_dp = >> + shash_find_and_delete(&isb_ts_dps, ts->name); >> + tunnel_key_reconcile_ts(ctx, data, ts, isb_dp, >> &nb_ts_mirrors, >> + vxlan_mode); >> + } >> + >> + const struct icnbrec_transit_router *tr; >> + ICNBREC_TRANSIT_ROUTER_FOR_EACH (tr, ctx->ovninb_idl) { >> + char *uuid_str = uuid_to_string(&tr->header_.uuid); >> + const struct icsbrec_datapath_binding *isb_dp = >> + shash_find_and_delete(&isb_tr_dps, uuid_str); >> + free(uuid_str); >> + tunnel_key_reconcile_tr(ctx, data, tr, isb_dp); >> + } >> + >> + /* Delete IC-SB Datapath_Bindings whose IC-NB entity is gone. */ >> + struct shash_node *sn; >> + SHASH_FOR_EACH (sn, &isb_ts_dps) { >> + icsbrec_datapath_binding_delete(sn->data); >> + } >> + SHASH_FOR_EACH (sn, &isb_tr_dps) { >> + icsbrec_datapath_binding_delete(sn->data); >> + } >> + >> + shash_destroy(&nb_ts_mirrors); >> + } >> + >> + shash_destroy(&isb_ts_dps); >> + shash_destroy(&isb_tr_dps); >> + >> + return EN_UPDATED; >> +} >> + >> +/* Reconciles only the transit switches named in 'ts_scope' and the >> transit >> + * routers whose NB IC UUID string is in 'tr_scope': creates a missing >> binding, >> + * refreshes a mis-ranged key, and deletes a binding whose IC-NB entity >> is >> + * gone. Returns EN_UNHANDLED (forcing a recompute) when the IC-SB >> + * lock/transaction needed to write the binding is not available yet. */ >> +static enum engine_input_handler_result >> +tunnel_key_sync_scope(struct ic_context *ctx, struct ed_type_tunnel_key >> *data, >> + struct sset *ts_scope, struct sset *tr_scope) >> +{ >> + if (sset_is_empty(ts_scope) && sset_is_empty(tr_scope)) { >> + return EN_HANDLED_UNCHANGED; >> + } >> + if (!ctx->ovnisb_txn || !is_az_leader(ctx->ovnisb_txn)) { >> + return EN_UNHANDLED; >> + } >> + >> + bool vxlan_mode = ic_ts_compute_vxlan_mode(ctx); >> + struct shash isb_ts_dps, isb_tr_dps; >> + tunnel_key_build_state(ctx, data, &isb_ts_dps, &isb_tr_dps); >> + struct shash nb_ts_mirrors; >> + tunnel_key_collect_nb_mirrors(ctx, &nb_ts_mirrors); >> + >> + /* IC-NB Transit_Switch has no generated get_for_name(); map name -> >> row so >> + * an in-scope transit switch can be looked up. */ >> + struct shash icnb_ts = SHASH_INITIALIZER(&icnb_ts); >> + if (!sset_is_empty(ts_scope)) { >> + const struct icnbrec_transit_switch *ts; >> + ICNBREC_TRANSIT_SWITCH_FOR_EACH (ts, ctx->ovninb_idl) { >> + if (!shash_find(&icnb_ts, ts->name)) { >> + shash_add(&icnb_ts, ts->name, >> + CONST_CAST(struct icnbrec_transit_switch *, >> ts)); >> + } >> + } >> + } >> + >> + const char *name; >> + SSET_FOR_EACH (name, ts_scope) { >> + const struct icnbrec_transit_switch *ts = >> + shash_find_data(&icnb_ts, name); >> + const struct icsbrec_datapath_binding *isb_dp = >> + shash_find_data(&isb_ts_dps, name); >> + if (ts) { >> + tunnel_key_reconcile_ts(ctx, data, ts, isb_dp, >> &nb_ts_mirrors, >> + vxlan_mode); >> + } else if (isb_dp) { >> + icsbrec_datapath_binding_delete(isb_dp); >> + } >> + } >> + shash_destroy(&icnb_ts); >> + >> + const char *uuid_str; >> + SSET_FOR_EACH (uuid_str, tr_scope) { >> + struct uuid tr_uuid; >> + if (!uuid_from_string(&tr_uuid, uuid_str)) { >> + continue; >> + } >> + const struct icnbrec_transit_router *tr = >> + icnbrec_transit_router_get_for_uuid(ctx->ovninb_idl, >> &tr_uuid); >> + const struct icsbrec_datapath_binding *isb_dp = >> + shash_find_data(&isb_tr_dps, uuid_str); >> + if (tr) { >> + tunnel_key_reconcile_tr(ctx, data, tr, isb_dp); >> + } else if (isb_dp) { >> + icsbrec_datapath_binding_delete(isb_dp); >> + } >> + } >> + >> + shash_destroy(&nb_ts_mirrors); >> + shash_destroy(&isb_ts_dps); >> + shash_destroy(&isb_tr_dps); >> + >> + return EN_HANDLED_UPDATED; >> +} >> + >> +/* IC-NB Transit_Switch: a new transit switch needs its IC-SB >> Datapath_Binding >> + * created; a deleted one needs it garbage-collected. Both are scoped >> by name >> + * and reconciled without a full recompute. */ >> +enum engine_input_handler_result >> +en_tunnel_key_icnb_transit_switch_handler(struct engine_node *node, >> + void *data_) >> +{ >> + struct ed_type_tunnel_key *data = data_; >> + struct ic_context *ctx = engine_get_context()->client_ctx; >> + const struct icnbrec_transit_switch_table *tbl = >> + EN_OVSDB_GET(engine_get_input("ICNB_transit_switch", node)); >> + struct sset ts_scope = SSET_INITIALIZER(&ts_scope); >> + struct sset tr_scope = SSET_INITIALIZER(&tr_scope); >> + const struct icnbrec_transit_switch *ts; >> + ICNBREC_TRANSIT_SWITCH_TABLE_FOR_EACH_TRACKED (ts, tbl) { >> > > You could check if the transit switch's name is updated and maybe > fall back to recompute. > Transit router uses uuid instead. > > Regards, > Lucas > > + sset_add(&ts_scope, ts->name); >> + } >> + >> + enum engine_input_handler_result ret = >> + tunnel_key_sync_scope(ctx, data, &ts_scope, &tr_scope); >> + sset_destroy(&ts_scope); >> + sset_destroy(&tr_scope); >> + return ret; >> +} >> + >> +/* IC-NB Transit_Router: analogous to the transit-switch handler, scoped >> by the >> + * transit router's NB IC UUID (the binding key). */ >> +enum engine_input_handler_result >> +en_tunnel_key_icnb_transit_router_handler(struct engine_node *node, >> + void *data_) >> +{ >> + struct ed_type_tunnel_key *data = data_; >> + struct ic_context *ctx = engine_get_context()->client_ctx; >> + const struct icnbrec_transit_router_table *tbl = >> + EN_OVSDB_GET(engine_get_input("ICNB_transit_router", node)); >> + struct sset ts_scope = SSET_INITIALIZER(&ts_scope); >> + struct sset tr_scope = SSET_INITIALIZER(&tr_scope); >> + const struct icnbrec_transit_router *tr; >> + ICNBREC_TRANSIT_ROUTER_TABLE_FOR_EACH_TRACKED (tr, tbl) { >> + char *uuid_str = uuid_to_string(&tr->header_.uuid); >> + sset_add(&tr_scope, uuid_str); >> + free(uuid_str); >> + } >> + >> + enum engine_input_handler_result ret = >> + tunnel_key_sync_scope(ctx, data, &ts_scope, &tr_scope); >> + sset_destroy(&ts_scope); >> + sset_destroy(&tr_scope); >> + return ret; >> +} >> + >> +/* IC-SB Datapath_Binding: react only to a binding *deletion* by >> re-creating it >> + * for the still-present IC-NB entity (self-healing after an out-of-band >> + * delete). Inserts and modifies are this node's own writes (or already >> + * consistent), so they are a no-op here - the requested-tnl-key >> follow-up is >> + * driven by en_ts/en_tr's own handlers on this same table. */ >> +enum engine_input_handler_result >> +en_tunnel_key_icsb_datapath_binding_handler(struct engine_node *node, >> + void *data_) >> +{ >> + struct ed_type_tunnel_key *data = data_; >> + struct ic_context *ctx = engine_get_context()->client_ctx; >> + const struct icsbrec_datapath_binding_table *tbl = >> + EN_OVSDB_GET(engine_get_input("ICSB_datapath_binding", node)); >> + struct sset ts_scope = SSET_INITIALIZER(&ts_scope); >> + struct sset tr_scope = SSET_INITIALIZER(&tr_scope); >> + const struct icsbrec_datapath_binding *isb_dp; >> + ICSBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (isb_dp, tbl) { >> + if (!icsbrec_datapath_binding_is_deleted(isb_dp)) { >> + continue; >> + } >> + if (ic_dp_get_type(isb_dp) == IC_ROUTER) { >> + if (!isb_dp->nb_ic_uuid) { >> + /* Not inserted by ovn-ic: nothing to reconcile. */ >> + continue; >> + } >> + char *uuid_str = uuid_to_string(isb_dp->nb_ic_uuid); >> + sset_add(&tr_scope, uuid_str); >> + free(uuid_str); >> + } else { >> + sset_add(&ts_scope, isb_dp->transit_switch); >> + } >> + } >> + >> + enum engine_input_handler_result ret = >> + tunnel_key_sync_scope(ctx, data, &ts_scope, &tr_scope); >> + sset_destroy(&ts_scope); >> + sset_destroy(&tr_scope); >> + return ret; >> +} >> + >> +/* Shared change handler for the IC-NB Global input: ignores >> + * nb_ic_cfg/sb_ic_cfg sequence-number-only changes, and forces a >> recompute >> + * only when the row is created/deleted or its 'options' (e.g. >> vxlan_mode) >> + * change. */ >> +enum engine_input_handler_result >> +ic_nb_global_options_handler(struct engine_node *node, void *data >> OVS_UNUSED) >> +{ >> + const struct icnbrec_ic_nb_global_table *tbl = >> + EN_OVSDB_GET(engine_get_input("ICNB_ic_nb_global", node)); >> + const struct icnbrec_ic_nb_global *icnb; >> + >> + ICNBREC_IC_NB_GLOBAL_TABLE_FOR_EACH_TRACKED (icnb, tbl) { >> + if (icnbrec_ic_nb_global_is_new(icnb) || >> + icnbrec_ic_nb_global_is_deleted(icnb) || >> + ovsdb_idl_track_is_updated(&icnb->header_, >> + >> &icnbrec_ic_nb_global_col_options)) { >> + return EN_UNHANDLED; >> + } >> + } >> + >> + return EN_HANDLED_UNCHANGED; >> +} >> + >> +void * >> +en_tunnel_key_init(struct engine_node *node OVS_UNUSED, >> + struct engine_arg *arg OVS_UNUSED) >> +{ >> + struct ed_type_tunnel_key *data = xzalloc(sizeof *data); >> + >> + hmap_init(&data->dp_tnlids); >> + >> + return data; >> +} >> + >> +void >> +en_tunnel_key_cleanup(void *data_) >> +{ >> + struct ed_type_tunnel_key *data = data_; >> + >> + ovn_destroy_tnlids(&data->dp_tnlids); >> +} >> diff --git a/ic/en-tunnel-key.h b/ic/en-tunnel-key.h >> new file mode 100644 >> index 000000000..d8f252cb7 >> --- /dev/null >> +++ b/ic/en-tunnel-key.h >> @@ -0,0 +1,60 @@ >> +/* >> + * Licensed under the Apache License, Version 2.0 (the "License"); >> + * you may not use this file except in compliance with the License. >> + * You may obtain a copy of the License at: >> + * >> + * http://www.apache.org/licenses/LICENSE-2.0 >> + * >> + * Unless required by applicable law or agreed to in writing, software >> + * distributed under the License is distributed on an "AS IS" BASIS, >> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >> implied. >> + * See the License for the specific language governing permissions and >> + * limitations under the License. >> + */ >> + >> +#ifndef EN_IC_TUNNEL_KEY_H >> +#define EN_IC_TUNNEL_KEY_H 1 >> + >> +#include "lib/inc-proc-eng.h" >> +#include "openvswitch/hmap.h" >> + >> +/* Data maintained by the "tunnel_key" engine node: the set of datapath >> tunnel >> + * keys currently in use by IC-SB Datapath_Bindings. This node is the >> single >> + * owner of datapath tunnel-key allocation: it runs after en_ts and en_tr >> + * (which only maintain the AZ NB Logical_Switch/Router mirrors) and >> + * reconciles every transit switch/router's IC-SB Datapath_Binding, >> allocating >> + * a globally unique tunnel key for each new one. Concentrating >> allocation in >> + * one node keeps the keys unique across both datapath types without any >> node >> + * mutating another node's data. */ >> +struct ed_type_tunnel_key { >> + /* Set of tunnel keys already in use by IC-SB Datapath_Bindings. >> Holds >> + * 'struct tnlid_node *' entries owned by this node. */ >> + struct hmap dp_tnlids; >> +}; >> + >> +enum engine_node_state en_tunnel_key_run(struct engine_node *node, void >> *data); >> +void *en_tunnel_key_init(struct engine_node *node, struct engine_arg >> *arg); >> +void en_tunnel_key_cleanup(void *data); >> + >> +/* Scoped incremental handlers: create/refresh/GC the IC-SB >> Datapath_Binding of >> + * only the transit switches/routers whose IC-NB row or IC-SB binding >> changed, >> + * without a full recompute. */ >> +enum engine_input_handler_result >> +en_tunnel_key_icnb_transit_switch_handler(struct engine_node *node, >> + void *data); >> +enum engine_input_handler_result >> +en_tunnel_key_icnb_transit_router_handler(struct engine_node *node, >> + void *data); >> +enum engine_input_handler_result >> +en_tunnel_key_icsb_datapath_binding_handler(struct engine_node *node, >> + void *data); >> + >> +/* Shared change handler for the IC-NB Global input of nodes that only >> care >> + * about its 'options' (e.g. vxlan_mode), not the nb_ic_cfg/sb_ic_cfg >> + * sequence numbers that the sequence-number protocol bumps on every >> change. >> + * Named without a node prefix because it is shared by en_tunnel_key and >> + * en_ts. */ >> +enum engine_input_handler_result >> +ic_nb_global_options_handler(struct engine_node *node, void *data); >> + >> +#endif /* EN_IC_TUNNEL_KEY_H */ >> diff --git a/ic/inc-proc-ic.c b/ic/inc-proc-ic.c >> index 66958dbf0..64aef2c82 100644 >> --- a/ic/inc-proc-ic.c >> +++ b/ic/inc-proc-ic.c >> @@ -28,10 +28,10 @@ >> #include "inc-proc-ic.h" >> #include "en-ic.h" >> #include "en-az.h" >> -#include "en-dp-enum.h" >> #include "en-gateway.h" >> #include "en-ts.h" >> #include "en-tr.h" >> +#include "en-tunnel-key.h" >> #include "en-port-binding.h" >> #include "en-route.h" >> #include "en-service-monitor.h" >> @@ -175,10 +175,10 @@ VLOG_DEFINE_THIS_MODULE(inc_proc_ic); >> /* Define engine nodes for other nodes. They should be defined as static >> to >> * avoid sparse errors. */ >> static ENGINE_NODE(az); >> -static ENGINE_NODE(dp_enum); >> static ENGINE_NODE(gateway); >> static ENGINE_NODE(ts); >> static ENGINE_NODE(tr); >> +static ENGINE_NODE(tunnel_key); >> static ENGINE_NODE(port_binding); >> static ENGINE_NODE(route); >> static ENGINE_NODE(service_monitor); >> @@ -201,21 +201,6 @@ void inc_proc_ic_init(struct ovsdb_idl_loop *nb, >> * splitting the monolithic ovn_db_run() into independently-gated >> nodes. >> * Change handlers are added incrementally in a later step. */ >> >> - /* en_dp_enum: enumerate IC-SB datapath bindings (tunnel-key >> allocator and >> - * transit switch/router datapath maps shared by en_ts and en_tr). >> - * >> - * en_ts and en_tr allocate datapath tunnel keys from the shared >> - * 'dp_tnlids' set owned by this node, mutating it during their >> run. To >> - * keep that allocator correct, en_dp_enum must rebuild it from >> scratch in >> - * the same iteration as any allocation. It therefore depends not >> only on >> - * the IC-SB datapath bindings themselves, but also on every input >> that can >> - * cause en_ts/en_tr to allocate a key: a new transit switch or >> router, or >> - * a change of vxlan_mode (which forces a tunnel-key refresh). */ >> - engine_add_input(&en_dp_enum, &en_icsb_datapath_binding, NULL); >> - engine_add_input(&en_dp_enum, &en_icnb_transit_switch, NULL); >> - engine_add_input(&en_dp_enum, &en_icnb_transit_router, NULL); >> - engine_add_input(&en_dp_enum, &en_icnb_ic_nb_global, NULL); >> - >> /* en_gateway: sync gateways/chassis between SB and IC-SB. */ >> engine_add_input(&en_gateway, &en_az, NULL); >> engine_add_input(&en_gateway, &en_icsb_availability_zone, NULL); >> @@ -224,20 +209,49 @@ void inc_proc_ic_init(struct ovsdb_idl_loop *nb, >> engine_add_input(&en_gateway, &en_sb_chassis, NULL); >> engine_add_input(&en_gateway, &en_sb_encap, NULL); >> >> - /* en_ts: sync transit switches to NB and IC-SB datapath bindings. */ >> + /* en_ts: sync transit switches to their AZ NB Logical_Switch >> mirrors. >> + * en_ts builds its own transit-switch 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 (downstream). */ >> engine_add_input(&en_ts, &en_az, NULL); >> - engine_add_input(&en_ts, &en_dp_enum, NULL); >> + engine_add_input(&en_ts, &en_icsb_datapath_binding, NULL); >> engine_add_input(&en_ts, &en_icnb_ic_nb_global, NULL); >> engine_add_input(&en_ts, &en_icnb_transit_switch, NULL); >> engine_add_input(&en_ts, &en_nb_logical_switch, NULL); >> engine_add_input(&en_ts, &en_icsb_encap, NULL); >> >> - /* en_tr: sync transit routers to NB and IC-SB datapath bindings. */ >> + /* 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. */ >> engine_add_input(&en_tr, &en_az, NULL); >> - engine_add_input(&en_tr, &en_dp_enum, NULL); >> + engine_add_input(&en_tr, &en_icsb_datapath_binding, NULL); >> engine_add_input(&en_tr, &en_icnb_transit_router, NULL); >> engine_add_input(&en_tr, &en_nb_logical_router, NULL); >> >> + /* en_tunnel_key: the single owner of IC-SB Datapath_Binding >> creation, >> + * tunnel-key allocation, VXLAN-range refresh and GC, for both >> transit >> + * switches and transit routers. Concentrating allocation in one >> node >> + * keeps the keys globally unique across both datapath types without >> any >> + * node mutating another's data. >> + * >> + * It is ordered after en_ts and en_tr (no-op edges) so the AZ NB >> mirrors >> + * exist before it publishes a brand-new binding's key to them (the >> + * anti-flap early publish in en_tunnel_key_run()). The IC-NB >> transit >> + * switch/router and IC-SB Datapath_Binding inputs drive create/GC; >> the >> + * IC-NB Global (vxlan_mode) and IC-SB Encap inputs drive the >> VXLAN-range >> + * refresh. */ >> + engine_add_input(&en_tunnel_key, &en_ts, engine_noop_handler); >> + engine_add_input(&en_tunnel_key, &en_tr, engine_noop_handler); >> + engine_add_input(&en_tunnel_key, &en_icsb_datapath_binding, >> + en_tunnel_key_icsb_datapath_binding_handler); >> + engine_add_input(&en_tunnel_key, &en_icnb_transit_switch, >> + en_tunnel_key_icnb_transit_switch_handler); >> + engine_add_input(&en_tunnel_key, &en_icnb_transit_router, >> + en_tunnel_key_icnb_transit_router_handler); >> + engine_add_input(&en_tunnel_key, &en_icnb_ic_nb_global, >> + ic_nb_global_options_handler); >> + engine_add_input(&en_tunnel_key, &en_icsb_encap, NULL); >> + >> /* en_port_binding: sync cross-AZ port bindings. */ >> engine_add_input(&en_port_binding, &en_az, NULL); >> engine_add_input(&en_port_binding, &en_icsb_availability_zone, NULL); >> @@ -290,13 +304,13 @@ void inc_proc_ic_init(struct ovsdb_idl_loop *nb, >> engine_add_input(&en_address_set, &en_sb_address_set, NULL); >> engine_add_input(&en_address_set, &en_icsb_address_set, NULL); >> >> - /* en_ic: output node aggregating all subsystems. Order matches the >> - * previous ovn_db_run() call order; in particular en_ts is added >> before >> - * en_tr so they allocate datapath tunnel keys from the shared >> en_dp_enum >> - * allocator in the same order as before. */ >> + /* en_ic: output node aggregating all subsystems. en_tunnel_key is >> added >> + * after en_ts and en_tr, matching its ordering dependency on them >> (it >> + * publishes a new binding's key to the NB mirror they create). */ >> engine_add_input(&en_ic, &en_gateway, NULL); >> engine_add_input(&en_ic, &en_ts, NULL); >> engine_add_input(&en_ic, &en_tr, NULL); >> + engine_add_input(&en_ic, &en_tunnel_key, NULL); >> engine_add_input(&en_ic, &en_port_binding, NULL); >> engine_add_input(&en_ic, &en_route, NULL); >> engine_add_input(&en_ic, &en_service_monitor, NULL); >> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c >> index 773762ae4..57a55eff0 100644 >> --- a/ic/ovn-ic.c >> +++ b/ic/ovn-ic.c >> @@ -99,7 +99,7 @@ Options:\n\ >> stream_usage("database", true, true, false); >> } >> >> -static uint32_t >> +uint32_t >> allocate_dp_key(struct hmap *dp_tnlids, bool vxlan_mode, const char >> *name) >> { >> uint32_t hint = vxlan_mode ? OVN_MIN_DP_VXLAN_KEY_GLOBAL >> @@ -109,7 +109,7 @@ allocate_dp_key(struct hmap *dp_tnlids, bool >> vxlan_mode, const char *name) >> &hint); >> } >> >> -static enum ic_datapath_type >> +enum ic_datapath_type >> ic_dp_get_type(const struct icsbrec_datapath_binding *isb_dp) >> { >> if (isb_dp->type && !strcmp(isb_dp->type, "transit-router")) { >> @@ -129,250 +129,177 @@ ic_pb_get_type(const struct icsbrec_port_binding >> *isb_pb) >> return IC_SWITCH_PORT; >> } >> >> -void >> -enumerate_datapaths(struct ic_context *ctx, struct hmap *dp_tnlids, >> - struct shash *isb_ts_dps, struct shash *isb_tr_dps) >> -{ >> - const struct icsbrec_datapath_binding *isb_dp; >> - ICSBREC_DATAPATH_BINDING_FOR_EACH (isb_dp, ctx->ovnisb_idl) { >> - ovn_add_tnlid(dp_tnlids, isb_dp->tunnel_key); >> - >> - enum ic_datapath_type dp_type = ic_dp_get_type(isb_dp); >> - if (dp_type == IC_ROUTER) { >> - char *uuid_str = uuid_to_string(isb_dp->nb_ic_uuid); >> - shash_add(isb_tr_dps, uuid_str, isb_dp); >> - free(uuid_str); >> - } else { >> - shash_add(isb_ts_dps, isb_dp->transit_switch, isb_dp); >> - } >> - } >> -} >> - >> /* >> * Check if the AZ is the leader by checking the lock. >> */ >> -static bool >> +bool >> is_az_leader(struct ovsdb_idl_txn *txn) >> { >> struct ovsdb_idl *idl = ovsdb_idl_txn_get_idl(txn); >> return idl && ovsdb_idl_has_lock(idl); >> } >> >> -void >> -ts_run(struct ic_context *ctx, struct hmap *dp_tnlids, >> - struct shash *isb_ts_dps) >> +/* Returns true if transit-switch datapaths must use the VXLAN tunnel-key >> + * range: IC-NB requests vxlan_mode and the IC-SB actually has a VXLAN >> encap. >> + * >> + * Warning: ovnisb_unlocked should not be used to insert data on IC_SB >> which >> + * can cause a constraint violation, as an example, inserting data to >> IC-SB >> + * datapath_binding. */ >> +bool >> +ic_ts_compute_vxlan_mode(struct ic_context *ctx) >> { >> - const struct icnbrec_transit_switch *ts; >> - bool dp_key_refresh = false; >> - bool vxlan_mode = false; >> const struct icnbrec_ic_nb_global *ic_nb = >> icnbrec_ic_nb_global_first(ctx->ovninb_idl); >> >> - /* >> - * Warning: ovnisb_unlocked should not be used to insert data on >> IC_SB >> - * which can cause a constraint violation, as an example, inserting >> data to >> - * IC-SB datapath_binding. >> - */ >> if (ic_nb && smap_get_bool(&ic_nb->options, "vxlan_mode", false)) { >> const struct icsbrec_encap *encap; >> ICSBREC_ENCAP_FOR_EACH (encap, ctx->ovnisb_unlocked_idl) { >> if (!strcmp(encap->type, "vxlan")) { >> - vxlan_mode = true; >> - break; >> + return true; >> } >> } >> } >> + return false; >> +} >> >> - /* Sync INB TS to AZ NB */ >> - if (ctx->ovnnb_txn) { >> - struct shash nb_tses = SHASH_INITIALIZER(&nb_tses); >> - const struct nbrec_logical_switch *ls; >> - >> - /* Get current NB Logical_Switch with other_config:interconn-ts >> */ >> - NBREC_LOGICAL_SWITCH_FOR_EACH (ls, ctx->ovnnb_idl) { >> - const char *ts_name = smap_get(&ls->other_config, >> "interconn-ts"); >> - if (ts_name) { >> - shash_add(&nb_tses, ts_name, ls); >> - } >> - } >> - >> - /* Create/update NB Logical_Switch for each TS */ >> - ICNBREC_TRANSIT_SWITCH_FOR_EACH (ts, ctx->ovninb_idl) { >> - ls = shash_find_and_delete(&nb_tses, ts->name); >> - if (!ls) { >> - ls = nbrec_logical_switch_insert(ctx->ovnnb_txn); >> - nbrec_logical_switch_set_name(ls, ts->name); >> - nbrec_logical_switch_update_other_config_setkey(ls, >> - >> "interconn-ts", >> - >> ts->name); >> - nbrec_logical_switch_update_other_config_setkey( >> - ls, "ic-vxlan_mode", vxlan_mode ? "true" : >> "false"); >> - } else { >> - bool _vxlan_mode = smap_get_bool(&ls->other_config, >> - "ic-vxlan_mode", false); >> - if (_vxlan_mode != vxlan_mode) { >> - dp_key_refresh = true; >> - nbrec_logical_switch_update_other_config_setkey( >> - ls, "ic-vxlan_mode", >> - vxlan_mode ? "true" : "false"); >> - } >> - } >> - >> - const struct icsbrec_datapath_binding *isb_dp; >> - isb_dp = shash_find_data(isb_ts_dps, ts->name); >> - if (isb_dp) { >> - int64_t nb_tnl_key = smap_get_int(&ls->other_config, >> - "requested-tnl-key", >> - 0); >> - if (nb_tnl_key != isb_dp->tunnel_key) { >> - VLOG_DBG("Set other_config:requested-tnl-key %"PRId64 >> - " for transit switch %s in NB.", >> - isb_dp->tunnel_key, ts->name); >> - char *tnl_key_str = xasprintf("%"PRId64, >> - isb_dp->tunnel_key); >> - nbrec_logical_switch_update_other_config_setkey( >> - ls, "requested-tnl-key", tnl_key_str); >> - free(tnl_key_str); >> - } >> - } >> - } >> - >> - /* Delete extra NB Logical_Switch with other_config:interconn-ts >> */ >> - struct shash_node *node; >> - SHASH_FOR_EACH (node, &nb_tses) { >> - nbrec_logical_switch_delete(node->data); >> - } >> - shash_destroy(&nb_tses); >> - } >> - >> - /* Sync TS between INB and ISB. This is performed after syncing >> with AZ >> - * SB, to avoid uncommitted ISB datapath tunnel key to be synced >> back to >> - * AZ. */ >> - if (ctx->ovnisb_txn && >> - is_az_leader(ctx->ovnisb_txn)) { >> - /* Create ISB Datapath_Binding */ >> - ICNBREC_TRANSIT_SWITCH_FOR_EACH (ts, ctx->ovninb_idl) { >> - const struct icsbrec_datapath_binding *isb_dp = >> - shash_find_and_delete(isb_ts_dps, ts->name); >> - if (!isb_dp) { >> - /* Allocate tunnel key */ >> - int64_t dp_key = allocate_dp_key(dp_tnlids, vxlan_mode, >> - "transit switch >> datapath"); >> - if (!dp_key) { >> - continue; >> - } >> - >> - isb_dp = >> icsbrec_datapath_binding_insert(ctx->ovnisb_txn); >> - icsbrec_datapath_binding_set_transit_switch(isb_dp, >> ts->name); >> - icsbrec_datapath_binding_set_tunnel_key(isb_dp, dp_key); >> - } else if (dp_key_refresh) { >> - /* Refresh tunnel key since encap mode has changed. */ >> - int64_t dp_key = allocate_dp_key(dp_tnlids, vxlan_mode, >> - "transit switch >> datapath"); >> - if (dp_key) { >> - icsbrec_datapath_binding_set_tunnel_key(isb_dp, >> dp_key); >> - } >> - } >> - >> - if (!isb_dp->type) { >> - icsbrec_datapath_binding_set_type(isb_dp, >> "transit-switch"); >> - } >> +/* Keep other_config:ic-vxlan_mode on the transit switch's NB >> Logical_Switch >> + * mirror in sync with the IC VXLAN mode. northd reads the boolean >> value with >> + * smap_get_bool() (northd/en-global-config.c), so writing "false" is >> correct >> + * and does not affect the datapath tunnel-id range. Only write when >> the value >> + * actually differs to avoid needless transaction churn. */ >> +static void >> +ts_set_ic_vxlan_mode(const struct nbrec_logical_switch *ls, bool >> vxlan_mode) >> +{ >> + const char *want = vxlan_mode ? "true" : "false"; >> + const char *cur = smap_get(&ls->other_config, "ic-vxlan_mode"); >> + if (!cur || strcmp(cur, want)) { >> + nbrec_logical_switch_update_other_config_setkey(ls, >> "ic-vxlan_mode", >> + want); >> + } >> +} >> >> - if (!isb_dp->nb_ic_uuid) { >> - icsbrec_datapath_binding_set_nb_ic_uuid(isb_dp, >> - >> &ts->header_.uuid, 1); >> - } >> - } >> +/* Reconciles a single transit switch 'ts's AZ NB Logical_Switch mirror: >> + * creates it if missing, keeps other_config:ic-vxlan_mode in sync, and >> copies >> + * the committed IC-SB Datapath_Binding tunnel key into >> + * other_config:requested-tnl-key. IC-SB Datapath_Binding >> creation/keying is >> + * owned by en_tunnel_key. >> + * >> + * 'nb_gc' (keyed by transit-switch name) doubles as a >> garbage-collection set: >> + * this function removes the mirror it claims, so whatever remains after >> every >> + * in-scope switch has been reconciled is stale and deleted by the >> caller. >> + * 'isb_gc' (the IC-SB Datapath_Binding map keyed by transit-switch >> name) is >> + * read-only here. */ >> +static void >> +ts_sync_one(struct ic_context *ctx, const struct icnbrec_transit_switch >> *ts, >> + struct shash *isb_gc, struct shash *nb_gc, bool vxlan_mode) >> +{ >> + if (!nb_gc) { >> + return; >> + } >> >> - struct shash_node *node; >> - SHASH_FOR_EACH (node, isb_ts_dps) { >> - icsbrec_datapath_binding_delete(node->data); >> + const struct nbrec_logical_switch *ls = >> + shash_find_and_delete(nb_gc, ts->name); >> + if (!ls) { >> + ls = nbrec_logical_switch_insert(ctx->ovnnb_txn); >> + nbrec_logical_switch_set_name(ls, ts->name); >> + >> + /* Write other_config as a whole column rather than with per-key >> + * partial map updates. A partial update is only queued as a map >> + * operation and is not reflected in 'ls->other_config' until the >> + * transaction commits, so en_tunnel_key would not recognize this >> + * brand-new row as a transit switch mirror and could not >> publish the >> + * freshly allocated tunnel key to it in this same iteration. >> northd >> + * would then pick its own datapath tunnel key and flap it once >> + * requested-tnl-key finally showed up. */ >> + struct smap other_config = SMAP_INITIALIZER(&other_config); >> + smap_add(&other_config, "interconn-ts", ts->name); >> + smap_add(&other_config, "ic-vxlan_mode", >> + vxlan_mode ? "true" : "false"); >> + nbrec_logical_switch_set_other_config(ls, &other_config); >> + smap_destroy(&other_config); >> + } else { >> + ts_set_ic_vxlan_mode(ls, vxlan_mode); >> + } >> + >> + const struct icsbrec_datapath_binding *isb_dp = >> + shash_find_data(isb_gc, ts->name); >> + if (isb_dp) { >> + int64_t nb_tnl_key = smap_get_int(&ls->other_config, >> + "requested-tnl-key", 0); >> + if (nb_tnl_key != isb_dp->tunnel_key) { >> + VLOG_DBG("Set other_config:requested-tnl-key %"PRId64 >> + " for transit switch %s in NB.", >> + isb_dp->tunnel_key, ts->name); >> + char *tnl_key_str = xasprintf("%"PRId64, isb_dp->tunnel_key); >> + nbrec_logical_switch_update_other_config_setkey( >> + ls, "requested-tnl-key", tnl_key_str); >> + free(tnl_key_str); >> } >> } >> } >> >> +/* Synchronizes transit switches to their AZ NB Logical_Switch mirrors. >> When >> + * 'ts_scope' is NULL every transit switch is reconciled (full >> recompute); >> + * otherwise only the switches named in 'ts_scope' are. A name still in >> scope >> + * but no longer present in IC-NB (a deleted switch) is honoured: its >> mirror >> + * ends up as a garbage-collection leftover and is deleted, matching >> + * full-recompute behaviour. >> + * >> + * 'isb_ts_dps' (the IC-SB Datapath_Binding map keyed by transit-switch >> name) >> + * is read only, to copy each committed tunnel key into the mirror's >> + * requested-tnl-key. IC-SB Datapath_Binding creation/keying/GC is >> owned by >> + * en_tunnel_key, so this function neither allocates keys nor mutates >> + * 'isb_ts_dps'. */ >> void >> -tr_run(struct ic_context *ctx, struct hmap *dp_tnlids, >> - struct shash *isb_tr_dps) >> +ts_sync_scope(struct ic_context *ctx, struct shash *isb_ts_dps, >> + const struct sset *ts_scope) >> { >> - /* >> - * Warning: ovnisb_unlocked should not be used to insert data on >> IC_SB >> - * which can cause a constraint violation, as an example, inserting >> data to >> - * IC-SB datapath_binding. >> - */ >> - const struct nbrec_logical_router *lr; >> + bool full = !ts_scope; >> + bool vxlan_mode = ic_ts_compute_vxlan_mode(ctx); >> >> + /* Build the NB Logical_Switch mirror GC set, keyed by transit-switch >> + * name. Only needed when an NB transaction is available. */ >> + struct shash nb_ts_mirrors = SHASH_INITIALIZER(&nb_ts_mirrors); >> + struct shash *nb_gc = NULL; >> if (ctx->ovnnb_txn) { >> - struct shash nb_tres = SHASH_INITIALIZER(&nb_tres); >> - NBREC_LOGICAL_ROUTER_FOR_EACH (lr, ctx->ovnnb_idl) { >> - const char *tr_name = smap_get(&lr->options, "interconn-tr"); >> - if (tr_name) { >> - shash_add(&nb_tres, tr_name, lr); >> - } >> - } >> - >> - const struct icnbrec_transit_router *tr; >> - ICNBREC_TRANSIT_ROUTER_FOR_EACH (tr, ctx->ovninb_idl) { >> - lr = shash_find_and_delete(&nb_tres, tr->name); >> - if (!lr) { >> - lr = nbrec_logical_router_insert(ctx->ovnnb_txn); >> - nbrec_logical_router_set_name(lr, tr->name); >> - nbrec_logical_router_update_options_setkey( >> - lr, "interconn-tr", tr->name); >> + nb_gc = &nb_ts_mirrors; >> + if (full) { >> + const struct nbrec_logical_switch *ls; >> + NBREC_LOGICAL_SWITCH_FOR_EACH (ls, ctx->ovnnb_idl) { >> + const char *ts_name = smap_get(&ls->other_config, >> + "interconn-ts"); >> + if (ts_name) { >> + shash_add(nb_gc, ts_name, ls); >> + } >> } >> - char *uuid_str = uuid_to_string(&tr->header_.uuid); >> - struct icsbrec_datapath_binding *isb_dp = shash_find_data( >> - isb_tr_dps, uuid_str); >> - free(uuid_str); >> - >> - if (isb_dp) { >> - char *tnl_key_str = xasprintf("%"PRId64, >> isb_dp->tunnel_key); >> - nbrec_logical_router_update_options_setkey( >> - lr, "requested-tnl-key", tnl_key_str); >> - free(tnl_key_str); >> + } else { >> + const char *name; >> + SSET_FOR_EACH (name, ts_scope) { >> + const struct nbrec_logical_switch *ls = >> + find_ts_in_nb(ctx, CONST_CAST(char *, name)); >> + if (ls && !shash_find(nb_gc, name)) { >> + shash_add(nb_gc, name, ls); >> + } >> } >> } >> + } >> >> - struct shash_node *node; >> - SHASH_FOR_EACH (node, &nb_tres) { >> - nbrec_logical_router_delete(node->data); >> - } >> - shash_destroy(&nb_tres); >> - } >> - >> - /* Sync TR between INB and ISB. This is performed after syncing >> with AZ >> - * SB, to avoid uncommitted ISB datapath tunnel key to be synced >> back to >> - * AZ. */ >> - if (ctx->ovnisb_txn && >> - is_az_leader(ctx->ovnisb_txn)) { >> - /* Create ISB Datapath_Binding */ >> - const struct icnbrec_transit_router *tr; >> - ICNBREC_TRANSIT_ROUTER_FOR_EACH (tr, ctx->ovninb_idl) { >> - char *uuid_str = uuid_to_string(&tr->header_.uuid); >> - struct icsbrec_datapath_binding *isb_dp = >> - shash_find_and_delete(isb_tr_dps, uuid_str); >> - free(uuid_str); >> - >> - if (!isb_dp) { >> - int dp_key = allocate_dp_key(dp_tnlids, false, >> - "transit router datapath"); >> - if (!dp_key) { >> - continue; >> - } >> - >> - isb_dp = >> icsbrec_datapath_binding_insert(ctx->ovnisb_txn); >> - icsbrec_datapath_binding_set_tunnel_key(isb_dp, dp_key); >> - icsbrec_datapath_binding_set_nb_ic_uuid(isb_dp, >> - >> &tr->header_.uuid, 1); >> - icsbrec_datapath_binding_set_type(isb_dp, >> "transit-router"); >> - } >> + const struct icnbrec_transit_switch *ts; >> + ICNBREC_TRANSIT_SWITCH_FOR_EACH (ts, ctx->ovninb_idl) { >> + if (full || sset_contains(ts_scope, ts->name)) { >> + ts_sync_one(ctx, ts, isb_ts_dps, nb_gc, vxlan_mode); >> } >> + } >> >> + /* Delete extra NB Logical_Switch with other_config:interconn-ts. */ >> + if (nb_gc) { >> struct shash_node *node; >> - SHASH_FOR_EACH (node, isb_tr_dps) { >> - icsbrec_datapath_binding_delete(node->data); >> + SHASH_FOR_EACH (node, nb_gc) { >> + nbrec_logical_switch_delete(node->data); >> } >> } >> + >> + shash_destroy(&nb_ts_mirrors); >> } >> >> /* Returns true if any information in gw and chassis is different. */ >> @@ -708,7 +635,7 @@ gateway_run(struct ic_context *ctx, >> shash_destroy(&remote_gws); >> } >> >> -static const struct nbrec_logical_switch * >> +const struct nbrec_logical_switch * >> find_ts_in_nb(struct ic_context *ctx, char *ts_name) >> { >> const struct nbrec_logical_switch *key = >> diff --git a/ic/ovn-ic.h b/ic/ovn-ic.h >> index 3d2e20483..91be04db6 100644 >> --- a/ic/ovn-ic.h >> +++ b/ic/ovn-ic.h >> @@ -63,21 +63,31 @@ enum ic_port_binding_type { IC_SWITCH_PORT, >> IC_ROUTER_PORT, IC_PORT_MAX }; >> >> struct hmap; >> struct shash; >> +struct sset; >> struct icsbrec_availability_zone; >> +struct icsbrec_datapath_binding; >> + >> +enum ic_datapath_type ic_dp_get_type( >> + const struct icsbrec_datapath_binding *isb_dp); >> >> -/* Per-subsystem entry points, invoked by the incremental-processing >> engine >> - * nodes (see ic/en-*.c). Each performs a full recompute of its >> subsystem and >> - * may be invoked independently when its engine inputs change. */ >> -void enumerate_datapaths(struct ic_context *ctx, struct hmap *dp_tnlids, >> - struct shash *isb_ts_dps, struct shash >> *isb_tr_dps); >> void gateway_run(struct ic_context *ctx, >> const struct icsbrec_availability_zone *runned_az); >> void address_set_run(struct ic_context *ctx, >> const struct icsbrec_availability_zone *runned_az); >> -void ts_run(struct ic_context *ctx, struct hmap *dp_tnlids, >> - struct shash *isb_ts_dps); >> -void tr_run(struct ic_context *ctx, struct hmap *dp_tnlids, >> - struct shash *isb_tr_dps); >> + >> +/* Reconciles the AZ NB Logical_Switch mirrors of the transit switches >> named in >> + * 'ts_scope' (NULL reconciles every transit switch). 'isb_ts_dps' is >> the >> + * transit-switch IC-SB Datapath_Binding map (keyed by transit-switch >> name), >> + * read only to copy each committed tunnel key into the mirror's >> + * requested-tnl-key. IC-SB Datapath_Binding creation/keying/GC is >> owned by >> + * the en_tunnel_key node. */ >> +void ts_sync_scope(struct ic_context *ctx, struct shash *isb_ts_dps, >> + const struct sset *ts_scope); >> + >> +/* True if transit-switch datapaths must use the VXLAN tunnel-key range: >> IC-NB >> + * requests vxlan_mode and the IC-SB actually has a VXLAN encap. */ >> +bool ic_ts_compute_vxlan_mode(struct ic_context *ctx); >> + >> void port_binding_run(struct ic_context *ctx, >> const struct icsbrec_availability_zone *runned_az); >> void route_run(struct ic_context *ctx, >> @@ -85,4 +95,12 @@ void route_run(struct ic_context *ctx, >> void sync_service_monitor(struct ic_context *ctx, >> const struct icsbrec_availability_zone >> *runned_az); >> >> +/* Shared IC helpers used by more than one engine node. */ >> +uint32_t >> +allocate_dp_key(struct hmap *dp_tnlids, bool vxlan_mode, const char >> *name); >> +const struct nbrec_logical_switch * >> +find_ts_in_nb(struct ic_context *ctx, char *ts_name); >> +bool >> +is_az_leader(struct ovsdb_idl_txn *txn); >> + >> #endif /* OVN_IC_H */ >> -- >> 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 >> > -- _‘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
