On Thu, Oct 9, 2025 at 5:49 PM Mairtin O'Loingsigh via dev <
[email protected]> wrote:

> Add support for transit routers and ovn-ic-nbctl commands to manage
> trasnit routers. When created, a transit router creates logical routers in
> each AZ
> with the same global datapath binding.
>
> This series also add support for commands to manage transit routers.
>
> Commands to add, delete and list transit routers.
>     $ ovn-ic-nbctl tr-add tr0
>     $ ovn-ic-nbctl tr-del tr0
>     $ ovn-ic-nbctl tr-list
>
> Signed-off-by: Mairtin O'Loingsigh <[email protected]>
> ---
>

Hi Mairtin,

thank you for the v3, I have a couple of comments down below.

 NEWS                     |   4 +
>  ic/ovn-ic.c              | 268 +++++++++++++++++++++++++++++++++------
>  ovn-architecture.7.xml   |  11 +-
>  tests/ovn-ic-nbctl.at    |  41 ++++++
>  tests/ovn-ic.at          |  27 ++++
>  utilities/ovn-ic-nbctl.c | 133 ++++++++++++++++++-
>  6 files changed, 443 insertions(+), 41 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index fb4d63194..593af397d 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -45,6 +45,10 @@ Post v25.09.0
>         enforced). Proper MTU needs to be configured to accomodate this
>         encapsulation.
>
> +   - Added Transit Router support:
> +    * Support the creation of Transit Routers.
> +    * Added new ovn-ic-nbctl 'tr-add','tr-del','tr-list' commands to
> manage
> +        Transit Router.
>  OVN v25.09.0 - xxx xx xxxx
>  --------------------------
>     - STT tunnels are no longer supported in ovn-encap-type.
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index 304953767..cbd325a31 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -66,6 +66,7 @@ struct ic_context {
>      struct ovsdb_idl_txn *ovnisb_txn;
>      const struct icsbrec_availability_zone *runned_az;
>      struct ovsdb_idl_index *nbrec_ls_by_name;
> +    struct ovsdb_idl_index *nbrec_lr_by_name;
>      struct ovsdb_idl_index *nbrec_lrp_by_name;
>      struct ovsdb_idl_index *nbrec_port_by_name;
>      struct ovsdb_idl_index *sbrec_chassis_by_name;
> @@ -90,6 +91,12 @@ struct ic_state {
>      bool paused;
>  };
>
> +enum ic_datapath_type {
> +    IC_SWITCH,
> +    IC_ROUTER,
> +    IC_PORT_MAX
> +};
> +
>  static const char *ovnnb_db;
>  static const char *ovnsb_db;
>  static const char *ovn_ic_nb_db;
> @@ -194,27 +201,84 @@ az_run(struct ic_context *ctx)
>  }
>
>  static uint32_t
> -allocate_ts_dp_key(struct hmap *dp_tnlids, bool vxlan_mode)
> +allocate_dp_key(struct hmap *dp_tnlids, bool vxlan_mode, const char *name)
>  {
>      uint32_t hint = vxlan_mode ? OVN_MIN_DP_VXLAN_KEY_GLOBAL
>                                 : OVN_MIN_DP_KEY_GLOBAL;
> -    return ovn_allocate_tnlid(dp_tnlids, "transit switch datapath", hint,
> +    return ovn_allocate_tnlid(dp_tnlids, name, hint,
>              vxlan_mode ? OVN_MAX_DP_VXLAN_KEY_GLOBAL :
> OVN_MAX_DP_KEY_GLOBAL,
>              &hint);
>  }
>
> +
> +struct ic_dp_info {
> +    struct hmap_node node;
> +    struct uuid uuid;
> +    const struct icsbrec_datapath_binding *isb_dp;
> +};
> +
> +static struct ic_dp_info *
> +ic_dp_find(struct hmap *dp_info, const struct uuid *uuid)
> +{
> +    struct ic_dp_info *info;
> +    HMAP_FOR_EACH_WITH_HASH (info, node, uuid_hash(uuid),
> +                             dp_info) {
> +        if (uuid_equals(&info->uuid, uuid)) {
> +            return info;
> +        }
> +
> +    }
> +    return NULL;
> +}
> +
> +static const struct icsbrec_datapath_binding *
> +ic_dp_find_and_remove(struct hmap *dp_info, const struct uuid *uuid)
> +{
> +    struct ic_dp_info *info = ic_dp_find(dp_info, uuid);
> +    if (info) {
> +        hmap_remove(dp_info, &info->node);
> +        const struct icsbrec_datapath_binding *isb_dp = info->isb_dp;
> +        free(info);
> +        return isb_dp;
> +    }
> +
> +    return NULL;
> +}
>

I would still probably prefer ic_dp_find_destroy
and in the code doing find and then destroy if the found
info is no longer relevant.


> +
> +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
> -ts_run(struct ic_context *ctx)
> +ts_run(struct ic_context *ctx, struct hmap *dp_tnlids,
> +       struct hmap *isb_ts_dps, struct hmap *isb_tr_dps)
>  {
>      const struct icnbrec_transit_switch *ts;
>      bool dp_key_refresh = false;
>
> -    struct hmap dp_tnlids = HMAP_INITIALIZER(&dp_tnlids);
> -    struct shash isb_dps = SHASH_INITIALIZER(&isb_dps);
>      const struct icsbrec_datapath_binding *isb_dp;
>      ICSBREC_DATAPATH_BINDING_FOR_EACH (isb_dp, ctx->ovnisb_idl) {
>

This loop should happen before both ts_run and tr_run, it's type
independent.


> -        shash_add(&isb_dps, isb_dp->transit_switch, isb_dp);
> -        ovn_add_tnlid(&dp_tnlids, isb_dp->tunnel_key);
> +        ovn_add_tnlid(dp_tnlids, isb_dp->tunnel_key);
> +        struct ic_dp_info *dp_info;
> +        dp_info = xmalloc(sizeof *dp_info);
> +        *dp_info = (struct ic_dp_info) {
> +            .isb_dp = isb_dp,
> +            .uuid = *isb_dp->nb_ic_uuid,
> +        };
> +
> +        if (ic_dp_get_type(isb_dp) == IC_ROUTER) {
> +            hmap_insert(isb_tr_dps, &dp_info->node, uuid_hash(
> +                isb_dp->nb_ic_uuid));
>
+            continue;
> +        }
> +
> +        hmap_insert(isb_ts_dps, &dp_info->node, uuid_hash(
> +            isb_dp->nb_ic_uuid));
>


nit: Those two do the same thing just for a different hmap,
you can just do a ternary operator to get the proper hmap
based on the type.

Also there is no need for the new struct ic_dp_info.
It can all be done via shash with a reference to
"icsbrec_datapath_binding".

     }
>
>      bool vxlan_mode = false;
> @@ -266,17 +330,18 @@ ts_run(struct ic_context *ctx)
>                  }
>              }
>
> -            isb_dp = shash_find_data(&isb_dps, ts->name);
> -            if (isb_dp) {
> +            struct ic_dp_info *dp_info = ic_dp_find(isb_ts_dps,
> +                &ts->header_.uuid);
> +            if (dp_info) {
>                  int64_t nb_tnl_key = smap_get_int(&ls->other_config,
>                                                    "requested-tnl-key",
>                                                    0);
> -                if (nb_tnl_key != isb_dp->tunnel_key) {
> +                if (nb_tnl_key != dp_info->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);
> +                             dp_info->isb_dp->tunnel_key, ls->name);
>                      char *tnl_key_str = xasprintf("%"PRId64,
> -                                                  isb_dp->tunnel_key);
> +
> dp_info->isb_dp->tunnel_key);
>                      nbrec_logical_switch_update_other_config_setkey(
>                          ls, "requested-tnl-key", tnl_key_str);
>                      free(tnl_key_str);
> @@ -298,10 +363,11 @@ ts_run(struct ic_context *ctx)
>      if (ctx->ovnisb_txn) {
>          /* Create ISB Datapath_Binding */
>          ICNBREC_TRANSIT_SWITCH_FOR_EACH (ts, ctx->ovninb_idl) {
> -            isb_dp = shash_find_and_delete(&isb_dps, ts->name);
> +            isb_dp = ic_dp_find_and_remove(isb_ts_dps, &ts->header_.uuid);
>              if (!isb_dp) {
>                  /* Allocate tunnel key */
> -                int64_t dp_key = allocate_ts_dp_key(&dp_tnlids,
> vxlan_mode);
> +                int dp_key = allocate_dp_key(dp_tnlids, vxlan_mode,
> +                    "transit switch datapath");
>

nit: Wrong alignment of the argument. That applies to other parts of the
change.


>                  if (!dp_key) {
>                      continue;
>                  }
> @@ -309,23 +375,145 @@ ts_run(struct ic_context *ctx)
>                  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);
> +                icsbrec_datapath_binding_set_nb_ic_uuid(isb_dp,
> +                    &ts->header_.uuid, 1);
> +                icsbrec_datapath_binding_set_type(isb_dp,
> "transit-switch");
> +                ovn_add_tnlid(dp_tnlids, isb_dp->tunnel_key);
> +
>              } else if (dp_key_refresh) {
> -                /* Refresh tunnel key since encap mode has changhed. */
> -                int64_t dp_key = allocate_ts_dp_key(&dp_tnlids,
> vxlan_mode);
> +                /* 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) {
> +                    ovn_free_tnlid(dp_tnlids, isb_dp->tunnel_key);
>                      icsbrec_datapath_binding_set_tunnel_key(isb_dp,
> dp_key);
>                  }
>
             }
>

We should ensure that the type and nb_ic_uuid is being set for
already existing TS too, to cover the upgrade scenario.



>          }
>
> -        /* Delete extra ISB Datapath_Binding */
> +        struct ic_dp_info *dp_info;
> +        HMAP_FOR_EACH_POP (dp_info, node, isb_ts_dps) {
> +            icsbrec_datapath_binding_delete(dp_info->isb_dp);
> +            free(dp_info);
> +        }
> +    }
> +}
> +
> +static const struct sbrec_chassis *
> +find_sb_chassis(struct ic_context *ctx, const char *name)
> +{
> +    const struct sbrec_chassis *key =
> +        sbrec_chassis_index_init_row(ctx->sbrec_chassis_by_name);
> +    sbrec_chassis_index_set_name(key, name);
> +
> +    const struct sbrec_chassis *chassis =
> +        sbrec_chassis_index_find(ctx->sbrec_chassis_by_name, key);
> +    sbrec_chassis_index_destroy_row(key);
> +
> +    return chassis;
> +}
>


Why was this function moved? I don't see any
usage update.


> +
> +static void
> +tr_run(struct ic_context *ctx, struct hmap *dp_tnlids,
> +       struct hmap *isb_tr_dps)
> +{
> +    struct shash nb_tres = SHASH_INITIALIZER(&nb_tres);
> +    const struct nbrec_logical_router *lr;
> +
> +    if (ctx->ovnnb_txn) {
> +        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);
> +            }
> +        }
>

