| Message ID | 20241112173122.3690702-1-arukomoinikova@k2.cloud |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v2] utilities: Added option to show separate chassis. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
Bleep bloop. Greetings Alexandra Rukomoinikova, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 81 characters long (recommended limit is 79) WARNING: Line lacks whitespace around operator #45 FILE: utilities/ovn-sbctl.c:94: show [CHASSIS | HOSTNAME | ENCAP-IP ...] print overview of database contents\n\ Lines checked: 200, Warnings: 2, Errors: 0 Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
On Tue, Nov 12, 2024 at 6:38 PM Alexandra Rukomoinikova <arukomoinikova@k2.cloud> wrote: > Added ability to output multiple individual chassis by their > name, hostname and encap-ip. > > Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud> > Tested-by: Ivan Burnin <iburnin@k2.cloud> > --- > v2: - removed all copy from ovs function > - removed hash for hostnames > --- > Hi Alexandra, thank you for the followup and sorry for the delay, I have a few minor comments that could be addressed during merge if maintainers agree. > tests/ovn-sbctl.at | 9 ++++ > utilities/ovn-sbctl.c | 109 +++++++++++++++++++++++++++++++++--------- > 2 files changed, 96 insertions(+), 22 deletions(-) > > diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at > index 19ac55c80..9743dacb8 100644 > --- a/tests/ovn-sbctl.at > +++ b/tests/ovn-sbctl.at > @@ -164,6 +164,15 @@ Chassis ch0 > Port_Binding vif0 > ]) > > +AT_CHECK([ovn-sbctl set Chassis ch0 hostname="ch0_host"]) > +AT_CHECK([ovn-sbctl chassis-add ch1 geneve 1.2.3.6]) > +AT_CHECK([ovn-sbctl chassis-add ch2 geneve 1.2.3.7]) > +AT_CHECK([ovn-sbctl show ch0_host ch1 1.2.3.7 | grep Chassis], [0], [dnl > +Chassis ch0 > +Chassis ch1 > +Chassis ch2 > +]) > + > uuid=$(ovn-sbctl --columns=_uuid list Chassis ch0 | cut -d ':' -f2 | tr > -d ' ') > AT_CHECK_UNQUOTED([ovn-sbctl --columns=logical_port,mac,chassis list > Port_Binding], [0], [dnl > logical_port : vif0 > diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c > index f1f8c2b42..e2ef37fa3 100644 > --- a/utilities/ovn-sbctl.c > +++ b/utilities/ovn-sbctl.c > @@ -91,7 +91,7 @@ sbctl_usage(void) > usage: %s [OPTIONS] COMMAND [ARG...]\n\ > \n\ > General commands:\n\ > - show print overview of database contents\n\ > + show [CHASSIS | HOSTNAME | ENCAP-IP ...] print overview of database > contents\n\ > As pointed out by checkpatch this line is longer than 79 characters. > \n\ > Chassis commands:\n\ > chassis-add CHASSIS ENCAP-TYPE ENCAP-IP create a new chassis named\n\ > @@ -355,12 +355,14 @@ static void > pre_get_info(struct ctl_context *ctx) > { > ovsdb_idl_add_column(ctx->idl, &sbrec_chassis_col_name); > + ovsdb_idl_add_column(ctx->idl, &sbrec_chassis_col_hostname); > ovsdb_idl_add_column(ctx->idl, &sbrec_chassis_col_encaps); > > ovsdb_idl_add_column(ctx->idl, &sbrec_chassis_private_col_name); > > ovsdb_idl_add_column(ctx->idl, &sbrec_encap_col_type); > ovsdb_idl_add_column(ctx->idl, &sbrec_encap_col_ip); > + ovsdb_idl_add_column(ctx->idl, &sbrec_encap_col_options); > > ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_logical_port); > ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_tunnel_key); > @@ -404,26 +406,6 @@ pre_get_info(struct ctl_context *ctx) > ovsdb_idl_add_column(ctx->idl, &sbrec_load_balancer_col_protocol); > } > > -static struct cmd_show_table cmd_show_tables[] = { > - {&sbrec_table_chassis, > - &sbrec_chassis_col_name, > - {&sbrec_chassis_col_hostname, > - &sbrec_chassis_col_encaps, > - NULL}, > - {&sbrec_table_port_binding, > - &sbrec_port_binding_col_logical_port, > - &sbrec_port_binding_col_chassis}}, > - > - {&sbrec_table_encap, > - &sbrec_encap_col_type, > - {&sbrec_encap_col_ip, > - &sbrec_encap_col_options, > - NULL}, > - {NULL, NULL, NULL}}, > - > - {NULL, NULL, {NULL, NULL, NULL}, {NULL, NULL, NULL}}, > -}; > - > static void > sbctl_init(struct ctl_context *ctx OVS_UNUSED) > { > @@ -564,6 +546,86 @@ cmd_lsp_unbind(struct ctl_context *ctx) > } > } > > +static void > +show_encap(struct ctl_context *ctx, > + const struct sbrec_encap *encap) > +{ > + ds_put_format(&ctx->output, " Encap %s\n", encap->type); > + ds_put_format(&ctx->output, " ip: \"%s\"\n", encap->ip); > + ds_put_format(&ctx->output, " options: {csum=\"%s\"}\n", > + smap_get_def(&encap->options, "csum", " ")); > +}; > + > +static void > +show_chassis(struct ctl_context *ctx, > + const struct sbrec_chassis *chassis) > +{ > + const struct sbrec_port_binding *pb; > + > + ds_put_format(&ctx->output, "Chassis %s\n", chassis->name); > + > + if (chassis->hostname && strlen(chassis->hostname)) { > No need for strlen, simple "chassis->hostname[0]" is sufficient. > + ds_put_format(&ctx->output, " hostname: %s\n", > chassis->hostname); > + } > + > + for (size_t i = 0; i < chassis->n_encaps; i++) { > + show_encap(ctx, chassis->encaps[i]); > + } > + > + SBREC_PORT_BINDING_FOR_EACH (pb, ctx->idl) { > + if (pb->chassis == chassis) { > + ds_put_format(&ctx->output, " Port_Binding %s\n", > + pb->logical_port); > + } > + } > +} > + > +static const struct sbrec_chassis * > +sbctl_find_chassis(struct ctl_context *ctx, const char *name) > +{ > + const struct sbctl_chassis *chassis = NULL; > + const struct sbrec_chassis *chassis_rec = NULL; > + > + /* Find chassis by his name. */ > + chassis = find_chassis(ctx, name, false); > + if (chassis) { > + return chassis->ch_cfg; > + } > + > + /* Find chassis by his hostname or encap ip. */ > + SBREC_CHASSIS_FOR_EACH (chassis_rec, ctx->idl) { > + if (chassis_rec->hostname && !strcmp(chassis_rec->hostname, > name)) { > + return chassis_rec; > + } > + for (size_t i = 0; i < chassis_rec->n_encaps; i++) { > + if (!strcmp(chassis_rec->encaps[i]->ip, name)) { > + return chassis_rec; > + } > + } > + } > + > + return NULL; > +} > + > +static void > +cmd_show(struct ctl_context *ctx) > +{ > + const struct sbrec_chassis *chassis; > + > + if (ctx->argc > 1) { > + for (int i = 1; i <= ctx->argc - 1; i++) { > + chassis = sbctl_find_chassis(ctx, ctx->argv[i]); > + if (chassis) { > + show_chassis(ctx, chassis); > + } > + } > + } else { > + SBREC_CHASSIS_FOR_EACH (chassis, ctx->idl) { > + show_chassis(ctx, chassis); > + } > + } > +} > + > enum { > PL_INGRESS, > PL_EGRESS, > @@ -1554,6 +1616,9 @@ static const struct ctl_table_class > tables[SBREC_N_TABLES] = { > static const struct ctl_command_syntax sbctl_commands[] = { > { "init", 0, 0, "", NULL, sbctl_init, NULL, "", RW }, > > + { "show", 0, INT_MAX, "[CHASSIS | HOSTNAME | ENCAP-IP ...]", > + pre_get_info, cmd_show, NULL, "", RO }, > + > /* Chassis commands. */ > {"chassis-add", 3, 3, "CHASSIS ENCAP-TYPE ENCAP-IP", pre_get_info, > cmd_chassis_add, NULL, "--may-exist", RW}, > @@ -1610,7 +1675,7 @@ main(int argc, char *argv[]) > > .idl_class = &sbrec_idl_class, > .tables = tables, > - .cmd_show_table = cmd_show_tables, > + .cmd_show_table = NULL, > .commands = sbctl_commands, > > .usage = sbctl_usage, > -- > 2.34.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > With those addressed: Acked-by: Ales Musil <amusil@redhat.com>
On 12/10/24 12:45 PM, Ales Musil wrote: > On Tue, Nov 12, 2024 at 6:38 PM Alexandra Rukomoinikova > <arukomoinikova@k2.cloud> wrote: > >> Added ability to output multiple individual chassis by their >> name, hostname and encap-ip. >> >> Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud> >> Tested-by: Ivan Burnin <iburnin@k2.cloud> >> --- >> v2: - removed all copy from ovs function >> - removed hash for hostnames >> --- >> > > Hi Alexandra, > > thank you for the followup and sorry for the delay, I have a few minor > comments that could be addressed during merge if maintainers agree. > Hi Alexandra, Ales, First of all, sorry for the delay in replying. > >> tests/ovn-sbctl.at | 9 ++++ >> utilities/ovn-sbctl.c | 109 +++++++++++++++++++++++++++++++++--------- >> 2 files changed, 96 insertions(+), 22 deletions(-) >> >> diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at >> index 19ac55c80..9743dacb8 100644 >> --- a/tests/ovn-sbctl.at >> +++ b/tests/ovn-sbctl.at >> @@ -164,6 +164,15 @@ Chassis ch0 >> Port_Binding vif0 >> ]) >> >> +AT_CHECK([ovn-sbctl set Chassis ch0 hostname="ch0_host"]) >> +AT_CHECK([ovn-sbctl chassis-add ch1 geneve 1.2.3.6]) >> +AT_CHECK([ovn-sbctl chassis-add ch2 geneve 1.2.3.7]) >> +AT_CHECK([ovn-sbctl show ch0_host ch1 1.2.3.7 | grep Chassis], [0], [dnl >> +Chassis ch0 >> +Chassis ch1 >> +Chassis ch2 >> +]) >> + >> uuid=$(ovn-sbctl --columns=_uuid list Chassis ch0 | cut -d ':' -f2 | tr >> -d ' ') >> AT_CHECK_UNQUOTED([ovn-sbctl --columns=logical_port,mac,chassis list >> Port_Binding], [0], [dnl >> logical_port : vif0 >> diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c >> index f1f8c2b42..e2ef37fa3 100644 >> --- a/utilities/ovn-sbctl.c >> +++ b/utilities/ovn-sbctl.c >> @@ -91,7 +91,7 @@ sbctl_usage(void) >> usage: %s [OPTIONS] COMMAND [ARG...]\n\ >> \n\ >> General commands:\n\ >> - show print overview of database contents\n\ >> + show [CHASSIS | HOSTNAME | ENCAP-IP ...] print overview of database >> contents\n\ >> > > As pointed out by checkpatch this line is longer than 79 characters. > > >> \n\ >> Chassis commands:\n\ >> chassis-add CHASSIS ENCAP-TYPE ENCAP-IP create a new chassis named\n\ >> @@ -355,12 +355,14 @@ static void >> pre_get_info(struct ctl_context *ctx) >> { >> ovsdb_idl_add_column(ctx->idl, &sbrec_chassis_col_name); >> + ovsdb_idl_add_column(ctx->idl, &sbrec_chassis_col_hostname); >> ovsdb_idl_add_column(ctx->idl, &sbrec_chassis_col_encaps); >> >> ovsdb_idl_add_column(ctx->idl, &sbrec_chassis_private_col_name); >> >> ovsdb_idl_add_column(ctx->idl, &sbrec_encap_col_type); >> ovsdb_idl_add_column(ctx->idl, &sbrec_encap_col_ip); >> + ovsdb_idl_add_column(ctx->idl, &sbrec_encap_col_options); >> >> ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_logical_port); >> ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_tunnel_key); >> @@ -404,26 +406,6 @@ pre_get_info(struct ctl_context *ctx) >> ovsdb_idl_add_column(ctx->idl, &sbrec_load_balancer_col_protocol); >> } >> >> -static struct cmd_show_table cmd_show_tables[] = { >> - {&sbrec_table_chassis, >> - &sbrec_chassis_col_name, >> - {&sbrec_chassis_col_hostname, >> - &sbrec_chassis_col_encaps, >> - NULL}, >> - {&sbrec_table_port_binding, >> - &sbrec_port_binding_col_logical_port, >> - &sbrec_port_binding_col_chassis}}, >> - >> - {&sbrec_table_encap, >> - &sbrec_encap_col_type, >> - {&sbrec_encap_col_ip, >> - &sbrec_encap_col_options, >> - NULL}, >> - {NULL, NULL, NULL}}, >> - >> - {NULL, NULL, {NULL, NULL, NULL}, {NULL, NULL, NULL}}, >> -}; >> - I'm not sure I like this too much unfortunately. It means we will have to write boilerplate code every time we want to add a new table to the "show" command. Isn't it a better option to enhance the OVS cmd_show() implementation [0] to accept an optional list of arguments to filter on? CC Ilya, in case he has ideas too. Thanks, Dumitru [0] https://github.com/openvswitch/ovs/blob/77ac0b28c868c724675b4004d554f5938ce13693/lib/db-ctl-base.c#L2548-L2554 >> static void >> sbctl_init(struct ctl_context *ctx OVS_UNUSED) >> { >> @@ -564,6 +546,86 @@ cmd_lsp_unbind(struct ctl_context *ctx) >> } >> } >> >> +static void >> +show_encap(struct ctl_context *ctx, >> + const struct sbrec_encap *encap) >> +{ >> + ds_put_format(&ctx->output, " Encap %s\n", encap->type); >> + ds_put_format(&ctx->output, " ip: \"%s\"\n", encap->ip); >> + ds_put_format(&ctx->output, " options: {csum=\"%s\"}\n", >> + smap_get_def(&encap->options, "csum", " ")); >> +}; >> + >> +static void >> +show_chassis(struct ctl_context *ctx, >> + const struct sbrec_chassis *chassis) >> +{ >> + const struct sbrec_port_binding *pb; >> + >> + ds_put_format(&ctx->output, "Chassis %s\n", chassis->name); >> + >> + if (chassis->hostname && strlen(chassis->hostname)) { >> > > No need for strlen, simple "chassis->hostname[0]" is sufficient. > > >> + ds_put_format(&ctx->output, " hostname: %s\n", >> chassis->hostname); >> + } >> + >> + for (size_t i = 0; i < chassis->n_encaps; i++) { >> + show_encap(ctx, chassis->encaps[i]); >> + } >> + >> + SBREC_PORT_BINDING_FOR_EACH (pb, ctx->idl) { >> + if (pb->chassis == chassis) { >> + ds_put_format(&ctx->output, " Port_Binding %s\n", >> + pb->logical_port); >> + } >> + } >> +} >> + >> +static const struct sbrec_chassis * >> +sbctl_find_chassis(struct ctl_context *ctx, const char *name) >> +{ >> + const struct sbctl_chassis *chassis = NULL; >> + const struct sbrec_chassis *chassis_rec = NULL; >> + >> + /* Find chassis by his name. */ >> + chassis = find_chassis(ctx, name, false); >> + if (chassis) { >> + return chassis->ch_cfg; >> + } >> + >> + /* Find chassis by his hostname or encap ip. */ >> + SBREC_CHASSIS_FOR_EACH (chassis_rec, ctx->idl) { >> + if (chassis_rec->hostname && !strcmp(chassis_rec->hostname, >> name)) { >> + return chassis_rec; >> + } >> + for (size_t i = 0; i < chassis_rec->n_encaps; i++) { >> + if (!strcmp(chassis_rec->encaps[i]->ip, name)) { >> + return chassis_rec; >> + } >> + } >> + } >> + >> + return NULL; >> +} >> + >> +static void >> +cmd_show(struct ctl_context *ctx) >> +{ >> + const struct sbrec_chassis *chassis; >> + >> + if (ctx->argc > 1) { >> + for (int i = 1; i <= ctx->argc - 1; i++) { >> + chassis = sbctl_find_chassis(ctx, ctx->argv[i]); >> + if (chassis) { >> + show_chassis(ctx, chassis); >> + } >> + } >> + } else { >> + SBREC_CHASSIS_FOR_EACH (chassis, ctx->idl) { >> + show_chassis(ctx, chassis); >> + } >> + } >> +} >> + >> enum { >> PL_INGRESS, >> PL_EGRESS, >> @@ -1554,6 +1616,9 @@ static const struct ctl_table_class >> tables[SBREC_N_TABLES] = { >> static const struct ctl_command_syntax sbctl_commands[] = { >> { "init", 0, 0, "", NULL, sbctl_init, NULL, "", RW }, >> >> + { "show", 0, INT_MAX, "[CHASSIS | HOSTNAME | ENCAP-IP ...]", >> + pre_get_info, cmd_show, NULL, "", RO }, >> + >> /* Chassis commands. */ >> {"chassis-add", 3, 3, "CHASSIS ENCAP-TYPE ENCAP-IP", pre_get_info, >> cmd_chassis_add, NULL, "--may-exist", RW}, >> @@ -1610,7 +1675,7 @@ main(int argc, char *argv[]) >> >> .idl_class = &sbrec_idl_class, >> .tables = tables, >> - .cmd_show_table = cmd_show_tables, >> + .cmd_show_table = NULL, >> .commands = sbctl_commands, >> >> .usage = sbctl_usage, >> -- >> 2.34.1 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> > With those addressed: > > Acked-by: Ales Musil <amusil@redhat.com> > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 12/11/24 12:49, Dumitru Ceara wrote: > On 12/10/24 12:45 PM, Ales Musil wrote: >> On Tue, Nov 12, 2024 at 6:38 PM Alexandra Rukomoinikova >> <arukomoinikova@k2.cloud> wrote: >> >>> Added ability to output multiple individual chassis by their >>> name, hostname and encap-ip. >>> >>> Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud> >>> Tested-by: Ivan Burnin <iburnin@k2.cloud> >>> --- >>> v2: - removed all copy from ovs function >>> - removed hash for hostnames >>> --- <snip> > > I'm not sure I like this too much unfortunately. It means we will have > to write boilerplate code every time we want to add a new table to the > "show" command. > > Isn't it a better option to enhance the OVS cmd_show() implementation > [0] to accept an optional list of arguments to filter on? > > CC Ilya, in case he has ideas too. Hi, Dumitru and Alexandra. I've been thinking abut what we can do from the generic db-ctl-base perspective. So, here is what I came up with: Today, we have multiple commands like find, list and show. They have different use cases and output formats. - list TABLE * Lists all database records in a specified table. * Does not expand references, i.e. just lists UUIDs. * Allows limiting the output to particular columns with --columns. - find TABLE CONDITION... * Same output format as in list. * Prints only records that satisfy all the provided conditions. * Allows limiting the output to particular columns with --columns. - show * Pretty-prints the table contents based on cmd_show_table array provided by the application. * Technically, still prints only one table, which is cmd_show_table[0], but expands the references printing referenced records from other tables recursively, if present in cmd_show_table. * Can also enrich the output with some other tables that hold references to the printed one. * Doesn't allow limiting the list of printed columns or filtering the output based on conditions dynamically (limiting is done statically via content of cmd_show_table). If I understand the purpose of this patch correctly: 1. We want to be able to find multiple records based on multiple conditions. 2. It should be possible to specify conditions as ANY as opposed to ALL, i.e. we want records that match at least one condition. 3. We want to find records based on conditions applied to a referenced record, e.g we want to print chassis if chassis->encaps->ip matches in any of referenced encaps. 4. We want to pretty-print the found records with all the benefits of the show command including the printing of records that hold references to the printed one, but not necessarily being referenced from it. We may be able to satisfy subsets or all the requirements above with some modifications to the current commands. Option 1: We may improve the condition syntax for the 'find' command allowing matching on referenced records and allowing ANY semantics of conditions. For example: ovn-sbctl --columns=name,hostname,encaps find chassis \ name=ch0 or hostname="ch0_host" and encaps{ref-any}ip=1.2.3.4 This will require some basic expression evaluation, but should not be hard to implement. Pros: A powerful tool for database searches, especially in scripts and tests. Cons: A little verbose, no pretty-printing, no printing of referenced tables. Might be a good thing to implement regardless of it being chosen for the currently discussed functionality in the patch. Option 2: We may try and add conditions to the 'show' command. Start with basic ones that 'find' already supports and extend to the condition expressions from the option 1. For example: ovn-sbctl show name=ch0 or hostname="ch0_host" and encaps{ref-any}ip=1.2.3.4 I'm not really sure about this one, because while it's clear from the code that 'show' only ever prints records from one table, it's not clear to the user. And this also only applies to the generic 'show'. ovn-nbctl, for example, implements a custom one that prints two tables and it's hard to write conditions in this case due to potential column name difference. The conditions may be extended to also specify the table name, but it will be even more verbose and may also be further less intuitive to use. I suspect, users of the 'show' command do not really know how the database schema looks like internally in most cases. It's not the command for power users. Pros: Pretty-printing. Cons: Confusing to use, interface is too verbose for an "overview" command. Option 3: We may add a generic text filter. For example: ovn-sbctl --columns=name,hostname,encaps --filter=ch0 list chassis ovn-sbctl --filter=ch0,1.2.3.4 show The first command would list all the chassis that have 'ch0' in a name or hostname or external id or any other column. The second will show only the chassis that have mentions of 'ch0' or '1.2.3.4' in a printed output for that chassis, i.e. if one of the port binding records has 'ch0' referenced, the chassis record that it references will be printed as well. This can be implemented as a post-filter, i.e. as a simple strstr() on what is going to be printed for the current record. With that implementation, the filter can also work for UUIDs, numbers and other things. Pros: Simple to use, can work for any command like list, find, show, should be easy to implement for custom 'show' commands as well, doesn't require knowing the schema or modifying the commands when the schema changes. Cons: Prone to false-positive matches if the user is not careful. I'm leaning towards options 1 and 3 as they seem to be generally useful and not actually mutually exclusive. Both may also work together with the future 'show-table' command (like a 'list' but expanding references like 'show' via dynamic generation of cmd_show_table) that Rosemarie worked on. What do you think? Best regards, Ilya Maximets.
diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at index 19ac55c80..9743dacb8 100644 --- a/tests/ovn-sbctl.at +++ b/tests/ovn-sbctl.at @@ -164,6 +164,15 @@ Chassis ch0 Port_Binding vif0 ]) +AT_CHECK([ovn-sbctl set Chassis ch0 hostname="ch0_host"]) +AT_CHECK([ovn-sbctl chassis-add ch1 geneve 1.2.3.6]) +AT_CHECK([ovn-sbctl chassis-add ch2 geneve 1.2.3.7]) +AT_CHECK([ovn-sbctl show ch0_host ch1 1.2.3.7 | grep Chassis], [0], [dnl +Chassis ch0 +Chassis ch1 +Chassis ch2 +]) + uuid=$(ovn-sbctl --columns=_uuid list Chassis ch0 | cut -d ':' -f2 | tr -d ' ') AT_CHECK_UNQUOTED([ovn-sbctl --columns=logical_port,mac,chassis list Port_Binding], [0], [dnl logical_port : vif0 diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c index f1f8c2b42..e2ef37fa3 100644 --- a/utilities/ovn-sbctl.c +++ b/utilities/ovn-sbctl.c @@ -91,7 +91,7 @@ sbctl_usage(void) usage: %s [OPTIONS] COMMAND [ARG...]\n\ \n\ General commands:\n\ - show print overview of database contents\n\ + show [CHASSIS | HOSTNAME | ENCAP-IP ...] print overview of database contents\n\ \n\ Chassis commands:\n\ chassis-add CHASSIS ENCAP-TYPE ENCAP-IP create a new chassis named\n\ @@ -355,12 +355,14 @@ static void pre_get_info(struct ctl_context *ctx) { ovsdb_idl_add_column(ctx->idl, &sbrec_chassis_col_name); + ovsdb_idl_add_column(ctx->idl, &sbrec_chassis_col_hostname); ovsdb_idl_add_column(ctx->idl, &sbrec_chassis_col_encaps); ovsdb_idl_add_column(ctx->idl, &sbrec_chassis_private_col_name); ovsdb_idl_add_column(ctx->idl, &sbrec_encap_col_type); ovsdb_idl_add_column(ctx->idl, &sbrec_encap_col_ip); + ovsdb_idl_add_column(ctx->idl, &sbrec_encap_col_options); ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_logical_port); ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_tunnel_key); @@ -404,26 +406,6 @@ pre_get_info(struct ctl_context *ctx) ovsdb_idl_add_column(ctx->idl, &sbrec_load_balancer_col_protocol); } -static struct cmd_show_table cmd_show_tables[] = { - {&sbrec_table_chassis, - &sbrec_chassis_col_name, - {&sbrec_chassis_col_hostname, - &sbrec_chassis_col_encaps, - NULL}, - {&sbrec_table_port_binding, - &sbrec_port_binding_col_logical_port, - &sbrec_port_binding_col_chassis}}, - - {&sbrec_table_encap, - &sbrec_encap_col_type, - {&sbrec_encap_col_ip, - &sbrec_encap_col_options, - NULL}, - {NULL, NULL, NULL}}, - - {NULL, NULL, {NULL, NULL, NULL}, {NULL, NULL, NULL}}, -}; - static void sbctl_init(struct ctl_context *ctx OVS_UNUSED) { @@ -564,6 +546,86 @@ cmd_lsp_unbind(struct ctl_context *ctx) } } +static void +show_encap(struct ctl_context *ctx, + const struct sbrec_encap *encap) +{ + ds_put_format(&ctx->output, " Encap %s\n", encap->type); + ds_put_format(&ctx->output, " ip: \"%s\"\n", encap->ip); + ds_put_format(&ctx->output, " options: {csum=\"%s\"}\n", + smap_get_def(&encap->options, "csum", " ")); +}; + +static void +show_chassis(struct ctl_context *ctx, + const struct sbrec_chassis *chassis) +{ + const struct sbrec_port_binding *pb; + + ds_put_format(&ctx->output, "Chassis %s\n", chassis->name); + + if (chassis->hostname && strlen(chassis->hostname)) { + ds_put_format(&ctx->output, " hostname: %s\n", chassis->hostname); + } + + for (size_t i = 0; i < chassis->n_encaps; i++) { + show_encap(ctx, chassis->encaps[i]); + } + + SBREC_PORT_BINDING_FOR_EACH (pb, ctx->idl) { + if (pb->chassis == chassis) { + ds_put_format(&ctx->output, " Port_Binding %s\n", + pb->logical_port); + } + } +} + +static const struct sbrec_chassis * +sbctl_find_chassis(struct ctl_context *ctx, const char *name) +{ + const struct sbctl_chassis *chassis = NULL; + const struct sbrec_chassis *chassis_rec = NULL; + + /* Find chassis by his name. */ + chassis = find_chassis(ctx, name, false); + if (chassis) { + return chassis->ch_cfg; + } + + /* Find chassis by his hostname or encap ip. */ + SBREC_CHASSIS_FOR_EACH (chassis_rec, ctx->idl) { + if (chassis_rec->hostname && !strcmp(chassis_rec->hostname, name)) { + return chassis_rec; + } + for (size_t i = 0; i < chassis_rec->n_encaps; i++) { + if (!strcmp(chassis_rec->encaps[i]->ip, name)) { + return chassis_rec; + } + } + } + + return NULL; +} + +static void +cmd_show(struct ctl_context *ctx) +{ + const struct sbrec_chassis *chassis; + + if (ctx->argc > 1) { + for (int i = 1; i <= ctx->argc - 1; i++) { + chassis = sbctl_find_chassis(ctx, ctx->argv[i]); + if (chassis) { + show_chassis(ctx, chassis); + } + } + } else { + SBREC_CHASSIS_FOR_EACH (chassis, ctx->idl) { + show_chassis(ctx, chassis); + } + } +} + enum { PL_INGRESS, PL_EGRESS, @@ -1554,6 +1616,9 @@ static const struct ctl_table_class tables[SBREC_N_TABLES] = { static const struct ctl_command_syntax sbctl_commands[] = { { "init", 0, 0, "", NULL, sbctl_init, NULL, "", RW }, + { "show", 0, INT_MAX, "[CHASSIS | HOSTNAME | ENCAP-IP ...]", + pre_get_info, cmd_show, NULL, "", RO }, + /* Chassis commands. */ {"chassis-add", 3, 3, "CHASSIS ENCAP-TYPE ENCAP-IP", pre_get_info, cmd_chassis_add, NULL, "--may-exist", RW}, @@ -1610,7 +1675,7 @@ main(int argc, char *argv[]) .idl_class = &sbrec_idl_class, .tables = tables, - .cmd_show_table = cmd_show_tables, + .cmd_show_table = NULL, .commands = sbctl_commands, .usage = sbctl_usage,