diff mbox series

[ovs-dev,v1,2/9] ovn-ic: Add a new engine-node 'enum-datapath'.

Message ID 20260209182814.842-3-guilherme.paulo@luizalabs.com
State Changes Requested
Headers show
Series Create multiple engines nodes for ovn-ic. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success

Commit Message

Paulo Guilherme Silva Feb. 9, 2026, 6:28 p.m. UTC 
This new engine now maintains the enum_datapath related data for ovn-ic
daemon which was earlier maintained by the ic engine node invoked the
enumerate_datapaths() function. The inputs to this engine node are
   en_icnb_transit_switch;
   en_icsb_datapath_binding';
In order to achieve this, we refactor in the following way:
* Introduce enum_datapaths_init() which initializes this data.
* Introduce enum_datapaths_destroy() which clears this data for a
  new iteration.
* Introduce enum_datapaths_run() which invokes the full recompute
  of the engine.

This engine node becomes an input to 'ic' node.

Signed-off-by: Paulo Guilherme Silva <guilherme.paulo@luizalabs.com>
---
 ic/automake.mk         |   2 +
 ic/en-enum-datapaths.c | 141 +++++++++++++++++++++++++++++++++++++++++
 ic/en-enum-datapaths.h |  30 +++++++++
 ic/en-ic.c             |  29 +++++----
 ic/inc-proc-ic.c       |   6 ++
 ic/ovn-ic.c            |  82 ++++++++++++------------
 ic/ovn-ic.h            |   8 +--
 lib/stopwatch-names.h  |   1 +
 8 files changed, 244 insertions(+), 55 deletions(-)
 create mode 100644 ic/en-enum-datapaths.c
 create mode 100644 ic/en-enum-datapaths.h

--
2.34.1

Comments

Mark Michelson Feb. 17, 2026, 11 p.m. UTC | #1 
Thanks Paulo, I have comments inline below.