nit: Add empty space please.


> +        const struct icnbrec_transit_router *tr;
> +        ICNBREC_TRANSIT_ROUTER_FOR_EACH (tr, ctx->ovninb_idl) {
> +            lr = shash_find_data(&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);
> +                shash_add(&nb_tres, tr->name, lr);
> +            }
> +
> +            struct ic_dp_info *dp_info = ic_dp_find(isb_tr_dps,
> +                &tr->header_.uuid);
> +            if (dp_info) {
> +                int64_t nb_tnl_key = smap_get_int(&lr->options,
> +                                                  "requested-tnl-key",
> +                                                  0);
> +                if (nb_tnl_key != dp_info->isb_dp->tunnel_key) {
> +                    VLOG_DBG("Set other_config:requested-tnl-key %"PRId64
> +                             " for transit router %s in NB.",
> +                             dp_info->isb_dp->tunnel_key, lr->name);
> +                    char *tnl_key_str = xasprintf("%"PRId64,
> +
> dp_info->isb_dp->tunnel_key);
> +                    nbrec_logical_router_update_options_setkey(
> +                        lr, "requested-tnl-key", tnl_key_str);
> +                    free(tnl_key_str);
> +                }
> +
> +            }
> +        }
> +    }
> +
> +    /* 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) {
> +        /* Create ISB Datapath_Binding */
> +        const struct icnbrec_transit_router *tr;
> +        ICNBREC_TRANSIT_ROUTER_FOR_EACH (tr, ctx->ovninb_idl) {
> +            lr = shash_find_and_delete(&nb_tres, tr->name);
>

The lr is not used for anything.


> +            struct ic_dp_info *dp_info = ic_dp_find(isb_tr_dps,
> +                &tr->header_.uuid);
> +            if (!dp_info) {
> +                int dp_key = smap_get_int(&tr->other_config,
> +                    "requested-tnl-key", 0);
>

We don't support "requested-tnl-key" as an option for TS so we
shouldn't do that for TR either.

+                if (!dp_key) {
> +                    dp_key = allocate_dp_key(dp_tnlids, false,
> +                        "transit router datapath");
> +                    if (!dp_key) {
> +                        VLOG_DBG("Set options:requested-tnl-key %"PRId32
> +                            " for lr %s in ISB.", dp_key, tr->name);
>

The vlog is wrong, if anything we should say that the allocation
failed.


> +                        continue;
> +                    }
> +
> +                }
> +
> +                struct icsbrec_datapath_binding *isb_dp;
> +                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");
> +            } else {
> +                hmap_remove(isb_tr_dps, &dp_info->node);
> +                free(dp_info);
>
+            }
> +
>

nit: Extra empty space.


> +        }
>

