On Fri, May 8, 2026 at 5:43 PM Mairtin O'Loingsigh via dev < [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 v2, I have a couple of comments down below. > NEWS | 4 + > ic/ovn-ic.c | 227 ++++++++++++++++++++++++++++++++---- > lib/ovn-util.c | 44 +++++++ > lib/ovn-util.h | 4 + > ovn-ic-nb.ovsschema | 23 +++- > ovn-ic-nb.xml | 37 ++++++ > tests/multinode.at | 217 ++++++++++++++++++++++++++++++++++- > tests/ovn-ic-nbctl.at | 25 ++++ > tests/ovn-ic.at | 50 ++++++++ > utilities/ovn-ic-nbctl.c | 241 ++++++++++++++++++++++++++++++++++++++- > utilities/ovn-nbctl.c | 52 +-------- > 11 files changed, 852 insertions(+), 72 deletions(-) > > diff --git a/NEWS b/NEWS > index 68cdbffea..73d91a8db 100644 > --- a/NEWS > +++ b/NEWS > @@ -9,6 +9,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' 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..b41b78e3e 100644 > --- a/ic/ovn-ic.c > +++ b/ic/ovn-ic.c > @@ -631,6 +631,23 @@ find_tr_in_nb(struct ic_context *ctx, char *tr_name) > return NULL; > } > > +static const struct nbrec_logical_router * > +find_router_by_port(struct ic_context *ctx, const char *port_name) > +{ > + const struct nbrec_logical_router *lr; > + const struct nbrec_logical_router_port *lrp; > + NBREC_LOGICAL_ROUTER_FOR_EACH (lr, ctx->ovnnb_idl) { > + for (size_t i = 0; i < lr->n_ports; i++) { > + lrp = lr->ports[i]; > + if (!strcmp(lrp->name, port_name)) { > + return lr; > + } > + } > + } > + > + return NULL; > +} > This lookup is very expensive, but it actually is not needed. See down below. + > static const struct sbrec_port_binding * > find_sb_pb_by_name(struct ovsdb_idl_index *sbrec_port_binding_by_name, > const char *name) > @@ -733,6 +750,28 @@ get_lp_address_for_sb_pb(struct ic_context *ctx, > return peer->n_mac ? *peer->mac : NULL; > } > > +static const char * > +get_lp_address_for_tr_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); > This can return NULL, we need to check for that. > + if (nb_lsp->type && !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) > { > @@ -865,6 +904,53 @@ sync_local_port(struct ic_context *ctx, > sync_lsp_tnl_key(lsp, isb_pb->tunnel_key); > } > > +/* For each local port: > + * - Sync from ISB to NB. > + * - Sync from ISB to SB. > + */ > +static void > +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) > +{ > + const char *address = get_lp_address_for_tr_pb(ctx, tsp); > + 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); > + } > + } > + > + /* Sync gateway from SB to ISB */ > + const struct sbrec_port_binding *crp = find_crp_for_sb_pb(ctx, sb_pb); > + if (crp && crp->chassis) { > + if (strcmp(crp->chassis->name, isb_pb->gateway)) { > + icsbrec_port_binding_set_gateway(isb_pb, crp->chassis->name); > + } > + } else if (!strcmp(tsp->type, "switch") && sb_pb->chassis) { + if (strcmp(sb_pb->chassis->name, isb_pb->gateway)) { > + icsbrec_port_binding_set_gateway(isb_pb, > sb_pb->chassis->name); > + } > + } else { > + if (isb_pb->gateway[0]) { > + icsbrec_port_binding_set_gateway(isb_pb, ""); > + } > + } > + > + /* Sync external_ids:router-id to ISB */ > + update_isb_pb_external_ids(ctx, sb_pb, isb_pb); > + > + /* Sync back tunnel key from ISB to NB */ > + sync_lsp_tnl_key(lsp, isb_pb->tunnel_key); > +} > + > /* For each remote port: > * - Sync from ISB to NB > * - Sync gateway from ISB to SB > @@ -991,6 +1077,21 @@ allocate_port_key(struct hmap *pb_tnlids) > 1, (1u << 15) - 1, &hint); > } > > +static void > +set_isb_pb_peer(struct ic_context *ctx, > + const struct icnbrec_transit_switch_port *tsp, > + const struct icsbrec_port_binding *isb_pb) > +{ > + const struct nbrec_logical_router *lr; > + lr = find_router_by_port(ctx, tsp->peer); > + if (lr) { > + char *uuid_s = xasprintf(UUID_FMT, UUID_ARGS(&lr->header_.uuid)); > + icsbrec_port_binding_update_external_ids_setkey(isb_pb, > "router-id", > + uuid_s); > + free(uuid_s); > + } > +} > This function is not needed. > + > static const struct icsbrec_port_binding * > create_isb_pb(struct ic_context *ctx, const char *logical_port, > const struct icsbrec_availability_zone *az, const char > *ts_name, > @@ -1010,6 +1111,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); > + > return isb_pb; > } > > @@ -1028,7 +1130,7 @@ 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) > +chassis_is_remote(struct ic_context *ctx, const char *chassis_name) > { > if (chassis_name) { > const struct sbrec_chassis *chassis = > @@ -1057,11 +1159,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" : ""; > + > + 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); > + > + 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); > + > if (address) { > icsbrec_port_binding_set_address(isb_pb, address); > } > @@ -1118,7 +1249,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 +1256,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 +1286,56 @@ 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 (!chassis_is_remote(ctx, tsp->chassis) || > + !strcmp(tsp->chassis, "")) { > This can be "!tsp->chassis[0]". Also the chassis_is_remote is strange as it is right now. Maybe it should be adjusted to better reflect what it actually does. The else case for empty string doesn't fit. + 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; > + } > + set_isb_pb_peer(ctx, tsp, isb_pb); > This call sets the router-id, however that is also set by the sync_switch_port later on anyway. So let's use that? + } > + > + 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_switch_port(ctx, tsp, isb_pb, lsp, sb_pb); > + } > + } 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); > + } > + } > + } > + } > This loop feels like it be actually unified with the legacy way. They are doing almost the same thing. A lot of those support functions could be removed too. > + > + /* 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")) { > @@ -1165,16 +1353,6 @@ port_binding_run(struct ic_context *ctx) > } 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 this for the legacy ports? > } else if (!strcmp(lsp->type, "remote")) { > /* The port is remote. */ > isb_pb = shash_find_and_delete(&remote_pbs, lsp->name); > @@ -1193,6 +1371,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 +1386,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 +1435,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 (chassis_is_remote(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 +1460,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 +2765,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; > } > > 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, > + struct lport_addresses *laddrs2, > + char *port_name, > + 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, > + 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..b660f10ab 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 if 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..30015eb82 100644 > --- a/tests/multinode.at > +++ b/tests/multinode.at > @@ -2453,7 +2453,222 @@ for i in 1 2; do > 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 > + check m_as $chassis ovn-nbctl lsp-add ts pod10 > + check m_as $chassis ovn-nbctl lsp-set-addresses pod10 > "00:00:00:00:10:10 10.100.200.10 10:200::10" > + check m_as $chassis ovn-nbctl set logical_switch_port pod10 > options:requested-tnl-key=10 > + > + check m_as $chassis ovn-nbctl lsp-add ts pod20 > + check m_as $chassis ovn-nbctl lsp-set-addresses pod20 > "00:00:00:00:10:20 10.100.200.20 10:200::20" > + check m_as $chassis ovn-nbctl set logical_switch_port pod20 > options:requested-tnl-key=20 > + > + # Add mgmt pod > + check m_as $chassis ovn-nbctl lsp-add ts mgmt > + check m_as $chassis ovn-nbctl lsp-set-addresses mgmt > "00:00:00:00:10:11 10.100.200.11 10:200::11" > + check m_as $chassis ovn-nbctl set logical_switch_port mgmt > options:requested-tnl-key=11 > +done > + > +# Add SNAT for the GW router that corresponds to "gw-tr" LRP IP > +check m_as ovn-chassis-1 ovn-nbctl lr-nat-add gw snat 100.65.0.1 > 192.168.100.0/24 > +check m_as ovn-chassis-1 ovn-nbctl lr-nat-add gw snat 100:65::1 1000::/64 > +check m_as ovn-chassis-2 ovn-nbctl lr-nat-add gw snat 100.65.0.5 > 192.168.100.0/24 > +check m_as ovn-chassis-2 ovn-nbctl lr-nat-add gw snat 100:65::5 1000::/64 > + > +# Add peer ports between GW and TR > +check m_as ovn-chassis-1 ovn-nbctl lrp-add gw gw-tr 00:00:00:00:30:01 > 100.65.0.1/30 100:65::1/126 peer=tr-gw1 > +check m_as ovn-chassis-1 ovn-nbctl set logical_router_port tr-gw1 > peer="gw-tr" > + > +check m_as ovn-chassis-2 ovn-nbctl lrp-add gw gw-tr 00:00:00:00:30:05 > 100.65.0.5/30 100:65::5/126 peer=tr-gw2 > +check m_as ovn-chassis-2 ovn-nbctl set logical_router_port tr-gw2 > peer="gw-tr" > + > +# Add routes for the TS subnet > +check m_as ovn-chassis-1 ovn-nbctl lr-route-add gw 10.100.200.0/24 > 100.65.0.2 > +check m_as ovn-chassis-1 ovn-nbctl lr-route-add gw 10:200::/64 100:65::2 > +check m_as ovn-chassis-2 ovn-nbctl lr-route-add gw 10.100.200.0/24 > 100.65.0.6 > +check m_as ovn-chassis-2 ovn-nbctl lr-route-add gw 10:200::/64 100:65::6 > + > +# Add LB on TS and condition NAT > +check m_as ovn-chassis-1 ovn-nbctl lb-add lb0 172.16.0.5:5656 > 10.100.200.10:2324 tcp > +check m_as ovn-chassis-1 ovn-nbctl ls-lb-add ts lb0 > +check m_as ovn-chassis-1 ovn-nbctl --match="eth.dst == 00:00:00:00:10:11" > lr-nat-add tr snat 172.16.0.2 10.100.200.0/24 > +check m_as ovn-chassis-1 ovn-nbctl set logical_router tr > options:ct-commit-all="true" > + > +check m_as ovn-chassis-1 ovn-nbctl lsp-add public external > +check m_as ovn-chassis-1 ovn-nbctl lsp-set-addresses external > "00:00:00:00:20:10 192.168.100.10 1000::10" > + > +# Configure ports on the transit switch as remotes > +check m_as ovn-chassis-1 ovn-nbctl lsp-set-type pod20 remote > +check m_as ovn-chassis-1 ovn-nbctl lsp-set-options pod10 > requested-chassis=ovn-chassis-1 > +check m_as ovn-chassis-1 ovn-nbctl lsp-set-options mgmt > requested-chassis=ovn-chassis-1 > +check m_as ovn-chassis-1 ovn-nbctl lsp-set-options pod20 > requested-chassis=ovn-chassis-2 > + > +check m_as ovn-chassis-2 ovn-nbctl lsp-set-type pod10 remote > +check m_as ovn-chassis-2 ovn-nbctl lsp-set-type mgmt remote > +check m_as ovn-chassis-2 ovn-nbctl lsp-set-options pod10 > requested-chassis=ovn-chassis-1 > +check m_as ovn-chassis-2 ovn-nbctl lsp-set-options mgmt > requested-chassis=ovn-chassis-1 > +check m_as ovn-chassis-2 ovn-nbctl lsp-set-options pod20 > requested-chassis=ovn-chassis-2 > + > +m_as ovn-chassis-1 /data/create_fake_vm.sh external external > 00:00:00:00:20:10 1500 192.168.100.10 24 192.168.100.1 1000::10/64 1000::1 > +m_as ovn-chassis-1 /data/create_fake_vm.sh pod10 pod10 00:00:00:00:10:10 > 1500 10.100.200.10 24 10.100.200.1 10:200::10/64 10:200::1 > +m_as ovn-chassis-1 /data/create_fake_vm.sh mgmt mgmt 00:00:00:00:10:11 > 1500 10.100.200.11 24 10.100.200.1 10:200::11/64 10:200::1 > +m_as ovn-chassis-2 /data/create_fake_vm.sh pod20 pod20 00:00:00:00:10:20 > 1500 10.100.200.20 24 10.100.200.1 10:200::20/64 10:200::1 > + > +# We cannot use any of the helpers as they assume that there is only > single ovn-northd instance running > +check m_as ovn-chassis-1 ovn-nbctl --wait=hv sync > +OVS_WAIT_UNTIL([test -n "$(m_as ovn-chassis-1 ovn-sbctl --bare --columns > _uuid find Port_Binding logical_port=external up=true)"]) > +OVS_WAIT_UNTIL([test -n "$(m_as ovn-chassis-1 ovn-sbctl --bare --columns > _uuid find Port_Binding logical_port=pod10 up=true)"]) > +check m_as ovn-chassis-2 ovn-nbctl --wait=hv sync > +OVS_WAIT_UNTIL([test -n "$(m_as ovn-chassis-2 ovn-sbctl --bare --columns > _uuid find Port_Binding logical_port=pod20 up=true)"]) > + > +M_NS_CHECK_EXEC([ovn-chassis-1], [external], [ping -q -c 5 -i 0.3 -w 2 > 10.100.200.20 | FORMAT_PING], \ > +[0], [dnl > +5 packets transmitted, 5 received, 0% packet loss, time 0ms > +]) > + > +M_NS_CHECK_EXEC([ovn-chassis-1], [external], [ping -q -c 5 -i 0.3 -w 2 > 10:200::20 | FORMAT_PING], \ > +[0], [dnl > +5 packets transmitted, 5 received, 0% packet loss, time 0ms > +]) > + > +M_NS_CHECK_EXEC([ovn-chassis-1], [mgmt], [ip a a 172.16.100.2/24 dev > mgmt]) > +M_NS_DAEMONIZE([ovn-chassis-1], [pod10], [nc -e /bin/cat -v -l -o > server.log 10.100.200.10 2324], [pod10.pid]) > +M_START_TCPDUMP([ovn-chassis-1], [-neei pod10-p ip], [pod10]) > +M_START_TCPDUMP([ovn-chassis-1], [-neei mgmt-p ip], [mgmt]) > + > +m_as ovn-chassis-1 sh -c 'echo -e "Hello\nHello" > msg.expected' > +check m_as ovn-chassis-1 ovn-nbctl --policy="src-ip" lr-route-add tr > 10.100.200.0/24 10.100.200.11 > + > +check test $(m_as ovn-chassis-1 grep -c "skipping output to input port" \ > + /var/log/openvswitch/ovs-vswitchd.log) -eq 0 > +check test $(m_as ovn-chassis-2 grep -c "skipping output to input port" \ > + /var/log/openvswitch/ovs-vswitchd.log) -eq 0 > + > +M_NS_CHECK_EXEC([ovn-chassis-1], [mgmt], [sh -c '(echo "Hello"; sleep 3) > | nc -s 172.16.100.2 -o client.log 172.16.0.5 5656'], [0], [ignore], > [ignore]) > +check m_as ovn-chassis-1 cmp server.log msg.expected > +check m_as ovn-chassis-1 cmp client.log msg.expected > + > +echo "Chassis1" > +m_as ovn-chassis-1 ovn-sbctl show > +m_as ovn-chassis-1 ovn-nbctl show > +m_as ovn-chassis-1 ovs-vsctl show > + > +echo "Chassis2" > +m_as ovn-chassis-2 ovn-sbctl show > +m_as ovn-chassis-2 ovn-nbctl show > +m_as ovn-chassis-2 ovs-vsctl show > + > +# Connect the chassis back to the original northd and remove northd per > chassis. > +for i in 1 2; do > + chassis="ovn-chassis-$i" > + ip=$(m_as $chassis ip -4 addr show eth1 | grep inet | awk '{print > $2}' | cut -d'/' -f1) > + > + multinode_cleanup_ic $chassis > + multinode_setup_controller $chassis $chassis $ip "170.168.0.2" > + multinode_cleanup_northd $chassis > +done > + > +m_as ovn-chassis-1 killall tcpdump > + > +AT_CLEANUP > + > +AT_SETUP([ovn multinode - Transit Router + Transit Switch]) > I don't think we need to copy the test, converting the existing one to use the new command is fine. It will also nicely illlistrate the improvement. > + > +# Check that ovn-fake-multinode setup is up and running > +check_fake_multinode_setup > + > +# Delete the multinode NB and OVS resources before starting the test. > +cleanup_multinode_resources > + > +# Network topology > +# ┌─────────────────────────────────┐ > ┌────────────────────────────────┐ > +# │ │ │ > │ > +# │ ┌───────────────────┐ AZ1 │ │ AZ2 > ┌───────────────────┐ │ > +# │ │ external │ │ │ │ > │ │ > +# │ │ │ │ │ │ > │ │ > +# │ │ 192.168.100.10/24 │ │ │ │ > ................. │ │ > +# │ │ 1000::10/64 │ │ │ │ > │ │ > +# │ └─────────┬─────────┘ │ │ > └─────────┬─────────┘ │ > +# │ │ │ │ │ > │ > +# │ │ │ │ │ > │ > +# │ ┌─────────┴─────────┐ │ │ > ┌─────────┴─────────┐ │ > +# │ │ 192.168.100.1/24 │ │ │ │ 192.168.100.2/24 > │ │ > +# │ │ 1000::1/64 │ │ │ │ 1000::2/64 > │ │ > +# │ │ │ │ │ │ > │ │ > +# │ │ GW │ │ │ │ GW > │ │ > +# │ │ │ │ │ │ > │ │ > +# │ │ 100.65.0.1/30 │ │ │ │ 100.65.0.5/30 > │ │ > +# │ │ 100:65::1/126 │ │ │ │ 100:65::5/126 > │ │ > +# │ └─────────┬─────────┘ │ │ > └───────────────────┘ │ > +# │ │ │ │ │ > │ > +# │ │ Peer ports │ │ │ Peer > ports │ > +# │ │ │ │ │ > │ > +# │ > ┌─────────┴──────────────────│─────│──────────────────┴─────────┐ │ > +# │ │ 100.65.0.2/30 │ │ 100.65.0.6/30 > │ │ > +# │ │ 100:65::2/126 │ │ 100:65::6/126 > │ │ > +# │ │ │ │ > │ │ > +# │ │ │ TR │ > │ │ > +# │ │ │ │ > │ │ > +# │ │ 10.100.200.1/24 │ │ 10.100.200.1/24 > │ │ > +# │ │ 10:200::1/64 │ │ 10:200::1/64 > │ │ > +# │ > └─────────┬──────────────────│─────│────────────────────────────┘ │ > +# │ │ │ │ │ > │ > +# │ │ │ │ │ > │ > +# │ │ │ │ │ > │ > +# │ > ┌─────────┴──────────────────│─────│────────────────────────────┐ │ > +# │ │ │ TS │ > │ │ > +# │ > └─────────┬──────────────────│─────│────────────────────────────┘ │ > +# │ │ │ │ │ > │ > +# │ │ │ │ │ > │ > +# │ │ │ │ │ > │ > +# │ ┌─────────┴─────────┐ │ │ > ┌─────────┴─────────┐ │ > +# │ │ pod10 │ │ │ │ pod20 > │ │ > +# │ │ │ │ │ │ > │ │ > +# │ │ 10.100.200.10/24 │ │ │ │ 10.100.200.20/24 > │ │ > +# │ │ 10:200::10/64 │ │ │ │ 10:200::20/64 > │ │ > +# │ └───────────────────┘ │ │ > └───────────────────┘ │ > +# └─────────────────────────────────┘ > └────────────────────────────────┘ > + > +for i in 1 2; do > + chassis="ovn-chassis-$i" > + ip=$(m_as $chassis ip -4 addr show eth1 | grep inet | awk '{print > $2}' | cut -d'/' -f1) > + central_ip=$(m_central_as ip -4 addr show eth1 | grep inet | awk > '{print $2}' | cut -d'/' -f1) > + > + multinode_setup_northd $chassis > + multinode_setup_controller $chassis $chassis $ip $ip > + multinode_setup_ic $chassis $central_ip > + > + check m_as $chassis ovs-vsctl set open . > external_ids:ovn-monitor-all=true > + check m_as $chassis ovs-vsctl set open . > external_ids:ovn-is-interconn=true > +done > + > +# 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 \ > + chassis=ovn-chassis-1 > +check m_central_as ovn-ic-nbctl trp-add tr tr-gw2 \ > + 00:00:00:00:30:06 100.65.0.6/30 100:65::6/126 \ > + 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" > + > + check m_as $chassis ovn-nbctl ls-add public > + > + check m_as $chassis ovn-nbctl lsp-add-router-port public public-gw > gw-public > + > + check m_as $chassis ovn-nbctl lr-add gw > + check m_as $chassis ovn-nbctl lrp-add gw gw-public 00:00:00:00:20:00 > 192.168.100.$i/24 1000::$i/64 > + > + check m_as $chassis ovn-nbctl set logical_router gw > options:chassis=$chassis > + > + 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 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..576828428 100644 > --- a/tests/ovn-ic-nbctl.at > +++ b/tests/ovn-ic-nbctl.at > @@ -61,6 +61,31 @@ 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 > +]) > + > OVN_IC_NBCTL_TEST_STOP > AT_CLEANUP > > diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at > index 0fa7c4f29..6c3ab4618 100644 > --- a/tests/ovn-ic.at > +++ b/tests/ovn-ic.at > @@ -164,6 +164,56 @@ 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 > +sleep 6000 > nit: Leftover? > +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"]] > +]) > + > # 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.c b/utilities/ovn-ic-nbctl.c > index 50e975283..9c1635d81 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 NETWORK add a transit switch PORT\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,68 @@ 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; > + > + ctx->error = tsp_by_name_or_uuid(ctx, tsp_name, must_exist, &tsp); > + if (ctx->error) { > + return; > + } > + > + const struct icnbrec_transit_switch *ts = NULL; The tsp here can be NULL actually with --if-exists. > + char *error = tsp_to_ts(ctx, tsp, &ts); > + if (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 +900,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++) { > + 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 +1268,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 +1310,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 +1551,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 +1569,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 0ef207272..56cfdf5b4 100644 > --- a/utilities/ovn-nbctl.c > +++ b/utilities/ovn-nbctl.c > @@ -1482,50 +1482,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, > @@ -1550,8 +1506,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; > @@ -8835,8 +8791,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.52.0 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > We should also document the new commands in ovn-ic-nbctl.8.xml. It seems that we don't have any tests for tsp-del, it should be part of the ovn-ic-nbctl.at. Regards, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