On Mon, Feb 9, 2026 at 1:29 PM Paulo Guilherme Silva via dev
<ovs-dev@openvswitch.org> wrote:
>
> This new engine now maintains the enum_datapath related data for ovn-ic
> daemon which was earlier maintained by the ic engine node invoked the
> enumerate_datapaths() function. The inputs to this engine node are
>    en_icnb_transit_switch;
>    en_icsb_datapath_binding';
> In order to achieve this, we refactor in the following way:
> * Introduce enum_datapaths_init() which initializes this data.
> * Introduce enum_datapaths_destroy() which clears this data for a
>   new iteration.
> * Introduce enum_datapaths_run() which invokes the full recompute
>   of the engine.
>
> This engine node becomes an input to 'ic' node.
>
> Signed-off-by: Paulo Guilherme Silva <guilherme.paulo@luizalabs.com>
> ---
>  ic/automake.mk         |   2 +
>  ic/en-enum-datapaths.c | 141 +++++++++++++++++++++++++++++++++++++++++
>  ic/en-enum-datapaths.h |  30 +++++++++
>  ic/en-ic.c             |  29 +++++----
>  ic/inc-proc-ic.c       |   6 ++
>  ic/ovn-ic.c            |  82 ++++++++++++------------
>  ic/ovn-ic.h            |   8 +--
>  lib/stopwatch-names.h  |   1 +
>  8 files changed, 244 insertions(+), 55 deletions(-)
>  create mode 100644 ic/en-enum-datapaths.c
>  create mode 100644 ic/en-enum-datapaths.h
>
> diff --git a/ic/automake.mk b/ic/automake.mk
> index a69b1030d..2766483b7 100644
> --- a/ic/automake.mk
> +++ b/ic/automake.mk
> @@ -4,6 +4,8 @@ ic_ovn_ic_SOURCES = ic/ovn-ic.c \
>         ic/ovn-ic.h \
>         ic/en-ic.c \
>         ic/en-ic.h \
> +       ic/en-enum-datapaths.c \
> +       ic/en-enum-datapaths.h \
>         ic/inc-proc-ic.c \
>         ic/inc-proc-ic.h
>  ic_ovn_ic_LDADD = \
> diff --git a/ic/en-enum-datapaths.c b/ic/en-enum-datapaths.c
> new file mode 100644
> index 000000000..7515c0b85
> --- /dev/null
> +++ b/ic/en-enum-datapaths.c
> @@ -0,0 +1,141 @@
> +/*
> + * 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"
> +#include "openvswitch/shash.h"
> +#include "openvswitch/hmap.h"
> +
> +/* OVN includes. */
> +#include "ovn-ic.h"
> +#include "en-enum-datapaths.h"
> +#include "inc-proc-ic.h"
> +#include "lib/inc-proc-eng.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_enum_datapaths);
> +COVERAGE_DEFINE(enum_datapaths_run);
> +
> +static void
> +enum_datapath_run(const struct icsbrec_datapath_binding_table *dp_table,
> +                  struct ed_type_enum_datapaths *dp_data);
> +static enum ic_datapath_type
> +    ic_dp_get_type(const struct icsbrec_datapath_binding *isb_dp);
> +static void enum_datapaths_init(struct ed_type_enum_datapaths *data);
> +static void enum_datapaths_destroy(struct ed_type_enum_datapaths *data);
> +static void enum_datapaths_clear(struct ed_type_enum_datapaths *data);
> +
> +void *
> +en_enum_datapaths_init(struct engine_node *node OVS_UNUSED,
> +                       struct engine_arg *arg OVS_UNUSED)
> +{
> +    struct ed_type_enum_datapaths *data = xzalloc(sizeof *data);
> +    enum_datapaths_init(data);
> +    return data;
> +}
> +
> +void
> +en_enum_datapaths_cleanup(void *data)
> +{
> +    enum_datapaths_destroy(data);
> +}
> +
> +enum engine_node_state
> +en_enum_datapaths_run(struct engine_node *node, void *data)
> +{
> +    struct ed_type_enum_datapaths *dp_data = data;
> +
> +    enum_datapaths_clear(dp_data);
> +
> +    const struct icsbrec_datapath_binding_table *dp_table =
> +        EN_OVSDB_GET(engine_get_input("ICSB_datapath_binding", node));
> +
> +    COVERAGE_INC(enum_datapaths_run);
> +    stopwatch_start(OVN_IC_ENUM_DATAPATHS_RUN_STOPWATCH_NAME, time_msec());
> +    enum_datapath_run(dp_table, dp_data);
> +    stopwatch_stop(OVN_IC_ENUM_DATAPATHS_RUN_STOPWATCH_NAME, time_msec());
> +
> +    return EN_UPDATED;
> +}
> +
> +static void
> +enum_datapath_run(const struct icsbrec_datapath_binding_table *dp_table,
> +                  struct ed_type_enum_datapaths *dp_data)
> +{
> +    const struct icsbrec_datapath_binding *isb_dp;
> +    ICSBREC_DATAPATH_BINDING_TABLE_FOR_EACH (isb_dp, dp_table) {
> +        /* 1. Adiciona Tunnel ID */
> +        ovn_add_tnlid(&dp_data->dp_tnlids, isb_dp->tunnel_key);
> +
> +        /* 2. Classifica o Datapath */

It appears some Portuguese has entered the code here :)

> +        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(&dp_data->isb_tr_dps, uuid_str, (void *) isb_dp);
> +            free(uuid_str);
> +        } else {
> +            shash_add(&dp_data->isb_ts_dps, isb_dp->transit_switch,
> +                      (void *) isb_dp);
> +        }
> +    }
> +}
> +
> +static 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")) {
> +        return IC_ROUTER;
> +    }
> +
> +    return IC_SWITCH;
> +}
> +
> +static void
> +enum_datapaths_init(struct ed_type_enum_datapaths *data)
> +{
> +    hmap_init(&data->dp_tnlids);
> +    shash_init(&data->isb_ts_dps);
> +    shash_init(&data->isb_tr_dps);
> +}
> +
> +static void
> +enum_datapaths_destroy(struct ed_type_enum_datapaths *data)
> +{
> +    enum_datapaths_clear(data);

There is no need to call enum_datapaths_clear() here. There is a
similar pattern employed in patches 3, 4, and 5 as well.

> +    ovn_destroy_tnlids(&data->dp_tnlids);
> +
> +    shash_destroy(&data->isb_ts_dps);
> +    shash_destroy(&data->isb_tr_dps);
> +}
> +
> +static void
> +enum_datapaths_clear(struct ed_type_enum_datapaths *data)
> +{
> +    ovn_destroy_tnlids(&data->dp_tnlids);
> +    hmap_init(&data->dp_tnlids);
> +
> +    shash_clear(&data->isb_ts_dps);
> +    shash_clear(&data->isb_tr_dps);
> +}
> diff --git a/ic/en-enum-datapaths.h b/ic/en-enum-datapaths.h
> new file mode 100644
> index 000000000..0aff9fc89
> --- /dev/null
> +++ b/ic/en-enum-datapaths.h
> @@ -0,0 +1,30 @@
> +#ifndef EN_IC_ENUM_DATAPATHS_H
> +#define EN_IC_ENUM_DATAPATHS_H 1
> +
> +#include <config.h>
> +
> +#include <stdbool.h>
> +#include <getopt.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +/* OVS includes. */
> +#include "lib/hmapx.h"
> +#include "openvswitch/hmap.h"
> +#include "openvswitch/shash.h"
> +
> +/* OVN includes. */
> +#include "lib/inc-proc-eng.h"
> +
> +/* struct which maintains the data of the engine node enumerate datapaths. */
> +struct ed_type_enum_datapaths {
> +    struct hmap dp_tnlids;
> +    struct shash isb_ts_dps;
> +    struct shash isb_tr_dps;
> +};
> +
> +void *en_enum_datapaths_init(struct engine_node *, struct engine_arg *);
> +enum engine_node_state en_enum_datapaths_run(struct engine_node *, void *data);
> +void en_enum_datapaths_cleanup(void *data);
> +
> +#endif
> \ No newline at end of file
> diff --git a/ic/en-ic.c b/ic/en-ic.c
> index ce7d5de76..16b0e12bd 100644
> --- a/ic/en-ic.c
> +++ b/ic/en-ic.c
> @@ -24,6 +24,8 @@
>  /* OVN includes. */
>  #include "ovn-ic.h"
>  #include "en-ic.h"
> +#include "en-enum-datapaths.h"
> +#include "lib/ovn-ic-sb-idl.h"
>  #include "lib/inc-proc-eng.h"
>  #include "lib/ovn-util.h"
>  #include "lib/stopwatch-names.h"
> @@ -152,18 +154,26 @@ enum engine_node_state
>  en_ic_run(struct engine_node *node, void *data)
>  {
>      const struct engine_context *eng_ctx = engine_get_context();
> -
> +    struct ic_data *ic_data = data;
>      struct ic_input input_data;
>
> -    ic_destroy(data);
> -    ic_init(data);
> +    struct ed_type_enum_datapaths *dp_node_data =
> +        engine_get_input_data("enum_datapaths", node);
> +
> +    if (!dp_node_data) {
> +        return EN_UNCHANGED;
> +    }

The NULL check above is unnecessary. The enum_datapaths node always
returns EN_UPDATED as its status, so the dp_node_data will always be
non-NULL.

> +
> +    ic_data->dp_tnlids = &dp_node_data->dp_tnlids;
> +    ic_data->isb_ts_dps = &dp_node_data->isb_ts_dps;
> +    ic_data->isb_tr_dps = &dp_node_data->isb_tr_dps;
>
>      ic_get_input_data(node, &input_data);
>      input_data.runned_az = eng_ctx->client_ctx;
>
>      COVERAGE_INC(ic_run);
>      stopwatch_start(IC_OVN_DB_RUN_STOPWATCH_NAME, time_msec());
> -    ovn_db_run(&input_data, data, (struct engine_context *) eng_ctx);
> +    ovn_db_run(&input_data, ic_data, (struct engine_context *) eng_ctx);
>      stopwatch_stop(IC_OVN_DB_RUN_STOPWATCH_NAME, time_msec());
>      return EN_UPDATED;
>  }
> @@ -186,17 +196,14 @@ en_ic_cleanup(void *data)
>  }
>
>  void
> -ic_destroy(struct ic_data *data)
> +ic_destroy(struct ic_data *data OVS_UNUSED)
>  {
> -    ovn_destroy_tnlids(&data->dp_tnlids);
> -    shash_destroy(&data->isb_ts_dps);
> -    shash_destroy(&data->isb_tr_dps);
>  }
>
>  void
>  ic_init(struct ic_data *data)
>  {
> -    hmap_init(&data->dp_tnlids);
> -    shash_init(&data->isb_ts_dps);
> -    shash_init(&data->isb_tr_dps);
> +    data->dp_tnlids = NULL;
> +    data->isb_ts_dps = NULL;
> +    data->isb_tr_dps = NULL;
>  }
> diff --git a/ic/inc-proc-ic.c b/ic/inc-proc-ic.c
> index 0160c3a3e..d7d247e7a 100644
> --- a/ic/inc-proc-ic.c
> +++ b/ic/inc-proc-ic.c
> @@ -27,6 +27,7 @@
>  #include "openvswitch/vlog.h"
>  #include "inc-proc-ic.h"
>  #include "en-ic.h"
> +#include "en-enum-datapaths.h"
>  #include "unixctl.h"
>  #include "util.h"
>
> @@ -158,6 +159,7 @@ 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(ic, SB_WRITE);
> +static ENGINE_NODE(enum_datapaths);
>
>  void
>  inc_proc_ic_init(struct ovsdb_idl_loop *nb,
> @@ -167,6 +169,10 @@ inc_proc_ic_init(struct ovsdb_idl_loop *nb,
>  {
>      /* Define relationships between nodes where first argument is dependent
>       * on the second argument */
> +    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_ic, &en_enum_datapaths, NULL);
>      engine_add_input(&en_ic, &en_nb_nb_global, NULL);
>      engine_add_input(&en_ic, &en_nb_logical_router_static_route, NULL);
>      engine_add_input(&en_ic, &en_nb_logical_router, NULL);
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index 8327054de..724201616 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -172,16 +172,6 @@ allocate_dp_key(struct hmap *dp_tnlids, bool vxlan_mode, const char *name)
>              &hint);
>  }
>
> -static 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")) {
> -        return IC_ROUTER;
> -    }
> -
> -    return IC_SWITCH;
> -}
> -
>  static enum ic_port_binding_type
>  ic_pb_get_type(const struct icsbrec_port_binding *isb_pb)
>  {
> @@ -192,28 +182,6 @@ ic_pb_get_type(const struct icsbrec_port_binding *isb_pb)
>      return IC_SWITCH_PORT;
>  }
>
> -static void
> -enumerate_datapaths(struct ic_input *ic,
> -                    struct hmap *dp_tnlids,
> -                    struct shash *isb_ts_dps,
> -                    struct shash *isb_tr_dps)
> -{
> -    const struct icsbrec_datapath_binding *isb_dp;
> -    ICSBREC_DATAPATH_BINDING_TABLE_FOR_EACH (isb_dp,
> -        ic->icsbrec_datapath_binding_table) {
> -        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);
> -        }
> -    }
> -}
> -
>  static void
>  ts_run(struct engine_context *ctx,
>         struct ic_input *ic,
> @@ -275,7 +243,17 @@ ts_run(struct engine_context *ctx,
>
>              const struct icsbrec_datapath_binding *isb_dp;
>              isb_dp = shash_find_data(isb_ts_dps, ts->name);
> -            if (isb_dp) {
> +            if (!isb_dp) {
> +                const struct icsbrec_datapath_binding *raw;
> +                ICSBREC_DATAPATH_BINDING_TABLE_FOR_EACH (raw,
> +                    ic->icsbrec_datapath_binding_table) {
> +                    if (raw->transit_switch && !strcmp(raw->transit_switch,
> +                                                       ts->name)) {
> +                        isb_dp = raw;
> +                        break;
> +                    }
> +                }

I'm having trouble understanding the need for this addition.
enum_datapaths should have filled in isb_ts_dps with all of the
transit switches from the datapath binding table, so I don't
understand how this loop helps.

If this loop does find a transit switch, then the else block below
will not be run. Therefore, the NB DB will not have the requested
tunnel key updated based on the transit switch config.

> +            } else {
>                  int64_t nb_tnl_key = smap_get_int(&ls->other_config,
>                                                    "requested-tnl-key",
>                                                    0);
> @@ -309,6 +287,24 @@ ts_run(struct engine_context *ctx,
>              ic->icnbrec_transit_switch_table) {
>              const struct icsbrec_datapath_binding *isb_dp =
>                  shash_find_and_delete(isb_ts_dps, ts->name);
> +
> +            if (!isb_dp) {
> +                const struct icsbrec_datapath_binding *raw_isb;
> +                ICSBREC_DATAPATH_BINDING_TABLE_FOR_EACH (raw_isb,
> +                    ic->icsbrec_datapath_binding_table) {
> +                    if (raw_isb->n_nb_ic_uuid > 0 &&
> +                        uuid_equals(&raw_isb->nb_ic_uuid[0],
> +                                    &ts->header_.uuid)) {
> +                        isb_dp = raw_isb;
> +                        if (isb_dp->transit_switch) {
> +                            shash_find_and_delete(isb_ts_dps,
> +                                                  isb_dp->transit_switch);
> +                        }
> +                        break;
> +                    }
> +                }
> +            }

I have the same question here: why would this loop be necessary if the
enum_datapaths node has populated the isb_ts_dps with all transit
switches?

> +
>              if (!isb_dp) {
>                  /* Allocate tunnel key */
>                  int64_t dp_key = allocate_dp_key(dp_tnlids, vxlan_mode,
> @@ -320,6 +316,9 @@ ts_run(struct engine_context *ctx,
>                  isb_dp = icsbrec_datapath_binding_insert(ctx->ovnisb_idl_txn);
>                  icsbrec_datapath_binding_set_transit_switch(isb_dp, ts->name);
>                  icsbrec_datapath_binding_set_tunnel_key(isb_dp, dp_key);
> +                icsbrec_datapath_binding_set_nb_ic_uuid(isb_dp,
> +                                                        &ts->header_.uuid, 1);
> +                icsbrec_datapath_binding_set_type(isb_dp, "transit-switch");
>              } else if (dp_key_refresh) {
>                  /* Refresh tunnel key since encap mode has changed. */
>                  int64_t dp_key = allocate_dp_key(dp_tnlids, vxlan_mode,
> @@ -339,9 +338,13 @@ ts_run(struct engine_context *ctx,
>              }
>          }
>
> -        struct shash_node *node;
> -        SHASH_FOR_EACH (node, isb_ts_dps) {
> -            icsbrec_datapath_binding_delete(node->data);
> +        struct shash_node *node, *next;
> +        SHASH_FOR_EACH_SAFE (node, next, isb_ts_dps) {
> +            struct icsbrec_datapath_binding *isb_dp_to_del = node->data;
> +            if (isb_dp_to_del->n_nb_ic_uuid > 0) {
> +                icsbrec_datapath_binding_delete(isb_dp_to_del);
> +            }
> +            shash_delete(isb_ts_dps, node);
>          }
>      }
>  }
> @@ -3114,10 +3117,8 @@ ovn_db_run(struct ic_input *input_data,
>             struct engine_context *eng_ctx)
>  {
>      gateway_run(eng_ctx, input_data);
> -    enumerate_datapaths(input_data, &ic_data->dp_tnlids,
> -                        &ic_data->isb_ts_dps, &ic_data->isb_tr_dps);
> -    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);
> +    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);
>      port_binding_run(eng_ctx, input_data);
>      route_run(eng_ctx, input_data);
>      sync_service_monitor(eng_ctx, input_data);
> @@ -3522,6 +3523,7 @@ main(int argc, char *argv[])
>                               ovn_conn_show, ovnisb_idl_loop.idl);
>
>      stopwatch_create(IC_OVN_DB_RUN_STOPWATCH_NAME, SW_MS);
> +    stopwatch_create(OVN_IC_ENUM_DATAPATHS_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 2c2efc046..9b0009274 100644
> --- a/ic/ovn-ic.h
> +++ b/ic/ovn-ic.h
> @@ -66,10 +66,10 @@ struct ic_input {
>  };
>
>  struct ic_data {
> -    /* Global state for 'en-ic'. */
> -    struct hmap dp_tnlids;
> -    struct shash isb_ts_dps;
> -    struct shash isb_tr_dps;
> +    /* Global state for 'en-enum-datapaths'. */
> +    struct hmap *dp_tnlids;
> +    struct shash *isb_ts_dps;
> +    struct shash *isb_tr_dps;

If at all possible, these three fields should be const. These fields
are created and managed by the enum_datapaths engine node, so any
other engine node should treat this data as read-only.

In this particular case, the ts_run() and tr_run() functions are
specifically not treating the isb_ts_dps and isb_tr_dps as const.
Those functions will remove items from the shash. If this can be
avoided, that would be for the best.


>  };
>  struct ic_state {
>      bool had_lock;
> diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
> index e933213d8..21572f023 100644
> --- a/lib/stopwatch-names.h
> +++ b/lib/stopwatch-names.h
> @@ -41,5 +41,6 @@
>  #define GROUP_ECMP_ROUTE_RUN_STOPWATCH_NAME "group_ecmp_route"
>
>  #define IC_OVN_DB_RUN_STOPWATCH_NAME "ovn_db_run"
> +#define OVN_IC_ENUM_DATAPATHS_RUN_STOPWATCH_NAME "enum_datapaths_run"
>
>  #endif
> --
> 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
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/ic/automake.mk b/ic/automake.mk
index a69b1030d..2766483b7 100644
--- a/ic/automake.mk
+++ b/ic/automake.mk
@@ -4,6 +4,8 @@  ic_ovn_ic_SOURCES = ic/ovn-ic.c \
 	ic/ovn-ic.h \
 	ic/en-ic.c \
 	ic/en-ic.h \
+	ic/en-enum-datapaths.c \
+	ic/en-enum-datapaths.h \
 	ic/inc-proc-ic.c \
 	ic/inc-proc-ic.h
 ic_ovn_ic_LDADD = \
diff --git a/ic/en-enum-datapaths.c b/ic/en-enum-datapaths.c
new file mode 100644
index 000000000..7515c0b85
--- /dev/null
+++ b/ic/en-enum-datapaths.c
@@ -0,0 +1,141 @@ 
+/*
+ * 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"
+#include "openvswitch/shash.h"
+#include "openvswitch/hmap.h"
+
+/* OVN includes. */
+#include "ovn-ic.h"
+#include "en-enum-datapaths.h"
+#include "inc-proc-ic.h"
+#include "lib/inc-proc-eng.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_enum_datapaths);
+COVERAGE_DEFINE(enum_datapaths_run);
+
+static void
+enum_datapath_run(const struct icsbrec_datapath_binding_table *dp_table,
+                  struct ed_type_enum_datapaths *dp_data);
+static enum ic_datapath_type
+    ic_dp_get_type(const struct icsbrec_datapath_binding *isb_dp);
+static void enum_datapaths_init(struct ed_type_enum_datapaths *data);
+static void enum_datapaths_destroy(struct ed_type_enum_datapaths *data);
+static void enum_datapaths_clear(struct ed_type_enum_datapaths *data);
+
+void *
+en_enum_datapaths_init(struct engine_node *node OVS_UNUSED,
+                       struct engine_arg *arg OVS_UNUSED)
+{
+    struct ed_type_enum_datapaths *data = xzalloc(sizeof *data);
+    enum_datapaths_init(data);
+    return data;
+}
+
+void
+en_enum_datapaths_cleanup(void *data)
+{
+    enum_datapaths_destroy(data);
+}
+
+enum engine_node_state
+en_enum_datapaths_run(struct engine_node *node, void *data)
+{
+    struct ed_type_enum_datapaths *dp_data = data;
+
+    enum_datapaths_clear(dp_data);
+
+    const struct icsbrec_datapath_binding_table *dp_table =
+        EN_OVSDB_GET(engine_get_input("ICSB_datapath_binding", node));
+
+    COVERAGE_INC(enum_datapaths_run);
+    stopwatch_start(OVN_IC_ENUM_DATAPATHS_RUN_STOPWATCH_NAME, time_msec());
+    enum_datapath_run(dp_table, dp_data);
+    stopwatch_stop(OVN_IC_ENUM_DATAPATHS_RUN_STOPWATCH_NAME, time_msec());
+
+    return EN_UPDATED;
+}
+
+static void
+enum_datapath_run(const struct icsbrec_datapath_binding_table *dp_table,
+                  struct ed_type_enum_datapaths *dp_data)
+{
+    const struct icsbrec_datapath_binding *isb_dp;
+    ICSBREC_DATAPATH_BINDING_TABLE_FOR_EACH (isb_dp, dp_table) {
+        /* 1. Adiciona Tunnel ID */
+        ovn_add_tnlid(&dp_data->dp_tnlids, isb_dp->tunnel_key);
+
+        /* 2. Classifica o Datapath */
+        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(&dp_data->isb_tr_dps, uuid_str, (void *) isb_dp);
+            free(uuid_str);
+        } else {
+            shash_add(&dp_data->isb_ts_dps, isb_dp->transit_switch,
+                      (void *) isb_dp);
+        }
+    }
+}
+
+static 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")) {
+        return IC_ROUTER;
+    }
+
+    return IC_SWITCH;
+}
+
+static void
+enum_datapaths_init(struct ed_type_enum_datapaths *data)
+{
+    hmap_init(&data->dp_tnlids);
+    shash_init(&data->isb_ts_dps);
+    shash_init(&data->isb_tr_dps);
+}
+
+static void
+enum_datapaths_destroy(struct ed_type_enum_datapaths *data)
+{
+    enum_datapaths_clear(data);
+    ovn_destroy_tnlids(&data->dp_tnlids);
+
+    shash_destroy(&data->isb_ts_dps);
+    shash_destroy(&data->isb_tr_dps);
+}
+
+static void
+enum_datapaths_clear(struct ed_type_enum_datapaths *data)
+{
+    ovn_destroy_tnlids(&data->dp_tnlids);
+    hmap_init(&data->dp_tnlids);
+
+    shash_clear(&data->isb_ts_dps);
+    shash_clear(&data->isb_tr_dps);
+}
diff --git a/ic/en-enum-datapaths.h b/ic/en-enum-datapaths.h
new file mode 100644
index 000000000..0aff9fc89
--- /dev/null
+++ b/ic/en-enum-datapaths.h
@@ -0,0 +1,30 @@ 
+#ifndef EN_IC_ENUM_DATAPATHS_H
+#define EN_IC_ENUM_DATAPATHS_H 1
+
+#include <config.h>
+
+#include <stdbool.h>
+#include <getopt.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+/* OVS includes. */
+#include "lib/hmapx.h"
+#include "openvswitch/hmap.h"
+#include "openvswitch/shash.h"
+
+/* OVN includes. */
+#include "lib/inc-proc-eng.h"
+
+/* struct which maintains the data of the engine node enumerate datapaths. */
+struct ed_type_enum_datapaths {
+    struct hmap dp_tnlids;
+    struct shash isb_ts_dps;
+    struct shash isb_tr_dps;
+};
+
+void *en_enum_datapaths_init(struct engine_node *, struct engine_arg *);
+enum engine_node_state en_enum_datapaths_run(struct engine_node *, void *data);
+void en_enum_datapaths_cleanup(void *data);
+
+#endif
\ No newline at end of file
diff --git a/ic/en-ic.c b/ic/en-ic.c
index ce7d5de76..16b0e12bd 100644
--- a/ic/en-ic.c
+++ b/ic/en-ic.c
@@ -24,6 +24,8 @@ 
 /* OVN includes. */
 #include "ovn-ic.h"
 #include "en-ic.h"
+#include "en-enum-datapaths.h"
+#include "lib/ovn-ic-sb-idl.h"
 #include "lib/inc-proc-eng.h"
 #include "lib/ovn-util.h"
 #include "lib/stopwatch-names.h"
@@ -152,18 +154,26 @@  enum engine_node_state
 en_ic_run(struct engine_node *node, void *data)
 {
     const struct engine_context *eng_ctx = engine_get_context();
-
+    struct ic_data *ic_data = data;
     struct ic_input input_data;

-    ic_destroy(data);
-    ic_init(data);
+    struct ed_type_enum_datapaths *dp_node_data =
+        engine_get_input_data("enum_datapaths", node);
+
+    if (!dp_node_data) {
+        return EN_UNCHANGED;
+    }
+
+    ic_data->dp_tnlids = &dp_node_data->dp_tnlids;
+    ic_data->isb_ts_dps = &dp_node_data->isb_ts_dps;
+    ic_data->isb_tr_dps = &dp_node_data->isb_tr_dps;

     ic_get_input_data(node, &input_data);
     input_data.runned_az = eng_ctx->client_ctx;

     COVERAGE_INC(ic_run);
     stopwatch_start(IC_OVN_DB_RUN_STOPWATCH_NAME, time_msec());
-    ovn_db_run(&input_data, data, (struct engine_context *) eng_ctx);
+    ovn_db_run(&input_data, ic_data, (struct engine_context *) eng_ctx);
     stopwatch_stop(IC_OVN_DB_RUN_STOPWATCH_NAME, time_msec());
     return EN_UPDATED;
 }
@@ -186,17 +196,14 @@  en_ic_cleanup(void *data)
 }

 void
-ic_destroy(struct ic_data *data)
+ic_destroy(struct ic_data *data OVS_UNUSED)
 {
-    ovn_destroy_tnlids(&data->dp_tnlids);
-    shash_destroy(&data->isb_ts_dps);
-    shash_destroy(&data->isb_tr_dps);
 }

 void
 ic_init(struct ic_data *data)
 {
-    hmap_init(&data->dp_tnlids);
-    shash_init(&data->isb_ts_dps);
-    shash_init(&data->isb_tr_dps);
+    data->dp_tnlids = NULL;
+    data->isb_ts_dps = NULL;
+    data->isb_tr_dps = NULL;
 }
diff --git a/ic/inc-proc-ic.c b/ic/inc-proc-ic.c
index 0160c3a3e..d7d247e7a 100644
--- a/ic/inc-proc-ic.c
+++ b/ic/inc-proc-ic.c
@@ -27,6 +27,7 @@ 
 #include "openvswitch/vlog.h"
 #include "inc-proc-ic.h"
 #include "en-ic.h"
+#include "en-enum-datapaths.h"
 #include "unixctl.h"
 #include "util.h"

@@ -158,6 +159,7 @@  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(ic, SB_WRITE);
+static ENGINE_NODE(enum_datapaths);

 void
 inc_proc_ic_init(struct ovsdb_idl_loop *nb,
@@ -167,6 +169,10 @@  inc_proc_ic_init(struct ovsdb_idl_loop *nb,
 {
     /* Define relationships between nodes where first argument is dependent
      * on the second argument */
+    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_ic, &en_enum_datapaths, NULL);
     engine_add_input(&en_ic, &en_nb_nb_global, NULL);
     engine_add_input(&en_ic, &en_nb_logical_router_static_route, NULL);
     engine_add_input(&en_ic, &en_nb_logical_router, NULL);
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index 8327054de..724201616 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -172,16 +172,6 @@  allocate_dp_key(struct hmap *dp_tnlids, bool vxlan_mode, const char *name)
             &hint);
 }

-static 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")) {
-        return IC_ROUTER;
-    }
-
-    return IC_SWITCH;
-}
-
 static enum ic_port_binding_type
 ic_pb_get_type(const struct icsbrec_port_binding *isb_pb)
 {
@@ -192,28 +182,6 @@  ic_pb_get_type(const struct icsbrec_port_binding *isb_pb)
     return IC_SWITCH_PORT;
 }

-static void
-enumerate_datapaths(struct ic_input *ic,
-                    struct hmap *dp_tnlids,
-                    struct shash *isb_ts_dps,
-                    struct shash *isb_tr_dps)
-{
-    const struct icsbrec_datapath_binding *isb_dp;
-    ICSBREC_DATAPATH_BINDING_TABLE_FOR_EACH (isb_dp,
-        ic->icsbrec_datapath_binding_table) {
-        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);
-        }
-    }
-}
-
 static void
 ts_run(struct engine_context *ctx,
        struct ic_input *ic,
@@ -275,7 +243,17 @@  ts_run(struct engine_context *ctx,

             const struct icsbrec_datapath_binding *isb_dp;
             isb_dp = shash_find_data(isb_ts_dps, ts->name);
-            if (isb_dp) {
+            if (!isb_dp) {
+                const struct icsbrec_datapath_binding *raw;
+                ICSBREC_DATAPATH_BINDING_TABLE_FOR_EACH (raw,
+                    ic->icsbrec_datapath_binding_table) {
+                    if (raw->transit_switch && !strcmp(raw->transit_switch,
+                                                       ts->name)) {
+                        isb_dp = raw;
+                        break;
+                    }
+                }
+            } else {
                 int64_t nb_tnl_key = smap_get_int(&ls->other_config,
                                                   "requested-tnl-key",
                                                   0);
@@ -309,6 +287,24 @@  ts_run(struct engine_context *ctx,
             ic->icnbrec_transit_switch_table) {
             const struct icsbrec_datapath_binding *isb_dp =
                 shash_find_and_delete(isb_ts_dps, ts->name);
+
+            if (!isb_dp) {
+                const struct icsbrec_datapath_binding *raw_isb;
+                ICSBREC_DATAPATH_BINDING_TABLE_FOR_EACH (raw_isb,
+                    ic->icsbrec_datapath_binding_table) {
+                    if (raw_isb->n_nb_ic_uuid > 0 &&
+                        uuid_equals(&raw_isb->nb_ic_uuid[0],
+                                    &ts->header_.uuid)) {
+                        isb_dp = raw_isb;
+                        if (isb_dp->transit_switch) {
+                            shash_find_and_delete(isb_ts_dps,
+                                                  isb_dp->transit_switch);
+                        }
+                        break;
+                    }
+                }
+            }
+
             if (!isb_dp) {
                 /* Allocate tunnel key */
                 int64_t dp_key = allocate_dp_key(dp_tnlids, vxlan_mode,
@@ -320,6 +316,9 @@  ts_run(struct engine_context *ctx,
                 isb_dp = icsbrec_datapath_binding_insert(ctx->ovnisb_idl_txn);
                 icsbrec_datapath_binding_set_transit_switch(isb_dp, ts->name);
                 icsbrec_datapath_binding_set_tunnel_key(isb_dp, dp_key);
+                icsbrec_datapath_binding_set_nb_ic_uuid(isb_dp,
+                                                        &ts->header_.uuid, 1);
+                icsbrec_datapath_binding_set_type(isb_dp, "transit-switch");
             } else if (dp_key_refresh) {
                 /* Refresh tunnel key since encap mode has changed. */
                 int64_t dp_key = allocate_dp_key(dp_tnlids, vxlan_mode,
@@ -339,9 +338,13 @@  ts_run(struct engine_context *ctx,
             }
         }

-        struct shash_node *node;
-        SHASH_FOR_EACH (node, isb_ts_dps) {
-            icsbrec_datapath_binding_delete(node->data);
+        struct shash_node *node, *next;
+        SHASH_FOR_EACH_SAFE (node, next, isb_ts_dps) {
+            struct icsbrec_datapath_binding *isb_dp_to_del = node->data;
+            if (isb_dp_to_del->n_nb_ic_uuid > 0) {
+                icsbrec_datapath_binding_delete(isb_dp_to_del);
+            }
+            shash_delete(isb_ts_dps, node);
         }
     }
 }
@@ -3114,10 +3117,8 @@  ovn_db_run(struct ic_input *input_data,
            struct engine_context *eng_ctx)
 {
     gateway_run(eng_ctx, input_data);
-    enumerate_datapaths(input_data, &ic_data->dp_tnlids,
-                        &ic_data->isb_ts_dps, &ic_data->isb_tr_dps);
-    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);
+    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);
     port_binding_run(eng_ctx, input_data);
     route_run(eng_ctx, input_data);
     sync_service_monitor(eng_ctx, input_data);
@@ -3522,6 +3523,7 @@  main(int argc, char *argv[])
                              ovn_conn_show, ovnisb_idl_loop.idl);

     stopwatch_create(IC_OVN_DB_RUN_STOPWATCH_NAME, SW_MS);
+    stopwatch_create(OVN_IC_ENUM_DATAPATHS_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 2c2efc046..9b0009274 100644
--- a/ic/ovn-ic.h
+++ b/ic/ovn-ic.h
@@ -66,10 +66,10 @@  struct ic_input {
 };

 struct ic_data {
-    /* Global state for 'en-ic'. */
-    struct hmap dp_tnlids;
-    struct shash isb_ts_dps;
-    struct shash isb_tr_dps;
+    /* Global state for 'en-enum-datapaths'. */
+    struct hmap *dp_tnlids;
+    struct shash *isb_ts_dps;
+    struct shash *isb_tr_dps;
 };
 struct ic_state {
     bool had_lock;
diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
index e933213d8..21572f023 100644
--- a/lib/stopwatch-names.h
+++ b/lib/stopwatch-names.h
@@ -41,5 +41,6 @@ 
 #define GROUP_ECMP_ROUTE_RUN_STOPWATCH_NAME "group_ecmp_route"

 #define IC_OVN_DB_RUN_STOPWATCH_NAME "ovn_db_run"
+#define OVN_IC_ENUM_DATAPATHS_RUN_STOPWATCH_NAME "enum_datapaths_run"

 #endif