nit: Missing empty space.


> +        struct ic_dp_info *dp_info;
> +        HMAP_FOR_EACH_POP (dp_info, node, isb_tr_dps) {
> +            icsbrec_datapath_binding_delete(dp_info->isb_dp);
> +            free(dp_info);
> +        }
>

nit: Missing empty space.


>          struct shash_node *node;
> -        SHASH_FOR_EACH (node, &isb_dps) {
> -            icsbrec_datapath_binding_delete(node->data);
> +        SHASH_FOR_EACH (node, &nb_tres) {
>

This shash loop is in the wrong place. Here the nb txn might be null
it needs to be part of the first if.


> +            lr = node->data;
> +            nbrec_logical_router_delete(lr);
> +            const struct icnbrec_transit_router_port *trp;
> +            ICNBREC_TRANSIT_ROUTER_PORT_FOR_EACH (trp, ctx->ovninb_idl) {
>

With the schema adjustments this loop won't be needed.


> +                if (!strcmp(trp->transit_router, lr->name)) {
> +                    icnbrec_transit_router_port_delete(trp);
> +                }
> +
> +            }
>          }
>      }
> -    ovn_destroy_tnlids(&dp_tnlids);
> -    shash_destroy(&isb_dps);
> +    shash_destroy(&nb_tres);
>  }
>
>  /* Returns true if any information in gw and chassis is different. */
> @@ -584,20 +772,6 @@ get_lp_address_for_sb_pb(struct ic_context *ctx,
>      return peer->n_mac ? *peer->mac : NULL;
>  }
>
> -static const struct sbrec_chassis *
> -find_sb_chassis(struct ic_context *ctx, const char *name)
> -{
> -    const struct sbrec_chassis *key =
> -        sbrec_chassis_index_init_row(ctx->sbrec_chassis_by_name);
> -    sbrec_chassis_index_set_name(key, name);
> -
> -    const struct sbrec_chassis *chassis =
> -        sbrec_chassis_index_find(ctx->sbrec_chassis_by_name, key);
> -    sbrec_chassis_index_destroy_row(key);
> -
> -    return chassis;
> -}
> -
>  static void
>  sync_lsp_tnl_key(const struct nbrec_logical_switch_port *lsp,
>                   int64_t isb_tnl_key)
> @@ -2744,11 +2918,29 @@ update_sequence_numbers(struct ic_context *ctx,
>  static void
>  ovn_db_run(struct ic_context *ctx)
>  {
> +    struct hmap dp_tnlids = HMAP_INITIALIZER(&dp_tnlids);
> +    struct hmap isb_ts_dps = HMAP_INITIALIZER(&isb_ts_dps);
> +    struct hmap isb_tr_dps = HMAP_INITIALIZER(&isb_tr_dps);
> +
>      gateway_run(ctx);
> -    ts_run(ctx);
> +    ts_run(ctx, &dp_tnlids, &isb_ts_dps, &isb_tr_dps);
> +    tr_run(ctx, &dp_tnlids, &isb_tr_dps);
>      port_binding_run(ctx);
>      route_run(ctx);
>      sync_service_monitor(ctx);
> +
> +    ovn_destroy_tnlids(&dp_tnlids);
> +
> +    struct ic_dp_info *dps;
> +    HMAP_FOR_EACH_SAFE (dps, node, &isb_ts_dps) {
> +        free(dps);
> +    }
> +    hmap_destroy(&isb_ts_dps);
> +
> +    HMAP_FOR_EACH_SAFE (dps, node, &isb_tr_dps) {
> +        free(dps);
> +    }
> +    hmap_destroy(&isb_tr_dps);
>  }
>
>  static void
> @@ -3118,6 +3310,9 @@ main(int argc, char *argv[])
>      struct ovsdb_idl_index *nbrec_ls_by_name
>          = ovsdb_idl_index_create1(ovnnb_idl_loop.idl,
>                                    &nbrec_logical_switch_col_name);
> +    struct ovsdb_idl_index *nbrec_lr_by_name
> +        = ovsdb_idl_index_create1(ovnnb_idl_loop.idl,
> +                                  &nbrec_logical_router_col_name);
>      struct ovsdb_idl_index *nbrec_port_by_name
>          = ovsdb_idl_index_create1(ovnnb_idl_loop.idl,
>                                    &nbrec_logical_switch_port_col_name);
> @@ -3234,6 +3429,7 @@ main(int argc, char *argv[])
>                  .ovnisb_idl = ovnisb_idl_loop.idl,
>                  .ovnisb_txn = ovsdb_idl_loop_run(&ovnisb_idl_loop),
>                  .nbrec_ls_by_name = nbrec_ls_by_name,
> +                .nbrec_lr_by_name = nbrec_lr_by_name,
>

I don't see this index used anywhere.


>                  .nbrec_lrp_by_name = nbrec_lrp_by_name,
>                  .nbrec_port_by_name = nbrec_port_by_name,
>                  .sbrec_port_binding_by_name = sbrec_port_binding_by_name,
> diff --git a/ovn-architecture.7.xml b/ovn-architecture.7.xml
> index 11efdf9a2..9a932fef7 100644
> --- a/ovn-architecture.7.xml
> +++ b/ovn-architecture.7.xml
> @@ -2372,17 +2372,20 @@
>    <p>
>      A global OVN Interconnection Northbound database is introduced for the
>      operator (probably through CMS systems) to configure transit logical
> -    switches that connect logical routers from different AZs.  A transit
> -    switch is similar to a regular logical switch, but it is used for
> +    switches/routers that connect logical routers/switches from different
> AZs.
> +    A transit switch is similar to a regular logical switch, but it is
> used for
>      interconnection purpose only.  Typically, each transit switch can be
> used
>      to connect all logical routers that belong to same tenant across all
> AZs.
> +    A transit router is similar to a regular logical router, but is used
> for
> +    redirecting traffic to a specific AZ or cloning a router port across
> +    multiple AZs.
>    </p>
>
>    <p>
>      A dedicated daemon process <code>ovn-ic</code>, OVN interconnection
>      controller, in each AZ will consume this data and populate
> corresponding
> -    logical switches to their own northbound databases for each AZ, so
> that
> -    logical routers can be connected to the transit switch by creating
> +    logical switches/routers to their own northbound databases for each
> AZ. So
> +    that logical routers can be connected to the transit switch by
> creating
>      patch port pairs in their northbound databases.  Any router ports
>      connected to the transit switches are considered interconnection
> ports,
>      which will be exchanged between AZs.
> diff --git a/tests/ovn-ic-nbctl.at b/tests/ovn-ic-nbctl.at
> index 2402d646c..058d66cb1 100644
> --- a/tests/ovn-ic-nbctl.at
> +++ b/tests/ovn-ic-nbctl.at
> @@ -63,3 +63,44 @@ AT_CHECK([ovn-ic-nbctl --if-exists ts-del ts2])
>
>  OVN_IC_NBCTL_TEST_STOP
>  AT_CLEANUP
> +
> +AT_SETUP([ovn-ic-nbctl])
> +OVN_IC_NBCTL_TEST_START
> +
> +AT_CHECK([ovn-ic-nbctl tr-add tr0])
> +AT_CHECK([ovn-ic-nbctl tr-list | uuidfilt], [0], [dnl
> +<0> (tr0)
> +])
> +
> +AT_CHECK([ovn-ic-nbctl tr-add tr1])
> +AT_CHECK([ovn-ic-nbctl tr-list | uuidfilt], [0], [dnl
> +<0> (tr0)
> +<1> (tr1)
> +])
> +
> +AT_CHECK([ovn-ic-nbctl tr-list | ovn-ic-nbctl tr-list | cut -d' ' -f2 |
> sort], [0], [dnl
> +(tr0)
> +(tr1)
> +])
>

nit: This check is a bit redundant, it does essentially
the same thing as the check above.


> +
> +AT_CHECK([ovn-ic-nbctl tr-del tr1])
> +AT_CHECK([ovn-ic-nbctl tr-list | uuidfilt], [0], [dnl
> +<0> (tr0)
> +])
> +
> +AT_CHECK([ovn-ic-nbctl tr-add tr0], [1], [],
> +  [ovn-ic-nbctl: tr0: a transit router with this name already exists
> +])
> +
> +AT_CHECK([ovn-ic-nbctl --may-exist tr-add tr0])
> +AT_CHECK([ovn-ic-nbctl tr-list | uuidfilt], [0], [dnl
> +<0> (tr0)
> +])
> +
> +AT_CHECK([ovn-ic-nbctl tr-del tr2], [1], [],
> +  [ovn-ic-nbctl: tr2: router name not found
> +])
> +
> +AT_CHECK([ovn-ic-nbctl --if-exists tr-del tr2])
> +OVN_IC_NBCTL_TEST_STOP
> +AT_CLEANUP
> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
> index ced3fc9ed..424e9be1c 100644
> --- a/tests/ovn-ic.at
> +++ b/tests/ovn-ic.at
> @@ -4301,6 +4301,33 @@ ovn_as az2 check ovn-nbctl remove
> logical_router_port lrp-lr12-ts1 options ic-ro
>  ovn_as az1 check ovn-nbctl remove logical_router_port lrp-lr11-ts1
> options ic-route-deny-learn
>
>
> +OVN_CLEANUP_IC([az1], [az2])
> +AT_CLEANUP
> +])
> +
> +AT_BANNER([OVN Interconnection Controller Transit Router])
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn-ic -- Add transit router])
> +
> +ovn_init_ic_db
> +ovn_start az1
> +ovn_start az2
> +
> +check ovn-ic-nbctl --wait=sb sync
> +AT_CHECK([ovn-ic-sbctl show], [0], [dnl
> +availability-zone az1
> +availability-zone az2
> +])
> +
> +ovn_as az1
> +check ovn-ic-nbctl tr-add tr0
> +check ovn-ic-nbctl --wait=sb sync

