On Thu, Oct 23, 2025 at 5:12 PM Xavier Simonart <[email protected]> wrote:
> Hi Ales > > Thanks for the patch > Three small nits below > > Acked-by: Xavier Simonart <[email protected]> > Thanks > Xavier > Hi Xaiver, thank you for the review. > > 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) :-) > You are right, I had my inner fight about what it should be called. I don't have a strong preference now that you mentioned I'm more inclined towards lsp-* :) I'll give others a bit of time to express their opinions about the naming before merging it. + 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 >> >> Regard, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
