Hi Ales

Thanks for the patch
Three small nits below

Acked-by: Xavier Simonart <[email protected]>
Thanks
Xavier

On Tue, Oct 21, 2025 at 5:09 PM Ales Musil via dev <[email protected]>
wrote:

> Add a new command that will create localnet port on specified LS.
> The localnet port will have proper type, network_name and addresses
> set.
>
> Add a new command that will create router port on specified LS.
> The router port will have proper type, router-port and addresses
> set.
>
> Reported-at: https://issues.redhat.com/browse/FDP-1838
> Reported-at: https://issues.redhat.com/browse/FDP-1839
> Suggested-by: Dumitru Ceara <[email protected]>
> Signed-off-by: Ales Musil <[email protected]>
>
> ovn-nbctl: Add new command called lsp-add-localnet.
>
> Add a new command that will create localnet port on specified LS.
> The localnet port will have proper type, network_name and addresses
> set.
>
> Reported-at: https://issues.redhat.com/browse/FDP-1839
> Signed-off-by: Ales Musil <[email protected]>
> ---
>  NEWS                  |   4 ++
>  tests/ovn-nbctl.at    |  92 ++++++++++++++++++++++++++++++++++++
>  utilities/ovn-nbctl.c | 106 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 202 insertions(+)
>
> diff --git a/NEWS b/NEWS
> index fb4d63194..3ef43bca3 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -44,6 +44,10 @@ Post v25.09.0
>         function and the chassis hosting the port where the ACL is being
>         enforced). Proper MTU needs to be configured to accomodate this
>         encapsulation.
> +   - Add ovn-nbctl ls-add-router-port which will create router port on
> +     specified LS.
> +   - Add ovn-nbctl ls-add-localnet-port which will create localnet port on
> +     specified LS.
>
>  OVN v25.09.0 - xxx xx xxxx
>  --------------------------
> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> index 266789417..8cd9919a6 100644
> --- a/tests/ovn-nbctl.at
> +++ b/tests/ovn-nbctl.at
> @@ -3335,3 +3335,95 @@ AT_CHECK([ovn-nbctl nf-list | uuidfilt], [0], [dnl
>  AT_CHECK([ovn-nbctl nf-del nf2])
>  AT_CHECK([ovn-nbctl nf-list | uuidfilt], [0], [])
>  ])
> +
> +dnl ---------------------------------------------------------------------
> +
> +OVN_NBCTL_TEST([ls_add_router_port], [Add router port], [
> +AT_CHECK([ovn-nbctl ls-add-router-port ls ls-lr lr-ls], [1], [],
> +  [ovn-nbctl: ls: switch name not found
> +])
> +
> +check ovn-nbctl ls-add ls
> +ovn-nbctl ls-add-router-port ls ls-lr lr-ls
> +check_row_count nb:Logical_Switch_Port 1 addresses=[[router]]
> options:router-port="lr-ls"
> +AT_CHECK([ovn-nbctl show | uuidfilt], [0], [dnl
> +switch <0> (ls)
> +    port ls-lr
> +        type: router
> +        router-port: lr-ls
> +])
> +
> +check ovn-nbctl lsp-del ls-lr
> +
> +check ovn-nbctl ls-add ls1/
> +check ovn-nbctl lsp-add ls1 ls-lr
> +
> +AT_CHECK([ovn-nbctl ls-add-router-port ls ls-lr lr-ls], [1], [],
> +  [ovn-nbctl: ls-lr: a port with this name already exists
> +])
> +
> +AT_CHECK([ovn-nbctl --may-exist ls-add-router-port ls ls-lr lr-ls], [1],
> [],
> +  [ovn-nbctl: ls-lr: a port already exists but in switch ls1
> +])
> +
> +check ovn-nbctl lsp-del ls-lr
> +check ovn-nbctl lsp-add ls ls-lr -- lsp-set-options ls-lr
> router-port=lr-ls2
> +
> +AT_CHECK([ovn-nbctl --may-exist ls-add-router-port ls ls-lr lr-ls], [1],
> [],
> +  [ovn-nbctl: ls-lr: a port already exists but with different type ""
> +])
> +
> +check ovn-nbctl lsp-set-type ls-lr router
> +AT_CHECK([ovn-nbctl --may-exist ls-add-router-port ls ls-lr lr-ls], [1],
> [],
> +  [ovn-nbctl: ls-lr: a port already exists but with different router-port
> "lr-ls2"
> +])
> +
> +check ovn-nbctl lsp-set-options ls-lr router-port=lr-ls
> +check ovn-nbctl --may-exist ls-add-router-port ls ls-lr lr-ls
> +])
> +
> +dnl ---------------------------------------------------------------------
> +
> +OVN_NBCTL_TEST([ls_add_localnet_port], [Add localnet port], [
> +AT_CHECK([ovn-nbctl ls-add-localnet-port ls ln_port net1], [1], [],
> +  [ovn-nbctl: ls: switch name not found
> +])
> +
> +check ovn-nbctl ls-add ls
> +check ovn-nbctl ls-add-localnet-port ls ln_port net1
> +check_row_count nb:Logical_Switch_Port 1 addresses=[[unknown]]
> options:network_name="net1"
> +AT_CHECK([ovn-nbctl show | uuidfilt], [0], [dnl
> +switch <0> (ls)
> +    port ln_port
> +        type: localnet
> +        addresses: [["unknown"]]
> +])
> +
> +check ovn-nbctl lsp-del ln_port
> +
> +check ovn-nbctl ls-add ls1
> +check ovn-nbctl lsp-add ls1 ln_port
> +
> +AT_CHECK([ovn-nbctl ls-add-localnet-port ls ln_port net1], [1], [],
> +  [ovn-nbctl: ln_port: a port with this name already exists
> +])
> +
> +AT_CHECK([ovn-nbctl --may-exist ls-add-localnet-port ls ln_port net1],
> [1], [],
> +  [ovn-nbctl: ln_port: a port already exists but in switch ls1
> +])
> +
> +check ovn-nbctl lsp-del ln_port
> +check ovn-nbctl lsp-add ls ln_port -- lsp-set-options ln_port
> network_name=net2
> +
> +AT_CHECK([ovn-nbctl --may-exist ls-add-localnet-port ls ln_port net1],
> [1], [],
> +  [ovn-nbctl: ln_port: a port already exists but with different type ""
> +])
> +
> +check ovn-nbctl lsp-set-type ln_port localnet
> +AT_CHECK([ovn-nbctl --may-exist ls-add-localnet-port ls ln_port net1],
> [1], [],
> +  [ovn-nbctl: ln_port: a port already exists but with different
> network_name "net2"
> +])
> +
> +check ovn-nbctl lsp-set-options ln_port network_name=net1
> +check ovn-nbctl --may-exist ls-add-localnet-port ls ln_port net1
> +])
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index 058ad8968..7d82bb9bd 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -538,6 +538,15 @@ MAC_Binding commands:\n\
>                                      Delete Static_MAC_Binding entry\n\
>    static-mac-binding-list           List all Static_MAC_Binding entries\n\
>  \n\
> +\n\
> +Miscellaneous commands:\n\
>
nit: Should they not be just listed after other logical switch commands ?
And should there be lsp-... instead of ls-...? Looks like you hesitated as
well (see second type below) :-)