+check_row_count nb:Logical_Router 1 name=tr0
> +
> +ovn_as az2
> +check ovn-ic-nbctl --wait=sb sync
>

The sync is for both azs no need to sync twice.



> +check_row_count nb:Logical_Router 1 name=tr0
> +
>

The test should also check delete and if the tnl-id allocated
by ovn-ic is properly set as an option.


>  OVN_CLEANUP_IC([az1], [az2])
>  AT_CLEANUP
>  ])

diff --git a/utilities/ovn-ic-nbctl.c b/utilities/ovn-ic-nbctl.c
> index ed07e5902..9d1784e86 100644
> --- a/utilities/ovn-ic-nbctl.c
> +++ b/utilities/ovn-ic-nbctl.c
> @@ -337,6 +337,11 @@ Transit switch commands:\n\
>    ts-del SWITCH              delete SWITCH\n\
>    ts-list                    print all transit switches\n\
>  \n\
> +Transit router commands:\n\
> +  tr-add ROUTER              create a transit router named ROUTER\n\
> +  tr-del ROUTER              delete ROUTER\n\
> +  tr-list                    print all transit routers\n\
> +\n\
>  Connection commands:\n\
>    get-connection             print the connections\n\
>    del-connection             delete the connections\n\
> @@ -397,7 +402,17 @@ struct ic_nbctl_context {
>  static struct cmd_show_table cmd_show_tables[] = {
>      {&icnbrec_table_transit_switch,
>       &icnbrec_transit_switch_col_name,
> -     {NULL},
> +     {NULL, NULL, NULL},
> +     {NULL, NULL, NULL}},
> +
> +    {&icnbrec_table_transit_router,
> +     &icnbrec_transit_router_col_name,
> +     {NULL, NULL, NULL},
> +     {NULL, NULL, NULL}},
> +
> +    {&icnbrec_table_transit_router_port,
> +     &icnbrec_transit_router_port_col_name,
> +     {&icnbrec_transit_router_port_col_nb_ic_uuid, NULL, NULL},
>       {NULL, NULL, NULL}},
>
>      {NULL, NULL, {NULL, NULL, NULL}, {NULL, NULL, NULL}},
> @@ -507,6 +522,106 @@ ic_nbctl_ts_list(struct ctl_context *ctx)
>      smap_destroy(&switches);
>      free(nodes);
>  }
> +
> +static char *
> +tr_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool
> must_exist,
> +                   const struct icnbrec_transit_router **tr_p)
> +{
> +    const struct icnbrec_transit_router *tr = NULL;
> +    *tr_p = NULL;
> +
> +    struct uuid tr_uuid;
> +    bool is_uuid = uuid_from_string(&tr_uuid, id);
> +    if (is_uuid) {
> +        tr = icnbrec_transit_router_get_for_uuid(ctx->idl, &tr_uuid);
> +    }
> +
> +    if (!tr) {
> +        const struct icnbrec_transit_router *iter;
> +
> +        ICNBREC_TRANSIT_ROUTER_FOR_EACH (iter, ctx->idl) {
> +            if (!strcmp(iter->name, id)) {
> +                tr = iter;
> +                break;
> +            }
> +        }
> +    }
> +
> +    if (!tr && must_exist) {
> +        return xasprintf("%s: router %s not found",
> +                         id, is_uuid ? "UUID" : "name");
> +    }
> +
> +    *tr_p = tr;
> +    return NULL;
> +}
> +
> +static void
> +ic_nbctl_tr_add(struct ctl_context *ctx)
> +{
> +    if (!ovsdb_idl_server_has_table(ctx->idl,
> &icnbrec_table_transit_router)) {
> +        VLOG_WARN("icnbrec_table_transit_router missing");
> +    }
> +
> +    const char *tr_name = ctx->argv[1];
> +    bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL;
> +
> +    const struct icnbrec_transit_router *tr = NULL;
> +    ICNBREC_TRANSIT_ROUTER_FOR_EACH (tr, ctx->idl) {
> +        if (!strcmp(tr->name, tr_name)) {
> +            if (may_exist) {
> +                return;
> +            }
> +            ctl_error(ctx, "%s: a transit router with this name already "
> +                      "exists", tr_name);
> +            return;
> +        }
> +    }
> +
> +    tr = icnbrec_transit_router_insert(ctx->txn);
> +    icnbrec_transit_router_set_name(tr, tr_name);
> +}
> +
> +
> +static void
> +ic_nbctl_tr_del(struct ctl_context *ctx)
> +{
> +    bool must_exist = !shash_find(&ctx->options, "--if-exists");
> +    const char *id = ctx->argv[1];
> +    const struct icnbrec_transit_router *tr = NULL;
> +
> +    char *error = tr_by_name_or_uuid(ctx, id, must_exist, &tr);
> +    if (error) {
> +        ctx->error = error;
> +        return;
> +    }
> +
> +    if (!tr) {
> +        return;
> +    }
> +
> +    icnbrec_transit_router_delete(tr);
> +}
> +
> +static void
> +ic_nbctl_tr_list(struct ctl_context *ctx)
> +{
> +    const struct icnbrec_transit_router *tr;
> +    struct smap routers;
> +
> +    smap_init(&routers);
> +    ICNBREC_TRANSIT_ROUTER_FOR_EACH (tr, ctx->idl) {
> +        smap_add_format(&routers, tr->name, UUID_FMT " (%s)",
> +                        UUID_ARGS(&tr->header_.uuid), tr->name);
> +    }
> +    const struct smap_node **nodes = smap_sort(&routers);
> +    for (size_t i = 0; i < smap_count(&routers); i++) {
> +        const struct smap_node *node = nodes[i];
> +        ds_put_format(&ctx->output, "%s\n", node->value);
> +    }
> +    smap_destroy(&routers);
> +    free(nodes);
> +}
>  static void
>  verify_connections(struct ctl_context *ctx)
>  {
> @@ -714,6 +829,15 @@ cmd_set_ssl(struct ctl_context *ctx)
>  static const struct ctl_table_class tables[ICNBREC_N_TABLES] = {
>      [ICNBREC_TABLE_TRANSIT_SWITCH].row_ids[0] =
>      {&icnbrec_transit_switch_col_name, NULL, NULL},
> +
> +    [ICNBREC_TABLE_TRANSIT_ROUTER].row_ids[0] =
> +    {&icnbrec_transit_router_col_name, NULL, NULL},
> +
> +    [ICNBREC_TABLE_TRANSIT_ROUTER_PORT].row_ids =
> +        {{&icnbrec_transit_router_port_col_name, NULL, NULL},
> +        {&icnbrec_transit_router_port_col_nb_ic_uuid, NULL, NULL},
> +        {&icnbrec_transit_router_port_col_mac, NULL, NULL},
> +        {&icnbrec_transit_router_port_col_transit_router, NULL, NULL}},
>  };
>
>
> @@ -1015,6 +1139,13 @@ static const struct ctl_command_syntax
> ic_nbctl_commands[] = {
>      { "ts-del", 1, 1, "SWITCH", NULL, ic_nbctl_ts_del, NULL,
> "--if-exists", RW },
>      { "ts-list", 0, 0, "", NULL, ic_nbctl_ts_list, NULL, "", RO },
>
> +    /* transit router commands. */
> +    { "tr-add", 1, 1, "ROUTER", NULL, ic_nbctl_tr_add, NULL,
> "--may-exist",
> +        RW },
> +    { "tr-del", 1, 1, "ROUTER", NULL, ic_nbctl_tr_del, NULL,
> "--if-exists",
> +        RW },
> +    { "tr-list", 0, 0, "", NULL, ic_nbctl_tr_list, NULL, "", RO },
> +
>      /* Connection commands. */
>      {"get-connection", 0, 0, "", pre_connection, cmd_get_connection,
> NULL, "",
>          RO},
> --
> 2.51.0
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Regards,
Ales
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to