On Wed, May 27, 2026 at 09:22:31AM +0200, Ales Musil wrote:
> On Wed, May 13, 2026 at 5:44 PM Mairtin O'Loingsigh <[email protected]>
> wrote:
> 
> > Enable the creation and management of transit switch ports, when
> > created, these ports are available across multiple AZs and may share
> > port binding depending on configuration. This patch also includes tests
> > and a multinode test.
> >
> > Commands to add, set address and delete port
> >     ovn-ic-nbctl tsp-add ts0 ts0-p0 chassis=chassis
> >     ovn-ic-nbctl tsp-set-addr ts0-p0 "00:11:22:11:22:34
> >         192.168.10.11/24"
> >     ovn-ic-nbctl tsp-del ts0-p0
> >
> > Reported-at: https://issues.redhat.com/browse/FDP-2878
> > Signed-off-by: Mairtin O'Loingsigh <[email protected]>
> > ---
> >
> 
> Hi Mairtin,
> 
> thank you for the v3. I have few comments down below.
> 
> 
> >
> > Changes since v2:
> >   - Remove function which was not needed.
> >   - Dont overwrite router-id.
> >   - Unify support functions.
> >   - Remove left over debug code from tests.
> >   - Add ovn-ic-nbctl.8.xml updates.
> >   - Add tsp-del test.
> >
> > Changes since v1:
> >   - Merge patches.
> >   - Bump OVNIC NB version number.
> >   - Add NEWS entry.
> >   - Remove option column from TSP.
> >   - Remove nb_uuid from TSP.
> >   - Clean up coding standard violations.
> >   - Fix missing port issue.
> >
> >  NEWS                         |   4 +
> >  ic/ovn-ic.c                  | 217 +++++++++++++++++++++++-------
> >  lib/ovn-util.c               |  44 +++++++
> >  lib/ovn-util.h               |   4 +
> >  ovn-ic-nb.ovsschema          |  23 +++-
> >  ovn-ic-nb.xml                |  37 ++++++
> >  tests/multinode.at           |  14 +-
> >  tests/ovn-ic-nbctl.at        |  30 +++++
> >  tests/ovn-ic.at              |  49 +++++++
> >  utilities/ovn-ic-nbctl.8.xml |  50 +++++++
> >  utilities/ovn-ic-nbctl.c     | 247 ++++++++++++++++++++++++++++++++++-
> >  utilities/ovn-nbctl.c        |  52 +-------
> >  12 files changed, 664 insertions(+), 107 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index 9839d19b9..ad0851a1c 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -15,6 +15,10 @@ Post v26.03.0
> >
> >  OVN v26.03.0 - xxx xx xxxx
> >  --------------------------
> > +   - Added Transit Switch port support:
> > +     * Support the creation of Transit Switch Ports.
> > +     * Added new ovn-ic-nbctl 'tsp-add', 'tsp-del' and 'tsp-set-addr'
> > commands
> > +       to manage Transit Switch Ports.
> >     - Added LSP/LRP option "requested-encap-ip" to let CMS request a
> > specific
> >       SB Port_Binding encap IP (e.g., for transit switch ports in ovn-k8s
> >       interconnect mode).
> > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> > index ba9490658..d5f16b087 100644
> > --- a/ic/ovn-ic.c
> > +++ b/ic/ovn-ic.c
> > @@ -733,6 +733,32 @@ get_lp_address_for_sb_pb(struct ic_context *ctx,
> >      return peer->n_mac ? *peer->mac : NULL;
> >  }
> >
> > +static const char *
> > +get_lp_address_for_ts_pb(struct ic_context *ctx,
> > +                         struct icnbrec_transit_switch_port *tsp)
> > +{
> > +    const struct nbrec_logical_switch_port *nb_lsp;
> > +
> > +    nb_lsp = get_lsp_by_ts_port_name(ctx, tsp->name);
> > +    if (!nb_lsp) {
> > +        return NULL;
> > +    }
> > +
> > +    if (!strcmp(nb_lsp->type, "switch")) {
> > +        /* Switches always have implicit "unknown" address, and IC-SB port
> > +         * binding can only have one address specified. */
> > +        return "unknown";
> > +    }
> > +
> > +    const struct sbrec_port_binding *peer =
> > +        find_sb_pb_by_name(ctx->sbrec_port_binding_by_name, tsp->peer);
> > +    if (peer && peer->n_mac) {
> > +        return *peer->mac;
> > +    } else {
> > +        return NULL;
> > +    }
> > +}
> > +
> >  static const struct sbrec_chassis *
> >  find_sb_chassis(struct ic_context *ctx, const char *name)
> >  {
> > @@ -818,27 +844,44 @@ update_isb_pb_external_ids(struct ic_context *ctx,
> >  }
> >
> >  /* For each local port:
> > - *   - Sync from NB to ISB.
> > - *   - Sync gateway from SB to ISB.
> > - *   - Sync tunnel key from ISB to NB.
> > + *   - Sync from ISB to NB/SB or from NB/SB to ISB.
> > + *   Legacy TSP sync from SB/NB towards ICB.
> > + *   TSP added using ICNB commands sync from ICB towards NB/SB.
> >   */
> >  static void
> > -sync_local_port(struct ic_context *ctx,
> > -                const struct icsbrec_port_binding *isb_pb,
> > -                const struct sbrec_port_binding *sb_pb,
> > -                const struct nbrec_logical_switch_port *lsp)
> > +sync_switch_port(struct ic_context *ctx,
> > +                 struct icnbrec_transit_switch_port *tsp,
> > +                 const struct icsbrec_port_binding *isb_pb,
> > +                 const struct nbrec_logical_switch_port *lsp,
> > +                 const struct sbrec_port_binding *sb_pb)
> >  {
> > -    /* Sync address from NB to ISB */
> > -    const char *address = get_lp_address_for_sb_pb(ctx, sb_pb);
> > -    if (!address) {
> > -        VLOG_DBG("Can't get router/switch port address for logical"
> > -                 " switch port %s", sb_pb->logical_port);
> > -        if (isb_pb->address[0]) {
> > -            icsbrec_port_binding_set_address(isb_pb, "");
> > +    if (tsp) {
> > +        const char *address = get_lp_address_for_ts_pb(ctx, tsp);
> > +        if (!address) {
> > +            VLOG_DBG("Can't get router/switch port address for transit "
> > +                     "switch port %s", lsp->name);
> > +            if (sb_pb->n_mac && sb_pb->mac[0]) {
> > +                //sbrec_port_binding_set_mac(sb_pb, NULL, 0);
> >
> 
> Why is this commented out?
Ack, It should not be commented out. Ill replace with sync of address
from ICSB.
> 
> 
> > +                icsbrec_port_binding_set_address(isb_pb, "");
> > +            }
> > +        } else {
> > +            if (sb_pb->n_mac && strcmp(address, sb_pb->mac[0])) {
> > +                sbrec_port_binding_set_mac(sb_pb, &address, 1);
> >
> 
> I'm also confused why we need to set MAC in SB?
> We should set only NB without touching SB,
> northd will take care of the rest.
> 
Ack.
> 
> > +                icsbrec_port_binding_set_address(isb_pb, address);
> > +            }
> >          }
> >
> 
> Looking at the loop below I think the part for TSP is wrong.
> We shouldn't sync anything from SB, we have an address and
> that should be populated from IC-NB. The sync should actually
> be more in line with the sync_router_port, there we sync from IC
> databases to NB.
Ack. Ill align the sync with sync_router_port, from ICNB towards SB.
> 
>      } else {
> > -        if (strcmp(address, isb_pb->address)) {
> > -            icsbrec_port_binding_set_address(isb_pb, address);
> > +        const char *address = get_lp_address_for_sb_pb(ctx, sb_pb);
> > +        if (!address) {
> > +            VLOG_DBG("Can't get router/switch port address for logical "
> > +                     "switch port %s", lsp->name);
> > +            if (isb_pb->address[0]) {
> > +                icsbrec_port_binding_set_address(isb_pb, "");
> > +            }
> > +        } else {
> > +            if (strcmp(address, isb_pb->address)) {
> > +                icsbrec_port_binding_set_address(isb_pb, address);
> > +            }
> >          }
> >      }
> >
> > @@ -1010,6 +1053,7 @@ create_isb_pb(struct ic_context *ctx, const char
> > *logical_port,
> >      icsbrec_port_binding_set_tunnel_key(isb_pb, pb_tnl_key);
> >      icsbrec_port_binding_set_nb_ic_uuid(isb_pb, nb_ic_uuid, 1);
> >      icsbrec_port_binding_set_type(isb_pb, type);
> > +
> >
> 
> nit: Unrelated change.
Ack.
> 
> 
> >      return isb_pb;
> >  }
> >
> > @@ -1028,19 +1072,15 @@ get_lrp_by_lrp_name(struct ic_context *ctx, const
> > char *lrp_name)
> >  }
> >
> >  static bool
> > -trp_is_remote(struct ic_context *ctx, const char *chassis_name)
> > +get_chassis_remote_status(struct ic_context *ctx, const char
> > *chassis_name)
> >
> 
> Also, this slightly changes the behavior slightly for TRPs,
> if there wasn't any chassis it was considered remote,
> now it will be considered local, that doesn't seem right.
> 
>  {
> > -    if (chassis_name) {
> > -        const struct sbrec_chassis *chassis =
> > -            find_sb_chassis(ctx, chassis_name);
> > -        if (chassis) {
> > -            return smap_get_bool(&chassis->other_config, "is-remote",
> > false);
> > -        } else {
> > -            return true;
> > -        }
> > +    const struct sbrec_chassis *chassis =
> > +        find_sb_chassis(ctx, chassis_name);
> >
> 
> nit: This fits to single line.
Ack
> 
> 
> > +    if (chassis) {
> > +        return smap_get_bool(&chassis->other_config, "is-remote", false);
> > +    } else {
> >
> 
> nit: No need to nest the return inside an else block.
Ack, ill remove else
> 
> 
> > +        return false;
> >      }
> > -
> > -    return false;
> >  }
> >
> >  static struct nbrec_logical_router_port *
> > @@ -1057,11 +1097,40 @@ lrp_create(struct ic_context *ctx, const struct
> > nbrec_logical_router *lr,
> >      return lrp;
> >  }
> >
> > +static struct nbrec_logical_switch_port *
> > +lsp_create(struct ic_context *ctx, const struct nbrec_logical_switch *ls,
> > +           const struct icnbrec_transit_switch_port *tsp)
> > +{
> > +    bool router_port = !strcmp(tsp->type, "router");
> > +    const char *type = router_port ? "router" : "";
> >
> 
> So we support only two types?
Ack. Ill remove this restriction as its not necessary.
> 
> 
> > +
> > +    struct nbrec_logical_switch_port *lsp =
> > +        nbrec_logical_switch_port_insert(ctx->ovnnb_txn);
> > +    nbrec_logical_switch_port_set_name(lsp, tsp->name);
> > +
> > +    nbrec_logical_switch_port_update_options_setkey(lsp, "interconn-ts",
> > +                                                    tsp->name);
> > +    nbrec_logical_switch_port_set_type(lsp, type);
> > +    nbrec_logical_switch_port_set_peer(lsp, tsp->peer);
> > +
> > +    if (router_port) {
> > +        nbrec_logical_switch_port_update_options_setkey(
> > +            lsp, "router-port", tsp->peer);
> > +    }
> > +
> > +    const char *addresses[] = { router_port ? "router": "unknown" };
> > +    nbrec_logical_switch_port_set_addresses(lsp, addresses, 1);
> >
> 
> I think we should actually copy the TSP address instead of deciding
> for it to be unknown for any port that isn'r router.
Ack, I'll update to this to take TSP address.
> 
> +
> > +    nbrec_logical_switch_update_ports_addvalue(ls, lsp);
> > +    return lsp;
> > +}
> > +
> >  static void
> >  sync_ts_isb_pb(struct ic_context *ctx, const struct sbrec_port_binding
> > *sb_pb,
> >                 const struct icsbrec_port_binding *isb_pb)
> >  {
> >      const char *address = get_lp_address_for_sb_pb(ctx, sb_pb);
> > +
> >
> 
> nit: Unrelated change.
Ack, Ill remove this.
> 
> 
> >      if (address) {
> >          icsbrec_port_binding_set_address(isb_pb, address);
> >      }
> > @@ -1118,7 +1187,6 @@ port_binding_run(struct ic_context *ctx)
> >      }
> >      icsbrec_port_binding_index_destroy_row(isb_pb_key);
> >
> > -    const struct sbrec_port_binding *sb_pb;
> >      const struct icnbrec_transit_switch *ts;
> >      ICNBREC_TRANSIT_SWITCH_FOR_EACH (ts, ctx->ovninb_idl) {
> >          const struct nbrec_logical_switch *ls = find_ts_in_nb(ctx,
> > ts->name);
> > @@ -1126,9 +1194,20 @@ port_binding_run(struct ic_context *ctx)
> >              VLOG_DBG("Transit switch %s not found in NB.", ts->name);
> >              continue;
> >          }
> > +        struct shash nb_ports = SHASH_INITIALIZER(&nb_ports);
> > +        struct shash old_nb_ports = SHASH_INITIALIZER(&old_nb_ports);
> >          struct shash local_pbs = SHASH_INITIALIZER(&local_pbs);
> >          struct shash remote_pbs = SHASH_INITIALIZER(&remote_pbs);
> >
> > +        for (size_t i = 0; i < ls->n_ports; i++) {
> > +            const struct nbrec_logical_switch_port *lsp = ls->ports[i];
> > +            if (smap_get(&lsp->options, "interconn-ts")) {
> > +                shash_add(&nb_ports, lsp->name, lsp);
> > +            } else {
> > +                shash_add(&old_nb_ports, lsp->name, lsp);
> > +            }
> > +        }
> > +
> >          isb_pb_key = icsbrec_port_binding_index_init_row(
> >              ctx->icsbrec_port_binding_by_ts);
> >          icsbrec_port_binding_index_set_transit_switch(isb_pb_key,
> > ts->name);
> > @@ -1145,9 +1224,54 @@ port_binding_run(struct ic_context *ctx)
> >          }
> >          icsbrec_port_binding_index_destroy_row(isb_pb_key);
> >
> > +        for (size_t i = 0; i < ts->n_ports; i++) {
> > +            struct icnbrec_transit_switch_port *tsp = ts->ports[i];
> > +            if (!get_chassis_remote_status(ctx, tsp->chassis)) {
> > +                isb_pb = shash_find_and_delete(&local_pbs, tsp->name);
> > +                if (!isb_pb) {
> > +                    isb_pb = create_isb_pb(ctx, tsp->name, ctx->runned_az,
> > +                                           ts->name, &ts->header_.uuid,
> > +                                           "transit-switch-port",
> > &pb_tnlids);
> > +                    if (!isb_pb) {
> > +                        continue;
> > +                    }
> > +                }
> > +
> > +                const struct nbrec_logical_switch_port *lsp =
> > +                    shash_find_and_delete(&nb_ports, tsp->name);
> > +                if (!lsp) {
> > +                    lsp = lsp_create(ctx, ls, tsp);
> > +                }
> >
> 
> Once we create that LSP and someone changes anything
> it won't be synchronized again.
Ack, Ill update to this to keep ports in sync.
> 
> 
> > +
> > +                const struct sbrec_port_binding *sb_pb;
> > +                sb_pb = find_lsp_in_sb(ctx, lsp);
> > +                if (sb_pb) {
> > +                    sync_switch_port(ctx, tsp, isb_pb, lsp, sb_pb);
> >
> 
> It's interesting but the sync_remote_port is actually
> more suited for both cases. Which is in line with the
> sync_router_port that is used for both cases.
Ack, Ill align sync_switch_port with router port.
> 
> 
> > +                }
> > +            } else {
> > +                /* Remote port sync */
> > +                isb_pb = shash_find_and_delete(&remote_pbs, tsp->name);
> > +                if (isb_pb) {
> > +                    const struct nbrec_logical_switch_port *lsp =
> > +                        shash_find_and_delete(&nb_ports, tsp->name);
> > +                    if (!lsp) {
> > +                        lsp = lsp_create(ctx, ls, tsp);
> > +                    }
> > +
> > +                    const struct sbrec_port_binding *sb_pb;
> > +                    sb_pb = find_lsp_in_sb(ctx, lsp);
> > +                    if (sb_pb) {
> > +                        sync_remote_port(ctx, isb_pb, lsp, sb_pb);
> > +                    }
> > +                }
> > +            }
> > +        }
> > +
> > +        /* Support legacy way of adding transit switch ports. */
> > +        const struct sbrec_port_binding *sb_pb;
> >          const struct nbrec_logical_switch_port *lsp;
> > -        for (int i = 0; i < ls->n_ports; i++) {
> > -            lsp = ls->ports[i];
> > +        SHASH_FOR_EACH (node, &old_nb_ports) {
> > +            lsp = node->data;
> >
> >              if (!strcmp(lsp->type, "router")
> >                  || !strcmp(lsp->type, "switch")) {
> > @@ -1163,17 +1287,7 @@ port_binding_run(struct ic_context *ctx)
> >                          &ts->header_.uuid, "transit-switch-port",
> > &pb_tnlids);
> >                      sync_ts_isb_pb(ctx, sb_pb, isb_pb);
> >                  } else {
> > -                    sync_local_port(ctx, isb_pb, sb_pb, lsp);
> > -                }
> > -
> > -                if (isb_pb->type) {
> > -                    icsbrec_port_binding_set_type(isb_pb,
> > -                                                  "transit-switch-port");
> > -                }
> > -
> > -                if (isb_pb->nb_ic_uuid) {
> > -                    icsbrec_port_binding_set_nb_ic_uuid(isb_pb,
> > -
> > &ts->header_.uuid, 1);
> >
> 
> Shouldn't we keep the behavior for legacy ports intact?
Ack, Ill reintroduce this to maintain legacy behavior.
> 
> 
> > +                    sync_switch_port(ctx, NULL, isb_pb, lsp, sb_pb);
> >                  }
> >              } else if (!strcmp(lsp->type, "remote")) {
> >                  /* The port is remote. */
> > @@ -1193,6 +1307,11 @@ port_binding_run(struct ic_context *ctx)
> >              }
> >          }
> >
> > +        SHASH_FOR_EACH (node, &nb_ports) {
> > +            nbrec_logical_switch_port_delete(node->data);
> > +            nbrec_logical_switch_update_ports_delvalue(ls, node->data);
> > +        }
> > +
> >          /* Delete extra port-binding from ISB */
> >          SHASH_FOR_EACH (node, &local_pbs) {
> >              icsbrec_port_binding_delete(node->data);
> > @@ -1203,8 +1322,10 @@ port_binding_run(struct ic_context *ctx)
> >              create_nb_lsp(ctx, node->data, ls);
> >          }
> >
> > +        shash_destroy(&nb_ports);
> >          shash_destroy(&local_pbs);
> >          shash_destroy(&remote_pbs);
> > +        shash_destroy(&old_nb_ports);
> >      }
> >
> >      SHASH_FOR_EACH (node, &switch_all_local_pbs) {
> > @@ -1250,7 +1371,7 @@ port_binding_run(struct ic_context *ctx)
> >          for (size_t i = 0; i < tr->n_ports; i++) {
> >              const struct icnbrec_transit_router_port *trp = tr->ports[i];
> >
> > -            if (trp_is_remote(ctx, trp->chassis)) {
> > +            if (get_chassis_remote_status(ctx, trp->chassis)) {
> >                  isb_pb = shash_find_and_delete(&remote_pbs, trp->name);
> >              } else {
> >                  isb_pb = shash_find_and_delete(&local_pbs, trp->name);
> > @@ -1275,7 +1396,7 @@ port_binding_run(struct ic_context *ctx)
> >              }
> >          }
> >
> > -        SHASH_FOR_EACH(node, &nb_ports) {
> > +        SHASH_FOR_EACH (node, &nb_ports) {
> >              nbrec_logical_router_port_delete(node->data);
> >              nbrec_logical_router_update_ports_delvalue(lr, node->data);
> >          }
> > @@ -2580,10 +2701,12 @@ route_run(struct ic_context *ctx)
> >          const struct nbrec_logical_switch_port *nb_lsp;
> >
> >          nb_lsp = get_lsp_by_ts_port_name(ctx, isb_pb->logical_port);
> > -        if (!strcmp(nb_lsp->type, "switch")) {
> > -            VLOG_DBG("IC-SB Port_Binding '%s' on ts '%s' corresponds to a
> > "
> > -                     "switch port, not considering for route collection.",
> > -                     isb_pb->logical_port, isb_pb->transit_switch);
> > +        if (!strcmp(nb_lsp->type, "switch") || !strcmp(nb_lsp->type, ""))
> > {
> > +            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> > +            VLOG_DBG_RL(&rl,
> > +                        "IC-SB Port_Binding '%s' on ts '%s' corresponds
> > to a "
> > +                        "switch port, not considering for route
> > collection.",
> > +                        isb_pb->logical_port, isb_pb->transit_switch);
> 
>              continue;
Ack.
> >          }
> >
> > diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> > index fb02825ac..d41ab366f 100644
> > --- a/lib/ovn-util.c
> > +++ b/lib/ovn-util.c
> > @@ -1869,3 +1869,47 @@ eth_addr_parse_masked(const char *s, struct
> > eth_addr *ea, unsigned int *plen)
> >      *ea = eth_addr_zero;
> >      return false;
> >  }
> > +
> > +bool
> > +sp_contains_duplicate_ip(struct lport_addresses *laddrs1,
> >
> 
> That is a little strange name, maybe `port_contains_duplicate_ip`?
Ack. Ill update name.
> 
> 
> > +                          struct lport_addresses *laddrs2,
> > +                          char *port_name,
> 
> 
> nit: const char *port_name.
Ack, Ill change to const.
> 
> 
> > +                          char **error_str)
> >
> 
> nit: Wrong alignment.
Ack.
> 
> 
> > +{
> > +    for (size_t i = 0; i < laddrs1->n_ipv4_addrs; i++) {
> > +        for (size_t j = 0; j < laddrs2->n_ipv4_addrs; j++) {
> > +            if (laddrs1->ipv4_addrs[i].addr ==
> > laddrs2->ipv4_addrs[j].addr) {
> > +                if (error_str) {
> > +                    *error_str = xasprintf("duplicate IPv4 address '%s' "
> > +                                           "found on logical switch "
> > +                                           "port '%s'",
> > +                                           laddrs1->ipv4_addrs[i].addr_s,
> > +                                           port_name);
> > +                }
> > +                return true;
> > +            }
> > +        }
> > +    }
> > +
> > +    for (size_t i = 0; i < laddrs1->n_ipv6_addrs; i++) {
> > +        for (size_t j = 0; j < laddrs2->n_ipv6_addrs; j++) {
> > +            if (IN6_ARE_ADDR_EQUAL(&laddrs1->ipv6_addrs[i].addr,
> > +                                   &laddrs2->ipv6_addrs[j].addr)) {
> > +                if (error_str) {
> > +                    *error_str = xasprintf("duplicate IPv6 address "
> > +                                           "'%s' found on logical "
> > +                                           "switch port '%s'",
> > +                                           laddrs1->ipv6_addrs[i].addr_s,
> > +                                           port_name);
> > +                }
> > +                return true;
> > +            }
> > +        }
> > +    }
> > +
> > +    if (error_str) {
> > +        *error_str = NULL;
> > +    }
> > +
> > +    return false;
> > +}
> > diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> > index b44c9c770..001af3c3e 100644
> > --- a/lib/ovn-util.h
> > +++ b/lib/ovn-util.h
> > @@ -790,6 +790,10 @@ char *normalize_ipv6_addr_str(const char *orig_addr);
> >
> >  char *normalize_addr_str(const char *orig_addr);
> >
> > +bool sp_contains_duplicate_ip(struct lport_addresses *laddrs1,
> > +                              struct lport_addresses *laddrs2, char *name,
> > +                              char **error_str);
> > +
> >  #define NEIGH_REDISTRIBUTE_MODES    \
> >      NEIGH_REDISTRIBUTE_MODE(FDB, 0) \
> >      NEIGH_REDISTRIBUTE_MODE(IP, 1)
> > diff --git a/ovn-ic-nb.ovsschema b/ovn-ic-nb.ovsschema
> > index ca67a2fa9..ae7f38377 100644
> > --- a/ovn-ic-nb.ovsschema
> > +++ b/ovn-ic-nb.ovsschema
> > @@ -1,7 +1,7 @@
> >  {
> >      "name": "OVN_IC_Northbound",
> > -    "version": "1.3.0",
> > -    "cksum": "1918565391 5082",
> > +    "version": "1.4.0",
> > +    "cksum": "1916436818 6015",
> >      "tables": {
> >          "IC_NB_Global": {
> >              "columns": {
> > @@ -24,9 +24,28 @@
> >                               "min": 0, "max": "unlimited"}}},
> >              "maxRows": 1,
> >              "isRoot": true},
> > +        "Transit_Switch_Port": {
> > +            "columns": {
> > +                "name": {"type": "string"},
> > +                "other_config": {
> > +                    "type": {"key": "string", "value": "string",
> > +                             "min": 0, "max": "unlimited"}},
> > +                "type": {"type": "string"},
> > +                "chassis": {"type": "string"},
> > +                "peer": {"type": "string"},
> > +                "addresses": {"type": {"key": "string",
> > +                                       "min": 0,
> > +                                       "max": "unlimited"}}},
> > +            "isRoot": false,
> > +            "indexes": [["name"]]},
> >          "Transit_Switch": {
> >              "columns": {
> >                  "name": {"type": "string"},
> > +                "ports": {"type": {"key": {"type": "uuid",
> > +                                           "refTable":
> > "Transit_Switch_Port",
> > +                                           "refType": "strong"},
> > +                                   "min": 0,
> > +                                   "max": "unlimited"}},
> >                  "other_config": {
> >                      "type": {"key": "string", "value": "string",
> >                               "min": 0, "max": "unlimited"}},
> > diff --git a/ovn-ic-nb.xml b/ovn-ic-nb.xml
> > index a3a35baf2..75e03c20b 100644
> > --- a/ovn-ic-nb.xml
> > +++ b/ovn-ic-nb.xml
> > @@ -112,6 +112,10 @@
> >        </column>
> >      </group>
> >
> > +    <column name="ports">
> > +      The switch's ports.
> > +    </column>
> > +
> >      <group title="Common Columns">
> >        <column name="other_config"/>
> >        <column name="external_ids">
> > @@ -190,6 +194,39 @@
> >      </group>
> >    </table>
> >
> > +  <table name="Transit_Switch_Port" title="Transit logical switch port">
> > +    <p>
> > +      Each row represents one transit logical switch port for
> > interconnection
> > +      between different OVN deployments (availability zones).
> > +    </p>
> > +
> > +    <group title="Naming">
> > +      <column name="name">
> > +        A name that uniquely identifies the transit logical switch port.
> > +      </column>
> > +    </group>
> > +
> > +    <column name="type">
> > +      Specify if the port is of type router or vif.
> > +    </column>
> > +
> > +    <column name="chassis">
> > +      The chassis this switch port should be bound to.
> > +    </column>
> > +
> > +    <column name="peer">
> > +      Name of peer port.
> > +    </column>
> > +
> > +    <column name="addresses">
> > +      Addresses to assign to port.
> > +    </column>
> > +
> > +    <group title="Common Columns">
> > +      <column name="other_config"/>
> > +    </group>
> > +  </table>
> > +
> >    <table name="SSL">
> >      SSL/TLS configuration for ovn-nb database access.
> >
> > diff --git a/tests/multinode.at b/tests/multinode.at
> > index d07660797..ffe13eb6d 100644
> > --- a/tests/multinode.at
> > +++ b/tests/multinode.at
> > @@ -2354,7 +2354,7 @@ fi
> >
> >  AT_CLEANUP
> >
> > -AT_SETUP([ovn multinode - Transit Router])
> > +AT_SETUP([ovn multinode - Transit Router + Transit Switch])
> >
> >  # Check that ovn-fake-multinode setup is up and running
> >  check_fake_multinode_setup
> > @@ -2424,7 +2424,7 @@ for i in 1 2; do
> >      check m_as $chassis ovs-vsctl set open .
> > external_ids:ovn-is-interconn=true
> >  done
> >
> > -# Do the ovn-ic setup.
> > +# Add TR and TS
> >  check m_central_as ovn-ic-nbctl tr-add tr
> >  check m_central_as ovn-ic-nbctl trp-add tr tr-gw1 \
> >      00:00:00:00:30:02 100.65.0.2/30 100:65::2/126 \
> > @@ -2434,6 +2434,9 @@ check m_central_as ovn-ic-nbctl trp-add tr tr-gw2 \
> >      chassis=ovn-chassis-2
> >  check m_central_as ovn-ic-nbctl --wait=sb sync
> >
> > +check m_central_as ovn-ic-nbctl ts-add ts
> > +check m_central_as ovn-ic-nbctl tsp-add ts ts-tr type=router peer=tr-ts
> > +
> >  for i in 1 2; do
> >      chassis="ovn-chassis-$i"
> >
> > @@ -2446,14 +2449,7 @@ for i in 1 2; do
> >
> >      check m_as $chassis ovn-nbctl set logical_router gw
> > options:chassis=$chassis
> >
> > -    # Add TR and set the same tunnel key for both chassis
> > -    check m_as $chassis ovn-nbctl ls-add ts
> > -    check m_as $chassis ovn-nbctl set logical_switch ts
> > other_config:requested-tnl-key=10
> > -
> > -    check m_as $chassis ovn-nbctl lsp-add-router-port ts ts-tr tr-ts
> > -
> >      check m_as $chassis ovn-nbctl lrp-add tr tr-ts 00:00:00:00:10:00
> > 10.100.200.1/24 10:200::1/64
> > -    check m_as $chassis ovn-nbctl set logical_router tr
> > options:requested-tnl-key=20
> >      check m_as $chassis ovn-nbctl lrp-set-gateway-chassis tr-ts $chassis
> >
> >      # Add TS pods, with the same tunnel keys on both sides
> > diff --git a/tests/ovn-ic-nbctl.at b/tests/ovn-ic-nbctl.at
> > index 4c5269784..315a7ce1c 100644
> > --- a/tests/ovn-ic-nbctl.at
> > +++ b/tests/ovn-ic-nbctl.at
> > @@ -61,6 +61,36 @@ AT_CHECK([ovn-ic-nbctl ts-del ts2], [1], [],
> >
> >  AT_CHECK([ovn-ic-nbctl --if-exists ts-del ts2])
> >
> > +AT_CHECK([ovn-ic-nbctl --may-exist ts-add ts0])
> > +AT_CHECK([ovn-ic-nbctl ts-list | uuidfilt], [0], [dnl
> > +<0> (ts0)
> > +])
> > +
> > +AT_CHECK([ovn-ic-nbctl tsp-add], [1], [],
> > +  [ovn-ic-nbctl: 'tsp-add' command requires at least 2 arguments
> > +])
> > +
> > +AT_CHECK([ovn-ic-nbctl tsp-add ts0], [1], [],
> > +  [ovn-ic-nbctl: 'tsp-add' command requires at least 2 arguments
> > +])
> > +
> > +AT_CHECK([ovn-ic-nbctl tsp-add ts0 ts0-p0 chassis=chassis])
> > +
> > +AT_CHECK([ovn-ic-nbctl --may-exist tsp-add ts0 ts0-p0 chassis=chassis])
> > +AT_CHECK([ovn-ic-nbctl tsp-set-addr ts0-p0 "00:11:22:11:22:33
> > 192.168.10.10/24"])
> > +AT_CHECK([ovn-ic-nbctl tsp-set-addr ts0-p1 "00:11:22:11:22:34
> > 192.168.10.11/24"], [1], [],
> > +    [ovn-ic-nbctl: ts0-p1: switch port name not found
> > +])
> > +
> > +AT_CHECK([ovn-ic-nbctl tsp-del], [1], [],
> > +  [ovn-ic-nbctl: 'tsp-del' command requires at least 1 arguments
> > +])
> > +AT_CHECK([ovn-ic-nbctl tsp-del ts0-p0])
> > +AT_CHECK([ovn-ic-nbctl tsp-del ts0-p0], [1], [],
> > +  [ovn-ic-nbctl: ts0-p0: switch port name not found
> > +])
> > +AT_CHECK([ovn-ic-nbctl --if-exists trp-del tr0-p0])
> > +
> >
> 
> We should definitely check address change here and also
> various types. Also there isn't any verification that the
> requested change actually made it through to IC-NB.
> 
Ack. Ill add updates and checks to verify IC-NB -> SB syncs are correct.
> 
> >  OVN_IC_NBCTL_TEST_STOP
> >  AT_CLEANUP
> >
> > diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
> > index 0fa7c4f29..44c49d17e 100644
> > --- a/tests/ovn-ic.at
> > +++ b/tests/ovn-ic.at
> > @@ -164,6 +164,55 @@ AT_CHECK([ovn-ic-sbctl show | grep -A2 lsp1], [0],
> > [dnl
> >              address: [["00:00:00:00:00:01 10.0.0.1/24"]]
> >  ])
> >
> > +# remove transit switch and check if port_binding is deleted
> > +check ovn-ic-nbctl --wait=sb ts-del ts1
> > +check_row_count ic-sb:Port_Binding 0 logical_port=lsp1
> > +for i in 1 2; do
> > +    az=az$i
> > +    ovn_as $az
> > +    OVN_CLEANUP_SBOX(gw-$az)
> > +    OVN_CLEANUP_AZ([$az])
> > +done
> > +OVN_CLEANUP_IC
> > +AT_CLEANUP
> > +])
> > +OVN_FOR_EACH_NORTHD([
> > +AT_SETUP([ovn-ic -- transit port-bindings deletion upon TS deletion])
> > +
> > +ovn_init_ic_db
> > +net_add n1
> > +
> > +# 1 GW per AZ
> > +for i in 1 2; do
> > +    az=az$i
> > +    ovn_start $az
> > +    sim_add gw-$az
> > +    as gw-$az
> > +    check ovs-vsctl add-br br-phys
> > +    ovn_az_attach $az n1 br-phys 192.168.1.$i
> > +    check ovs-vsctl set open . external-ids:ovn-is-interconn=true
> > +done
> > +
> > +ovn_as az1
> > +
> > +# create transit switch and connect to LR
> > +check ovn-ic-nbctl --wait=sb ts-add ts1
> > +check ovn-nbctl lr-add lr1
> > +check ovn-nbctl lrp-add lr1 lrp1 00:00:00:00:00:01 10.0.0.1/24
> > +check ovn-nbctl lrp-set-gateway-chassis lrp1 gw-az1
> > +check ovn-ic-nbctl tsp-add ts1 lsp1 type=router chassis=gw-az1 peer=lrp1
> > +
> > +wait_row_count Datapath_Binding 1 external_ids:interconn-ts=ts1
> > +
> > +# Sync ic-sb DB to see the TS changes.
> > +check ovn-ic-nbctl --wait=sb sync
> > +
> > +AT_CHECK([ovn-ic-sbctl show | grep -A2 lsp1], [0], [dnl
> > +        port lsp1
> > +            transit switch: ts1
> > +            address: [["00:00:00:00:00:01 10.0.0.1/24"]]
> > +])
> > +
> >
> 
> This should also test other types to see if the address is properly
> reflected.
Ack. Ill extend tests to cover various types.
> 
> 
> >  # remove transit switch and check if port_binding is deleted
> >  check ovn-ic-nbctl --wait=sb ts-del ts1
> >  check_row_count ic-sb:Port_Binding 0 logical_port=lsp1
> > diff --git a/utilities/ovn-ic-nbctl.8.xml b/utilities/ovn-ic-nbctl.8.xml
> > index 633863294..0ffc9daa7 100644
> > --- a/utilities/ovn-ic-nbctl.8.xml
> > +++ b/utilities/ovn-ic-nbctl.8.xml
> > @@ -50,6 +50,56 @@
> >        <dd>
> >          Lists all existing switches on standard output, one per line.
> >        </dd>
> > +
> > +      <dt>[<code>--may-exist</code>] <code>tsp-add</code>
> > <var>switch</var> <var>port</var></dt>
> > +      <dd>
> > +        <p>
> > +          Creates a new transit switch port named <var>port</var> on
> > <var>switch</var>.
> > +        </p>
> > +
> > +        <p>
> > +          Transit switch ports names must be unique. Adding a duplicated
> > name results
> > +          in error.  With <code>--may-exist</code>, adding a duplicate
> > name
> > +          succeeds but does not create a new transit switch port.
> > +        </p>
> > +      </dd>
> > +
> > +
> > +      <dt>[<code>--if-exists</code>] <code>tsp-del</code>
> > <var>port</var></dt>
> > +      <dd>
> > +        Deletes <var>port</var>.  It is an error if <var>port</var> does
> > +        not exist, unless <code>--if-exists</code> is specified.
> > +      </dd>
> > +
> > +      <dt><code>tsp-set-addr</code> <var>port</var>
> > [<var>address</var>]...</dt>
> > +      <dd>
> > +        <p>
> > +          Sets the addresses associated with <var>port</var> to
> > +          <var>address</var>.  Each <var>address</var> should be one of
> > the
> > +          following:
> > +        </p>
> > +
> > +        <dl>
> > +          <dt>an Ethernet address, optionally followed by a space and one
> > or more IP addresses</dt>
> > +          <dd>
> > +            OVN delivers packets for the Ethernet address to this port.
> > +          </dd>
> > +
> > +          <dt><code>router</code></dt>
> > +          <dd>
> > +            Accepted only when the <code>type</code> of the logical switch
> > +            port is <code>router</code>.  This indicates that the
> > Ethernet,
> > +            IPv4, and IPv6 addresses for this logical switch port should
> > be
> > +            obtained from the connected logical router port, as specified
> > by
> > +            peer column.
> > +          </dd>
> > +        </dl>
> > +
> > +        <p>
> > +          Multiple addresses may be set.  If no <var>address</var>
> > argument is
> > +          given, <var>port</var> will have no addresses associated with
> > it.
> > +        </p>
> > +      </dd>
> >      </dl>
> >
> >      <h1>Database Commands</h1>
> > diff --git a/utilities/ovn-ic-nbctl.c b/utilities/ovn-ic-nbctl.c
> > index 50e975283..1e9327d4c 100644
> > --- a/utilities/ovn-ic-nbctl.c
> > +++ b/utilities/ovn-ic-nbctl.c
> > @@ -336,6 +336,9 @@ Transit switch commands:\n\
> >    ts-add SWITCH              create a transit switch named SWITCH\n\
> >    ts-del SWITCH              delete SWITCH\n\
> >    ts-list                    print all transit switches\n\
> > +  tsp-add SWITCH PORT        add a transit switch PORT\n\
> > +  tsp-set-addr PORT ADDRESS set a transit switch PORT address\n\
> > +  tsp-del PORT               delete a transit switch PORT\n\
> >  \n\
> >  Transit router commands:\n\
> >    tr-add ROUTER              create a transit router named ROUTER\n\
> > @@ -400,6 +403,7 @@ struct ic_nbctl_context {
> >       * ic_nbctl_context_invalidate_cache() or manually update the cache to
> >       * maintain its correctness. */
> >      bool cache_valid;
> > +    struct shash tsp_to_ts_map;
> >  };
> >
> >  static struct cmd_show_table cmd_show_tables[] = {
> > @@ -617,6 +621,38 @@ trp_by_name_or_uuid(struct ctl_context *ctx, const
> > char *id, bool must_exist,
> >      return NULL;
> >  }
> >
> > +static char *
> > +tsp_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool
> > must_exist,
> > +                    const struct icnbrec_transit_switch_port **tsp_p)
> > +{
> > +    const struct icnbrec_transit_switch_port *tsp = NULL;
> > +    *tsp_p = NULL;
> > +    struct uuid tsp_uuid;
> > +    bool is_uuid = uuid_from_string(&tsp_uuid, id);
> > +    if (is_uuid) {
> > +        tsp = icnbrec_transit_switch_port_get_for_uuid(ctx->idl,
> > &tsp_uuid);
> > +    }
> > +
> > +    if (!tsp) {
> > +        const struct icnbrec_transit_switch_port *iter;
> > +
> > +        ICNBREC_TRANSIT_SWITCH_PORT_FOR_EACH (iter, ctx->idl) {
> > +            if (!strcmp(iter->name, id)) {
> > +                tsp = iter;
> > +                break;
> > +            }
> > +        }
> > +    }
> > +
> > +    if (!tsp && must_exist) {
> > +        return xasprintf("%s: switch port %s not found", id,
> > +                         is_uuid ? "UUID" : "name");
> > +    }
> > +
> > +    *tsp_p = tsp;
> > +    return NULL;
> > +}
> > +
> >  static void
> >  ic_nbctl_tr_del(struct ctl_context *ctx)
> >  {
> > @@ -664,6 +700,74 @@ ic_nbctl_trp_del(struct ctl_context *ctx)
> >      icnbrec_transit_router_port_delete(trp);
> >  }
> >
> > +static struct ic_nbctl_context *
> > +ic_nbctl_context_get(struct ctl_context *base)
> > +{
> > +    struct ic_nbctl_context *icnbctx
> > +        = CONTAINER_OF(base, struct ic_nbctl_context, base);
> > +    if (icnbctx->cache_valid) {
> > +        return icnbctx;
> > +    }
> > +
> > +    const struct icnbrec_transit_switch *ts;
> > +    ICNBREC_TRANSIT_SWITCH_FOR_EACH (ts, base->idl) {
> > +        for (size_t i = 0; i < ts->n_ports; i++) {
> > +            shash_add_once(&icnbctx->tsp_to_ts_map, ts->ports[i]->name,
> > ts);
> > +        }
> > +    }
> > +
> > +    icnbctx->cache_valid = true;
> > +    return icnbctx;
> > +}
> > +
> > +/* Returns the logical switch that contains 'lsp'. */
> > +static char * OVS_WARN_UNUSED_RESULT
> > +tsp_to_ts(struct ctl_context *ctx,
> > +          const struct icnbrec_transit_switch_port *tsp,
> > +          const struct icnbrec_transit_switch **ts_p)
> > +{
> > +    struct ic_nbctl_context *icnbctx = ic_nbctl_context_get(ctx);
> > +    const struct icnbrec_transit_switch *ts;
> > +    *ts_p = NULL;
> > +
> > +    ts = shash_find_data(&icnbctx->tsp_to_ts_map, tsp->name);
> > +    if (ts) {
> > +        *ts_p = ts;
> > +        return NULL;
> > +    }
> > +    /* Can't happen because of the database schema */
> > +    return xasprintf("transit port %s is not part of any transit switch",
> > +                     tsp->name);
> > +}
> > +
> > +static void
> > +ic_nbctl_tsp_del(struct ctl_context *ctx)
> > +{
> > +    bool must_exist = !shash_find(&ctx->options, "--if-exists");
> > +    const char *tsp_name = ctx->argv[1];
> > +    const struct icnbrec_transit_switch_port *tsp = NULL;
> > +
> > +    char *error = tsp_by_name_or_uuid(ctx, tsp_name, must_exist, &tsp);
> > +    if (error) {
> > +        ctx->error = error;
> > +        return;
> > +    }
> > +
> > +    if (!tsp) {
> > +        return;
> > +    }
> > +
> > +    const struct icnbrec_transit_switch *ts = NULL;
> > +    error = tsp_to_ts(ctx, tsp, &ts);
> > +    if (error) {
> > +        ctx->error = error;
> > +        return;
> > +    }
> > +
> > +    icnbrec_transit_switch_update_ports_delvalue(ts, tsp);
> > +    icnbrec_transit_switch_port_delete(tsp);
> > +}
> > +
> >  static void
> >  ic_nbctl_tr_list(struct ctl_context *ctx)
> >  {
> > @@ -802,6 +906,140 @@ ic_nbctl_trp_add(struct ctl_context *ctx)
> >      icnbrec_transit_router_update_ports_addvalue(tr, trp);
> >  }
> >
> > +static void
> > +ic_nbctl_tsp_add(struct ctl_context *ctx)
> > +{
> > +    bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL;
> > +    const char *ts_name = ctx->argv[1];
> > +    const char *tsp_name = ctx->argv[2];
> > +    const struct icnbrec_transit_switch *ts;
> > +
> > +    ctx->error = ts_by_name_or_uuid(ctx, ts_name, true, &ts);
> > +    if (ctx->error) {
> > +        return;
> > +    }
> > +
> > +    const struct icnbrec_transit_switch_port *tsp;
> > +    ctx->error = tsp_by_name_or_uuid(ctx, tsp_name, false, &tsp);
> > +    if (ctx->error) {
> > +        return;
> > +    }
> > +
> > +    if (tsp) {
> > +        if (!may_exist) {
> > +            ctl_error(ctx, "%s: a port with this name already exists",
> > +                      tsp_name);
> > +        }
> > +        return;
> > +    }
> > +
> > +    tsp = icnbrec_transit_switch_port_insert(ctx->txn);
> > +    icnbrec_transit_switch_port_set_name(tsp, tsp_name);
> > +
> > +    int n_settings = ctx->argc - 3;
> > +    char **settings = (char **) &ctx->argv[3];
> > +    for (size_t i = 0; i < n_settings; i++) {
> > +        ctx->error = ctl_set_column("Transit_Switch_Port", &tsp->header_,
> > +                                    settings[i], ctx->symtab);
> > +        if (ctx->error) {
> > +            return;
> > +        }
> > +    }
> > +
> > +    icnbrec_transit_switch_update_ports_addvalue(ts, tsp);
> > +}
> > +
> > +static char *
> > +tsp_contains_duplicates(const struct icnbrec_transit_switch *ts,
> > +                        const struct icnbrec_transit_switch_port *tsp,
> > +                        const char *address)
> > +{
> > +    char *sub_error = NULL;
> > +    struct lport_addresses laddrs;
> > +    if (!extract_lsp_addresses(address, &laddrs)) {
> > +        return NULL;
> > +    }
> > +
> > +    for (size_t i = 0; i < ts->n_ports; i++) {
> > +        struct icnbrec_transit_switch_port *tsp_test = ts->ports[i];
> > +        if (tsp_test == tsp) {
> > +            continue;
> > +        }
> > +
> > +        for (size_t j = 0; j < tsp_test->n_addresses; j++) {
> > +            struct lport_addresses laddrs_test;
> > +            char *addr = tsp_test->addresses[j];
> > +            if (extract_lsp_addresses(addr, &laddrs_test)) {
> > +                bool has_duplicate =
> > +                    sp_contains_duplicate_ip(&laddrs, &laddrs_test,
> > +                                             tsp_test->name, &sub_error);
> > +                destroy_lport_addresses(&laddrs_test);
> > +                if (has_duplicate) {
> > +                    goto err_out;
> > +                }
> > +            }
> > +        }
> > +    }
> > +
> > +err_out: ;
> > +    char *error = NULL;
> > +    if (sub_error) {
> > +        error = xasprintf("Error on switch %s: %s", ts->name, sub_error);
> > +        free(sub_error);
> > +    }
> > +    destroy_lport_addresses(&laddrs);
> > +    return error;
> > +}
> > +
> > +static void
> > +ic_nbctl_tsp_set_addr(struct ctl_context *ctx)
> > +{
> > +    const char *tsp_name = ctx->argv[1];
> > +
> > +    const struct icnbrec_transit_switch_port *tsp;
> > +    ctx->error = tsp_by_name_or_uuid(ctx, tsp_name, true, &tsp);
> > +    if (ctx->error) {
> > +        return;
> > +    }
> > +
> > +    const struct icnbrec_transit_switch *ts = NULL;
> > +    char *error = tsp_to_ts(ctx, tsp, &ts);
> > +    if (error) {
> > +        ctx->error = error;
> > +        return;
> > +    }
> > +
> > +    int i;
> > +    for (i = 2; i < ctx->argc; i++) {
> >
> >
> nit: for (int i = 2,...) {
Ack, ill place int inline.
> 
> 
> > +        char ipv6_s[IPV6_SCAN_LEN + 1];
> > +        struct eth_addr ea;
> > +        ovs_be32 ip;
> > +
> > +        if (strcmp(ctx->argv[i], "unknown") && strcmp(ctx->argv[i],
> > "dynamic")
> > +            && strcmp(ctx->argv[i], "router")
> > +            && !ovs_scan(ctx->argv[i], ETH_ADDR_SCAN_FMT,
> > +                         ETH_ADDR_SCAN_ARGS(ea))
> > +            && !ovs_scan(ctx->argv[i], "dynamic "IPV6_SCAN_FMT, ipv6_s)
> > +            && !ovs_scan(ctx->argv[i], "dynamic "IP_SCAN_FMT,
> > +                         IP_SCAN_ARGS(&ip))) {
> > +            ctl_error(ctx, "%s: Invalid address format. See ovn-nb(5). "
> > +                      "Hint: An Ethernet address must be "
> > +                      "listed before an IP address, together as a single "
> > +                      "argument.", ctx->argv[i]);
> > +            return;
> > +        }
> > +
> > +        ctx->error = tsp_contains_duplicates(ts, tsp, ctx->argv[i]);
> > +        if (ctx->error) {
> > +            return;
> > +        }
> > +    }
> > +
> > +    icnbrec_transit_switch_port_set_addresses(tsp,
> > +                                              (const char **) ctx->argv +
> > 2,
> > +                                              ctx->argc - 2);
> > +}
> > +
> >  static void
> >  verify_connections(struct ctl_context *ctx)
> >  {
> > @@ -1036,6 +1274,7 @@ ic_nbctl_context_init(struct ic_nbctl_context
> > *ic_nbctl_ctx,
> >      ctl_context_init(&ic_nbctl_ctx->base, command, idl, txn, symtab,
> >                       NULL);
> >      ic_nbctl_ctx->cache_valid = false;
> > +    shash_init(&ic_nbctl_ctx->tsp_to_ts_map);
> >  }
> >
> >  static void
> > @@ -1077,6 +1316,7 @@ ic_nbctl_context_done(struct ic_nbctl_context
> > *ic_nbctl_ctx,
> >                     struct ctl_command *command)
> >  {
> >      ctl_context_done(&ic_nbctl_ctx->base, command);
> > +    shash_destroy(&ic_nbctl_ctx->tsp_to_ts_map);
> >  }
> >
> >  static void
> > @@ -1317,7 +1557,13 @@ static const struct ctl_command_syntax
> > ic_nbctl_commands[] = {
> >      { "ts-add", 1, 1, "SWITCH", NULL, ic_nbctl_ts_add, NULL,
> > "--may-exist", RW },
> >      { "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 },
> > +    { "tsp-add", 2, INT_MAX, "SWITCH PORT COLUMN[:KEY]=VALUE]...",
> > +        NULL, ic_nbctl_tsp_add, NULL, "--may-exist", RW },
> > +    { "tsp-set-addr", 2, INT_MAX, "PORT ADDRESS...",
> > +        NULL, ic_nbctl_tsp_set_addr, NULL, "", RW },
> >
> > +    { "tsp-del", 1, 1, "PORT", NULL, ic_nbctl_tsp_del, NULL,
> > "--if-exists",
> > +        RW },
> >      /* transit router commands. */
> >      { "tr-add", 1, 1, "ROUTER", NULL, ic_nbctl_tr_add, NULL,
> > "--may-exist",
> >          RW },
> > @@ -1329,7 +1575,6 @@ static const struct ctl_command_syntax
> > ic_nbctl_commands[] = {
> >          NULL, ic_nbctl_trp_add, NULL, "--may-exist", RW },
> >      { "trp-del", 1, 1, "PORT", NULL, ic_nbctl_trp_del, NULL,
> > "--if-exists",
> >          RW },
> > -
> >      /* Connection commands. */
> >      {"get-connection", 0, 0, "", pre_connection, cmd_get_connection,
> > NULL, "",
> >          RO},
> > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> > index 9f6bb374b..861065371 100644
> > --- a/utilities/ovn-nbctl.c
> > +++ b/utilities/ovn-nbctl.c
> > @@ -1509,50 +1509,6 @@ nbctl_pre_lsp_set_addresses(struct ctl_context *ctx)
> >
> > &nbrec_logical_switch_port_col_dynamic_addresses);
> >  }
> >
> > -static bool
> > -lsp_contains_duplicate_ip(struct lport_addresses *laddrs1,
> > -                          struct lport_addresses *laddrs2,
> > -                          const struct nbrec_logical_switch_port
> > *lsp_test,
> > -                          char **error_str)
> > -{
> > -    for (size_t i = 0; i < laddrs1->n_ipv4_addrs; i++) {
> > -        for (size_t j = 0; j < laddrs2->n_ipv4_addrs; j++) {
> > -            if (laddrs1->ipv4_addrs[i].addr ==
> > laddrs2->ipv4_addrs[j].addr) {
> > -                if (error_str) {
> > -                    *error_str = xasprintf("duplicate IPv4 address '%s' "
> > -                                           "found on logical switch "
> > -                                           "port '%s'",
> > -                                           laddrs1->ipv4_addrs[i].addr_s,
> > -                                           lsp_test->name);
> > -                }
> > -                return true;
> > -            }
> > -        }
> > -    }
> > -
> > -    for (size_t i = 0; i < laddrs1->n_ipv6_addrs; i++) {
> > -        for (size_t j = 0; j < laddrs2->n_ipv6_addrs; j++) {
> > -            if (IN6_ARE_ADDR_EQUAL(&laddrs1->ipv6_addrs[i].addr,
> > -                                   &laddrs2->ipv6_addrs[j].addr)) {
> > -                if (error_str) {
> > -                    *error_str = xasprintf("duplicate IPv6 address "
> > -                                           "'%s' found on logical "
> > -                                           "switch port '%s'",
> > -                                           laddrs1->ipv6_addrs[i].addr_s,
> > -                                           lsp_test->name);
> > -                }
> > -                return true;
> > -            }
> > -        }
> > -    }
> > -
> > -    if (error_str) {
> > -        *error_str = NULL;
> > -    }
> > -
> > -    return false;
> > -}
> > -
> >  static char *
> >  lsp_contains_duplicates(const struct nbrec_logical_switch *ls,
> >                          const struct nbrec_logical_switch_port *lsp,
> > @@ -1577,8 +1533,8 @@ lsp_contains_duplicates(const struct
> > nbrec_logical_switch *ls,
> >              }
> >              if (extract_lsp_addresses(addr, &laddrs_test)) {
> >                  bool has_duplicate =
> > -                    lsp_contains_duplicate_ip(&laddrs, &laddrs_test,
> > -                                              lsp_test, &sub_error);
> > +                    sp_contains_duplicate_ip(&laddrs, &laddrs_test,
> > +                                             lsp_test->name, &sub_error);
> >                  destroy_lport_addresses(&laddrs_test);
> >                  if (has_duplicate) {
> >                      goto err_out;
> > @@ -8862,8 +8818,8 @@ lsp_health_check_parse_target_address(
> >              goto cleanup;
> >          }
> >
> > -        if (lsp_contains_duplicate_ip(&target_address,
> > -                                      &lsp_address, lsp, NULL)) {
> > +        if (sp_contains_duplicate_ip(&target_address,
> > +                                      &lsp_address, lsp->name, NULL)) {
> >              ip_found_on_port = true;
> >          }
> >
> > --
> > 2.54.0
> >
> >
> Regards,
> Ales

Hi Ales,

Thanks for the review!
Comments above.

Regards,
Mairtin

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to