> +  ls-add-route-port LS PORT LRP_PEER\n\
>
nit: typo: s/ls-add-route-port/ls-add-router-port/

> +                                    Create LSP of type router with\n\
> +                                    router-port set to LRP_PEER\n\
> +  lsp-add-localnet-port LS PORT NETWORK\n\
>
nit: typo: s/lsp-add/ls-add/

> +                                    Create LSP of type localnet with\n\
> +                                    network_name set to NETWORK\n\
> +\n\
>  %s\
>  %s\
>  \n\
> @@ -8680,6 +8689,95 @@ nbctl_mirror_list(struct ctl_context *ctx)
>      vector_destroy(&mirrors);
>  }
>
> +static void
> +nbctl_pre_ls_add_misc_port(struct ctl_context *ctx)
> +{
> +    nbctl_pre_context(ctx);
> +
> +    ovsdb_idl_add_column(ctx->idl, &nbrec_logical_switch_port_col_type);
> +    ovsdb_idl_add_column(ctx->idl,
> &nbrec_logical_switch_port_col_options);
> +    ovsdb_idl_add_column(ctx->idl,
> &nbrec_logical_switch_port_col_addresses);
> +}
> +
> +static void
> +nbctl_ls_add_misc_port(struct ctl_context *ctx)
> +{
> +    struct nbctl_context *nbctx = nbctl_context_get(ctx);
> +
> +    bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL;
> +    bool router_port = !strcmp(ctx->argv[0], "ls-add-router-port");
> +    const char *ls_name = ctx->argv[1];
> +    const char *lsp_name = ctx->argv[2];
> +    const char *option = ctx->argv[3];
> +    const char *type = router_port ? "router" : "localnet";
> +    const char *option_name = router_port ? "router-port" :
> "network_name";
> +
> +    const struct nbrec_logical_switch *ls;
> +    ctx->error = ls_by_name_or_uuid(ctx, ls_name, true, &ls);
> +    if (ctx->error) {
> +        return;
> +    }
> +
> +    const struct nbrec_logical_switch_port *lsp;
> +    ctx->error = lsp_by_name_or_uuid(ctx, lsp_name, false, &lsp);
> +    if (ctx->error) {
> +        return;
> +    }
> +
> +    if (lsp) {
> +        if (!may_exist) {
> +            ctl_error(ctx, "%s: a port with this name already exists",
> +                      lsp_name);
> +            return;
> +        }
> +
> +        const struct nbrec_logical_switch *lsw;
> +        ctx->error = lsp_to_ls(ctx, lsp, &lsw);
> +        if (ctx->error) {
> +            return;
> +        }
> +
> +        if (lsw != ls) {
> +            char uuid_s[UUID_LEN + 1];
> +            ctl_error(ctx, "%s: a port already exists but in switch %s",
> +                      lsp_name, ls_get_name(lsw, uuid_s, sizeof uuid_s));
> +            return;
> +        }
> +
> +        if (strcmp(lsp->type, type)) {
> +            ctl_error(ctx, "%s: a port already exists but with different"
> +                      " type \"%s\"", lsp_name, lsp->type);
> +            return;
> +        }
> +
> +        const char *lsp_option =
> +            smap_get_def(&lsp->options, option_name, "");
> +        if (strcmp(lsp_option, option)) {
> +            ctl_error(ctx, "%s: a port already exists but with different"
> +                      " %s \"%s\"", lsp_name, option_name, lsp_option);
> +            return;
> +        }
> +
> +        return;
> +    }
> +
> +    lsp = nbrec_logical_switch_port_insert(ctx->txn);
> +    nbrec_logical_switch_port_set_name(lsp, lsp_name);
> +    nbrec_logical_switch_port_set_type(lsp, type);
> +
> +    const char *addresses[] = { router_port ? "router": "unknown" };
> +    nbrec_logical_switch_port_set_addresses(lsp, addresses, 1);
> +
> +    struct smap options = SMAP_CONST1(&options, option_name, option);
> +    nbrec_logical_switch_port_set_options(lsp, &options);
> +
> +    /* Insert the logical port into the logical switch. */
> +    nbrec_logical_switch_update_ports_addvalue(ls, lsp);
> +
> +    /* Updating runtime cache. */
> +    shash_add(&nbctx->lsp_to_ls_map, lsp_name, ls);
> +}
> +
>  static const struct ctl_table_class tables[NBREC_N_TABLES] = {
>      [NBREC_TABLE_DHCP_OPTIONS].row_ids
>      = {{&nbrec_logical_switch_port_col_name, NULL,
> @@ -9061,6 +9159,14 @@ static const struct ctl_command_syntax
> nbctl_commands[] = {
>        nbctl_pre_static_mac_binding, nbctl_static_mac_binding_list, NULL,
>        "", RO },
>
> +    /* Misc command */
> +    { "ls-add-router-port", 3, 3, "LOGICAL_SWITCH PORT LRP_PEER",
> +      nbctl_pre_ls_add_misc_port, nbctl_ls_add_misc_port, NULL,
> +      "--may-exist", RW },
> +    { "ls-add-localnet-port", 3, 3, "LOGICAL_SWITCH PORT NETWORK",
> +      nbctl_pre_ls_add_misc_port, nbctl_ls_add_misc_port, NULL,
> +      "--may-exist", RW },
> +
>      {NULL, 0, 0, NULL, NULL, NULL, NULL, "", RO},
>  };
>
> --
> 2.51.0
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to