Hi Paulo, I have some comments below.
On Mon, Feb 9, 2026 at 1:29 PM Paulo Guilherme Silva via dev
<[email protected]> wrote:
>
> This new engine now maintains the transit-router related data for ovn-ic
> daemon which was earlier maintained by the ic engine node invoked the
> tr_run() function. The inputs to this engine node are:
> en_enum_datapaths;
> en_icsb_datapath_binding;
> en_nb_logical_router;
> en_icnb_transit_router;
> en_icnb_transit_router_port;
>
> In order to achieve this, we refactor in the following way:
> * Introduce tr_init() which initializes this data.
> * Introduce tr_destroy() which clears this data for a new iteration.
> * Introduce tr_run() which invokes the full recompute of the engine.
>
> This engine node becomes an input to 'ic' node.
>
> Signed-off-by: Paulo Guilherme Silva <[email protected]>
> ---
> ic/automake.mk | 2 +
> ic/en-ic.c | 2 -
> ic/en-tr.c | 181 ++++++++++++++++++++++++++++++++++++++++++
> ic/en-tr.h | 23 ++++++
> ic/inc-proc-ic.c | 11 ++-
> ic/ovn-ic.c | 87 +-------------------
> ic/ovn-ic.h | 3 +-
> lib/stopwatch-names.h | 1 +
> 8 files changed, 220 insertions(+), 90 deletions(-)
> create mode 100644 ic/en-tr.c
> create mode 100644 ic/en-tr.h
>
> diff --git a/ic/automake.mk b/ic/automake.mk
> index 1fca1eb38..180fcb252 100644
> --- a/ic/automake.mk
> +++ b/ic/automake.mk
> @@ -8,6 +8,8 @@ ic_ovn_ic_SOURCES = ic/ovn-ic.c \
> ic/en-gateway.h \
> ic/en-enum-datapaths.c \
> ic/en-enum-datapaths.h \
> + ic/en-tr.c \
> + ic/en-tr.h \
> ic/en-port-binding.c \
> ic/en-port-binding.h \
> ic/en-route.c \
> diff --git a/ic/en-ic.c b/ic/en-ic.c
> index af240c20a..e9450a290 100644
> --- a/ic/en-ic.c
> +++ b/ic/en-ic.c
> @@ -56,8 +56,6 @@ ic_get_input_data(struct engine_node *node,
> EN_OVSDB_GET(engine_get_input("ICNB_ic_nb_global", node));
> input_data->icnbrec_transit_switch_table =
> EN_OVSDB_GET(engine_get_input("ICNB_transit_switch", node));
> - input_data->icnbrec_transit_router_table =
> - EN_OVSDB_GET(engine_get_input("ICNB_transit_router", node));
> input_data->icsbrec_ic_sb_global_table =
> EN_OVSDB_GET(engine_get_input("ICSB_ic_sb_global", node));
> input_data->icsbrec_availability_zone_table =
> diff --git a/ic/en-tr.c b/ic/en-tr.c
> new file mode 100644
> index 000000000..642d11b85
> --- /dev/null
> +++ b/ic/en-tr.c
> @@ -0,0 +1,181 @@
> +/*
> + * 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 <getopt.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +/* OVS includes. */
> +#include "openvswitch/vlog.h"
> +
> +/* OVN includes. */
> +#include "ovn-ic.h"
> +#include "en-tr.h"
> +#include "en-enum-datapaths.h"
> +#include "inc-proc-ic.h"
> +#include "lib/inc-proc-eng.h"
> +#include "lib/ovn-nb-idl.h"
> +#include "lib/ovn-ic-nb-idl.h"
> +#include "lib/ovn-ic-sb-idl.h"
> +#include "lib/ovn-util.h"
> +#include "lib/stopwatch-names.h"
> +#include "coverage.h"
> +#include "stopwatch.h"
> +#include "stopwatch-names.h"
> +
> +VLOG_DEFINE_THIS_MODULE(en_transit_router);
> +COVERAGE_DEFINE(tr_run);
> +
> +static void
> +tr_run(const struct engine_context *eng_ctx,
> + struct ed_type_transit_router *tr_data,
> + struct ed_type_enum_datapaths *dp_node_data,
> + const struct nbrec_logical_router_table *nbrec_lr_table,
> + const struct icnbrec_transit_router_table *icnbrec_tr_table);
> +static void tr_init(struct ed_type_transit_router *data);
> +static void tr_destroy(struct ed_type_transit_router *data);
> +
> +enum engine_node_state
> +en_tr_run(struct engine_node *node, void *data)
> +{
> + const struct engine_context *eng_ctx = engine_get_context();
> + struct ed_type_transit_router *tr_data = data;
> +
> + struct ed_type_enum_datapaths *dp_node_data =
> + engine_get_input_data("enum_datapaths", node);
> +
> + const struct nbrec_logical_router_table *nbrec_lr_table =
> + EN_OVSDB_GET(engine_get_input("NB_logical_router", node));
> + const struct icnbrec_transit_router_table *icnbrec_tr_table =
> + EN_OVSDB_GET(engine_get_input("ICNB_transit_router", node));
> +
> + COVERAGE_INC(tr_run);
> + stopwatch_start(OVN_IC_TRANSIT_ROUTER_RUN_STOPWATCH_NAME, time_usec());
> + tr_run(eng_ctx, tr_data, dp_node_data, nbrec_lr_table, icnbrec_tr_table);
> + stopwatch_stop(OVN_IC_TRANSIT_ROUTER_RUN_STOPWATCH_NAME, time_usec());
> +
> + return EN_UPDATED;
> +}
> +
> +void *
> +en_tr_init(struct engine_node *node OVS_UNUSED,
> + struct engine_arg *arg OVS_UNUSED)
> +{
> + struct ed_type_transit_router *data = xzalloc(sizeof *data);
> + tr_init(data);
> + return data;
> +}
> +
> +void
> +en_tr_cleanup(void *data)
> +{
> + tr_destroy(data);
> +}
> +
> +static void
> +tr_init(struct ed_type_transit_router *data)
> +{
> + shash_init(&data->isb_tr_dps);
data->isb_tr_dps is initialized but never used by this engine node.
Instead, the node uses the isb_tr_dps from the enum_datapaths node
during tr_run(). This same pattern is used in patch 7 by the "ts"
engine node.
> + hmap_init(&data->dp_tnlids);
> +}
> +
> +static void
> +tr_destroy(struct ed_type_transit_router *data)
> +{
> + shash_destroy(&data->isb_tr_dps);
> + ovn_destroy_tnlids(&data->dp_tnlids);
> +}
> +
> +static void
> +tr_run(const struct engine_context *eng_ctx,
> + struct ed_type_transit_router *tr_data OVS_UNUSED,
> + struct ed_type_enum_datapaths *dp_node_data,
> + const struct nbrec_logical_router_table *nbrec_lr_table,
> + const struct icnbrec_transit_router_table *icnbrec_tr_table)
> +{
> + const struct nbrec_logical_router *lr;
> + if (eng_ctx->ovnnb_idl_txn) {
> + struct shash nb_tres = SHASH_INITIALIZER(&nb_tres);
> + NBREC_LOGICAL_ROUTER_TABLE_FOR_EACH (lr, nbrec_lr_table) {
> + 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_TABLE_FOR_EACH (tr, icnbrec_tr_table) {
> + lr = shash_find_and_delete(&nb_tres, tr->name);
> + if (!lr) {
> + lr = nbrec_logical_router_insert(eng_ctx->ovnnb_idl_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);
> + struct icsbrec_datapath_binding *isb_dp = shash_find_data(
> + &dp_node_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);
> + }
> +
> + /* 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 (eng_ctx->ovnisb_idl_txn) {
> + /* Create ISB Datapath_Binding */
> + const struct icnbrec_transit_router *tr;
> + ICNBREC_TRANSIT_ROUTER_TABLE_FOR_EACH (tr, icnbrec_tr_table) {
> + char *uuid_str = uuid_to_string(&tr->header_.uuid);
> + struct icsbrec_datapath_binding *isb_dp =
> + shash_find_and_delete(&dp_node_data->isb_tr_dps, uuid_str);
Like I mentioned in patch 2, it's best if we can treat the input data
from enum_datapaths as const. The same applies to patch 7 for the ts
node.
> + free(uuid_str);
> +
> + if (!isb_dp) {
> + int dp_key = allocate_dp_key(&dp_node_data->dp_tnlids, false,
> + "transit router datapath");
> + if (!dp_key) {
> + continue;
> + }
> +
> + isb_dp = icsbrec_datapath_binding_insert(
> + eng_ctx->ovnisb_idl_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");
> + }
> + }
> +
> + struct shash_node *node;
> + SHASH_FOR_EACH (node, &dp_node_data->isb_tr_dps) {
> + icsbrec_datapath_binding_delete(node->data);
> + }
> + }
> +}
> diff --git a/ic/en-tr.h b/ic/en-tr.h
> new file mode 100644
> index 000000000..674e1c07d
> --- /dev/null
> +++ b/ic/en-tr.h
> @@ -0,0 +1,23 @@
> +#ifndef EN_IC_TR_RUN_H
> +#define EN_IC_TR_RUN_H 1
> +
> +#include <config.h>
> +
> +#include <stdbool.h>
> +#include <getopt.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +/* OVN includes. */
> +#include "lib/inc-proc-eng.h"
> +
> +struct ed_type_transit_router {
> + struct hmap dp_tnlids;
> + struct shash isb_tr_dps;
> +};
> +
> +void *en_tr_init(struct engine_node *, struct engine_arg *);
> +enum engine_node_state en_tr_run(struct engine_node *, void *data);
> +void en_tr_cleanup(void *data);
> +
> +#endif
> diff --git a/ic/inc-proc-ic.c b/ic/inc-proc-ic.c
> index 6ee9a3fc7..77bc1dd50 100644
> --- a/ic/inc-proc-ic.c
> +++ b/ic/inc-proc-ic.c
> @@ -29,6 +29,7 @@
> #include "en-ic.h"
> #include "en-gateway.h"
> #include "en-enum-datapaths.h"
> +#include "en-tr.h"
> #include "en-port-binding.h"
> #include "en-route.h"
> #include "unixctl.h"
> @@ -164,6 +165,7 @@ VLOG_DEFINE_THIS_MODULE(inc_proc_ic);
> static ENGINE_NODE(ic, SB_WRITE);
> static ENGINE_NODE(gateway, SB_WRITE);
> static ENGINE_NODE(enum_datapaths);
> +static ENGINE_NODE(tr);
> static ENGINE_NODE(port_binding, SB_WRITE);
> static ENGINE_NODE(route);
>
> @@ -181,6 +183,12 @@ inc_proc_ic_init(struct ovsdb_idl_loop *nb,
> engine_add_input(&en_enum_datapaths, &en_icnb_transit_switch, NULL);
> engine_add_input(&en_enum_datapaths, &en_icsb_datapath_binding, NULL);
>
> + engine_add_input(&en_tr, &en_enum_datapaths, NULL);
> + engine_add_input(&en_tr, &en_icsb_datapath_binding, NULL);
> + engine_add_input(&en_tr, &en_nb_logical_router, NULL);
> + engine_add_input(&en_tr, &en_icnb_transit_router, NULL);
> + engine_add_input(&en_tr, &en_icnb_transit_router_port, NULL);
> +
> engine_add_input(&en_port_binding, &en_icnb_transit_switch, NULL);
> engine_add_input(&en_port_binding, &en_icnb_transit_router, NULL);
> engine_add_input(&en_port_binding, &en_icsb_port_binding, NULL);
> @@ -199,6 +207,7 @@ inc_proc_ic_init(struct ovsdb_idl_loop *nb,
>
> engine_add_input(&en_ic, &en_gateway, NULL);
> engine_add_input(&en_ic, &en_enum_datapaths, NULL);
> + engine_add_input(&en_ic, &en_tr, NULL);
> engine_add_input(&en_ic, &en_port_binding, NULL);
> engine_add_input(&en_ic, &en_route, NULL);
>
> @@ -218,8 +227,6 @@ inc_proc_ic_init(struct ovsdb_idl_loop *nb,
>
> engine_add_input(&en_ic, &en_icnb_ic_nb_global, NULL);
> engine_add_input(&en_ic, &en_icnb_transit_switch, NULL);
> - engine_add_input(&en_ic, &en_icnb_transit_router, NULL);
> - engine_add_input(&en_ic, &en_icnb_transit_router_port, NULL);
>
> engine_add_input(&en_ic, &en_icsb_port_binding, NULL);
> engine_add_input(&en_ic, &en_icsb_ic_sb_global, NULL);
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index e5e3fd617..7a0aa567d 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -159,7 +159,7 @@ az_run(struct ovsdb_idl *ovnnb_idl,
> return NULL;
> }
>
> -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
> @@ -346,89 +346,6 @@ ts_run(struct engine_context *ctx,
> }
> }
>
> -static void
> -tr_run(struct engine_context *ctx,
> - struct ic_input *ic,
> - struct hmap *dp_tnlids,
> - struct shash *isb_tr_dps)
> -{
> - const struct nbrec_logical_router *lr;
> -
> - if (ctx->ovnnb_idl_txn) {
> - struct shash nb_tres = SHASH_INITIALIZER(&nb_tres);
> - NBREC_LOGICAL_ROUTER_TABLE_FOR_EACH (lr,
> - ic->nbrec_logical_router_table)
> {
> - 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_TABLE_FOR_EACH (tr,
> - ic->icnbrec_transit_router_table) {
> - lr = shash_find_and_delete(&nb_tres, tr->name);
> - if (!lr) {
> - lr = nbrec_logical_router_insert(ctx->ovnnb_idl_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);
> - 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);
> - }
> -
> - /* 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_idl_txn) {
> - /* Create ISB Datapath_Binding */
> - const struct icnbrec_transit_router *tr;
> - ICNBREC_TRANSIT_ROUTER_TABLE_FOR_EACH (tr,
> - ic->icnbrec_transit_router_table) {
> - 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_idl_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");
> - }
> - }
> -
> - struct shash_node *node;
> - SHASH_FOR_EACH (node, isb_tr_dps) {
> - icsbrec_datapath_binding_delete(node->data);
> - }
> - }
> -}
> -
> const struct nbrec_logical_router_port *
> get_lrp_by_lrp_name(struct ovsdb_idl_index *nbrec_lrp_by_name,
> const char *lrp_name)
> @@ -1047,7 +964,6 @@ ovn_db_run(struct ic_input *input_data,
> struct engine_context *eng_ctx)
> {
> ts_run(eng_ctx, input_data, ic_data->dp_tnlids, ic_data->isb_ts_dps);
> - tr_run(eng_ctx, input_data, ic_data->dp_tnlids, ic_data->isb_tr_dps);
> sync_service_monitor(eng_ctx, input_data);
> }
>
> @@ -1454,6 +1370,7 @@ main(int argc, char *argv[])
> stopwatch_create(OVN_IC_PORT_BINDING_RUN_STOPWATCH_NAME, SW_MS);
> stopwatch_create(OVN_IC_ROUTE_RUN_STOPWATCH_NAME, SW_MS);
> stopwatch_create(OVN_IC_GATEWAY_RUN_STOPWATCH_NAME, SW_MS);
> + stopwatch_create(OVN_IC_TRANSIT_ROUTER_RUN_STOPWATCH_NAME, SW_MS);
>
> /* Initialize incremental processing engine for ovn-northd */
> inc_proc_ic_init(&ovnnb_idl_loop, &ovnsb_idl_loop,
> diff --git a/ic/ovn-ic.h b/ic/ovn-ic.h
> index e6918c8d2..225dc73f5 100644
> --- a/ic/ovn-ic.h
> +++ b/ic/ovn-ic.h
> @@ -30,7 +30,6 @@ struct ic_input {
> /* InterconnectNorthbound table references */
> const struct icnbrec_transit_switch_table *icnbrec_transit_switch_table;
> const struct icnbrec_ic_nb_global_table *icnbrec_ic_nb_global_table;
> - const struct icnbrec_transit_router_table *icnbrec_transit_router_table;
>
> /* InterconnectSouthbound table references */
> const struct icsbrec_encap_table *icsbrec_encap_table;
> @@ -73,6 +72,8 @@ struct icsbrec_port_binding;
> enum ic_datapath_type { IC_SWITCH, IC_ROUTER, IC_DATAPATH_MAX };
> enum ic_port_binding_type { IC_SWITCH_PORT, IC_ROUTER_PORT, IC_PORT_MAX };
>
> +uint32_t
> +allocate_dp_key(struct hmap *dp_tnlids, bool vxlan_mode, const char *name);
> const struct nbrec_logical_router_port *
> get_lrp_by_lrp_name(struct ovsdb_idl_index *nbrec_lrp_by_name,
> const char *lrp_name);
> diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
> index e62434aad..869384271 100644
> --- a/lib/stopwatch-names.h
> +++ b/lib/stopwatch-names.h
> @@ -43,6 +43,7 @@
> #define IC_OVN_DB_RUN_STOPWATCH_NAME "ovn_db_run"
> #define OVN_IC_GATEWAY_RUN_STOPWATCH_NAME "gateway_run"
> #define OVN_IC_ENUM_DATAPATHS_RUN_STOPWATCH_NAME "enum_datapaths_run"
> +#define OVN_IC_TRANSIT_ROUTER_RUN_STOPWATCH_NAME "transit_router_run"
> #define OVN_IC_PORT_BINDING_RUN_STOPWATCH_NAME "port_binding_run"
> #define OVN_IC_ROUTE_RUN_STOPWATCH_NAME "route_run"
>
> --
> 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
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
