On 5/20/25 7:16 PM, Alexandra Rukomoinikova wrote:
> Added the ability to filter the output of the show command
> using the filter option.
>
> Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud>
> ---
> v1 --> v2 : did as Ilya said.
> ---
> lib/db-ctl-base.c | 75 +++++++++++++++++++++++++++++++++++++++++++++-
> tests/ovs-vsctl.at | 28 +++++++++++++++++
> 2 files changed, 102 insertions(+), 1 deletion(-)
Hi, Alexandra. Thanks for the new version!
nit: The 'lib:' in the patch name doesn't make a lot of sense as vast
majority of OVS code is in lib. It should be 'db-ctl-base:' instead.
Since this patch adds a new option, it needs to be documented in the
man page (utilities/ovs-vsctl.8.in) and a NEWS entry should be added.
Some more comments below.
>
> diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
> index 1f157e46c..45d2d51c0 100644
> --- a/lib/db-ctl-base.c
> +++ b/lib/db-ctl-base.c
> @@ -2243,19 +2243,92 @@ cmd_show_row(struct ctl_context *ctx, const struct
> ovsdb_idl_row *row,
> sset_find_and_delete_assert(shown, show->table->name);
> }
>
> +struct output_filter {
> + char **filters;
> + size_t n_filters;
> +};
> +
> +static void
> +filter_row(struct ctl_context *ctx,
> + struct output_filter *filter,
> + size_t old_lenght)
s/lenght/length/
here and in a few other places in the code.
> +{
> + bool filter_match = false;
> +
> + for (size_t i = 0; i < filter->n_filters; i++) {
> + if (strstr(&ctx->output.string[old_lenght], filter->filters[i])) {
> + filter_match = true;
break here. There is no need to continue the loop.
> + }
> + }
> +
> + if (!filter_match) {
> + ds_truncate(&ctx->output, old_lenght);
> + }
> +}
> +
> +static struct output_filter *
> +init_filters(struct ctl_context *ctx)
> +{
> + char *filter_str = shash_find_data(&ctx->options, "--filter");
> + if (!filter_str || !*filter_str) {
> + return NULL;
> + }
> +
> + size_t count = 1;
> + for (const char *p = filter_str; *p; p++) {
> + if (*p == '|') {
> + count++;
> + }
> + }
> +
> + struct output_filter *filter = xmalloc(sizeof(struct output_filter));
> + filter->filters = xmalloc(count * (sizeof(char *)));
> + filter->n_filters = count;
> +
> + char *ptr = NULL;
> + char *token = strtok_r(filter_str, "|", &ptr);
> + size_t idx = 0;
> +
> + while (token && idx < count) {
> + filter->filters[idx++] = xstrdup(token);
> + token = strtok_r(NULL, "|", &ptr);
> + }
> +
> + return filter;
> +}
If we use sset as a data structure for the filters, the code above can
be mostly replaced with an sset_from_delimited_string() call.
The sset is slightly less memory-efficient, but I don't think we care
in this particular case.
> +
> +static void
> +free_filters(struct output_filter *filter)
> +{
> + if (!filter) {
> + return;
> + }
> + for (size_t i = 0; i < filter->n_filters; i++) {
> + free(filter->filters[i]);
> + }
> + free(filter->filters);
> + free(filter);
> +}
> +
> static void
> cmd_show(struct ctl_context *ctx)
> {
> const struct ovsdb_idl_row *row;
> + struct output_filter *filter = init_filters(ctx);
> struct sset shown = SSET_INITIALIZER(&shown);
>
> for (row = ovsdb_idl_first_row(ctx->idl, cmd_show_tables[0].table);
> row; row = ovsdb_idl_next_row(row)) {
> + size_t lenght_before = ctx->output.length;
> cmd_show_row(ctx, row, 0, &shown);
> + if (filter) {
> + filter_row(ctx, filter, lenght_before);
> + }
This only covers the top-level elements, but the idea was to perform the
filtering recursively for every level. You should move the filtering into
the cmd_show_row() itself, i.e. save the length at the top of cmd_show_row()
and run the filtering at the bottom. This will ensure that we're filtering
all the way through.
For example, if we have a setup like this:
$ ovs-vsctl add-br br1
$ ovs-vsctl add-br br0
$ ovs-vsctl add-port br0 p1
$ ovs-vsctl add-port br0 tunnel_port \
-- set Interface tunnel_port type=geneve \
options:remote_ip=flow options:key=123
$ ovs-vsctl add-port br0 tunnel_port2 \
-- set Interface tunnel_port2 type=geneve \
options:remote_ip=flow options:key=125
Then I can get a list of bridges only with:
$ ovs-vsctl --filter="Bridge" show
d55f21cd-a59a-4548-b1bc-fc165b0ec0a0
Bridge br1
Bridge br0
Since the 'Bridge' match happens at the higher level, all the port
and interface information is filtered out.
Or a list of ports per bridge:
$ ovs-vsctl --filter="Port" show
d55f21cd-a59a-4548-b1bc-fc165b0ec0a0
Bridge br1
Port br1
Bridge br0
Port p1
Port tunnel_port
Port tunnel_port2
Port br0
Here only the interface information is filtered out since once we match
on the 'Port' string, all the higher-level nodes will match as well.
Or if I want to only show geneve interfaces:
$ ovs-vsctl --filter="geneve" show
d55f21cd-a59a-4548-b1bc-fc165b0ec0a0
Bridge br0
Port tunnel_port
Interface tunnel_port
type: geneve
options: {key="123", remote_ip=flow}
Port tunnel_port2
Interface tunnel_port2
type: geneve
options: {key="125", remote_ip=flow}
Or if I want to show all tunnels with the tunnel key 123:
$ ovs-vsctl --filter='key="123"' show
d55f21cd-a59a-4548-b1bc-fc165b0ec0a0
Bridge br0
Port tunnel_port
Interface tunnel_port
type: geneve
options: {key="123", remote_ip=flow}
Or I want the previous one, but also names of all the ports per bridge:
$ ovs-vsctl --filter='key="123"|Port' show
d55f21cd-a59a-4548-b1bc-fc165b0ec0a0
Bridge br1
Port br1
Bridge br0
Port p1
Port tunnel_port
Interface tunnel_port
type: geneve
options: {key="123", remote_ip=flow}
Port tunnel_port2
Port br0
What do you think?
> }
>
> ovs_assert(sset_is_empty(&shown));
> sset_destroy(&shown);
> + free_filters(filter);
> }
>
>
> @@ -2548,7 +2621,7 @@ ctl_init__(const struct ovsdb_idl_class *idl_class_,
> cmd_show_tables = cmd_show_tables_;
> if (cmd_show_tables) {
> static const struct ctl_command_syntax show =
> - {"show", 0, 0, "", pre_cmd_show, cmd_show, NULL, "", RO};
> + {"show", 0, 0, "", pre_cmd_show, cmd_show, NULL, "--filter=",
> RO};
> ctl_register_command(&show);
> }
> }
> diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
> index e488e292d..010889789 100644
> --- a/tests/ovs-vsctl.at
> +++ b/tests/ovs-vsctl.at
> @@ -1831,3 +1831,31 @@ AT_CHECK([ovs-vsctl --no-wait --bare --columns
> _uuid,name list bridge tst1], [0]
>
> OVS_VSCTL_CLEANUP
> AT_CLEANUP
> +
> +AT_SETUP([ovs-vsctl filter option usage])
nit: I'd name it 'ovs-vsctl show filtered' or something like that.
> +AT_KEYWORDS([ovs-vsctl filter option])
nit :I'd replace the 'option' with 'show'. 'option' is a not very specific
keyword to use.
> +
> +OVS_VSWITCHD_START([dnl
> + add-port br0 p1 -- set Interface p1 type=internal ofport_request=1 -- \
> + add-port br0 p2 -- set Interface p2 type=internal ofport_request=2 -- \
> + add-port br0 p3 -- set Interface p3 type=internal ofport_request=3
> +])
> +
> +AT_CHECK([ovs-vsctl --filter='p1' show | grep Port | sort], [0], [dnl
> + Port br0
> + Port p1
> + Port p2
> + Port p3
> +])
> +
> +AT_CHECK([ovs-vsctl --filter='test' show | grep Port | sort], [0], [])
> +
> +AT_CHECK([ovs-vsctl --filter='Bridge' show | grep Port | sort], [0], [dnl
> + Port br0
> + Port p1
> + Port p2
> + Port p3
> +])
We should print the whole thing here. It will not change from run to run,
because database structures are sorted and our top-level table only has one
row. The only thing that will change is the datapath uuid at the very top.
You may just cut out the first line with tail -n +2 or feed the output into
the uuidfilt.
Maybe add a bit more variety to the tests. Different interface types.
> +
> +OVS_VSCTL_CLEANUP
> +AT_CLEANUP
> \ No newline at end of file
Add the new line here!
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
