[ovs-dev,v3] db-ctl-base: Added filter option in show command.

The --filter option allows filtering output in two ways:
1. Basic filtering (comma-separated list, e.g., 'filter1,filter2').
2. Recursive filtering by table (e.g., 'interface(geneve)').

Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud>
---
 NEWS                     |   2 +-
 lib/db-ctl-base.c        | 162 +++++++++++++++++++++++++++++++++++++--
 tests/ovs-vsctl.at       |  42 ++++++++++
 utilities/ovs-vsctl.8.in |  15 ++++
 4 files changed, 214 insertions(+), 7 deletions(-)

On 6/9/25 1:31 PM, Alexandra Rukomoinikova wrote:
> The --filter option allows filtering output in two ways:
> 1. Basic filtering (comma-separated list, e.g., 'filter1,filter2').
> 2. Recursive filtering by table (e.g., 'interface(geneve)').
> 
> Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud>

Hi.  Thanks for the update!  Sorry for the delay (was traveling for DevConf).
See some comments below.

> ---
>  NEWS                     |   2 +-
>  lib/db-ctl-base.c        | 162 +++++++++++++++++++++++++++++++++++++--
>  tests/ovs-vsctl.at       |  42 ++++++++++
>  utilities/ovs-vsctl.8.in |  15 ++++
>  4 files changed, 214 insertions(+), 7 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 40c92c429..fc3bacb0c 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -23,7 +23,7 @@ Post-v3.5.0
>       the OVS distribution in the 3.0 release and is no longer present in
>       any supported versions of OVS. The remaining documentation of this
>       kernel module relates to topics for older releases of OVS.
> -
> +   - ovs-vsctl: Added new '--filter' option to the 'show' command.

We should keep 2 lines between major versions in the NEWS file.
You may also move this item up closer to ovs-appctl and move the
part after ':' to a separate line as an item.

>  
>  v3.5.0 - 17 Feb 2025
>  --------------------
> diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
> index 1f157e46c..c7edd640f 100644
> --- a/lib/db-ctl-base.c
> +++ b/lib/db-ctl-base.c
> @@ -2135,14 +2135,71 @@ cmd_show_weak_ref(struct ctl_context *ctx, const struct cmd_show_table *show,
>      }
>  }
>  
> +static bool
> +filter_output(struct ctl_context *ctx,
> +              const struct sset *filter_sset,
> +              size_t base_length)
> +{
> +    const char *output = &ctx->output.string[base_length];
> +    const char *value;
> +
> +    SSET_FOR_EACH (value, filter_sset) {
> +        if (strcasestr(output, value)) {
> +            return true;
> +        }
> +    }
> +
> +    ds_truncate(&ctx->output, base_length);
> +
> +    return false;
> +}
> +
> +static char * OVS_WARN_UNUSED_RESULT
> +filter_row(struct ctl_context *ctx,
> +           const struct cmd_show_table *show,
> +           const struct shash *row_filters,
> +           size_t base_length)
> +{
> +    if (shash_is_empty(row_filters)) {
> +        return NULL;
> +    }
> +
> +    const struct ovsdb_idl_table_class *tmp_table;
> +    struct shash_node *node;
> +    char *error;
> +    bool table_matched = false;
> +
> +    SHASH_FOR_EACH (node, row_filters) {
> +        error = get_table(node->name, &tmp_table);

We shouldn't iterate here.  We should lookup with shash_find_data instead.
The get_table() should be done at the init function and the real table name
used as a key in the shash.

It will also not be possible for the filter_row() to fail in this case,
which should simplify the callers a little bit.

> +        if (error) {
> +            return error;
> +        }
> +
> +        if (show && tmp_table == show->table) {
> +            table_matched = true;
> +            if (filter_output(ctx, node->data, base_length)) {
> +                return NULL;
> +            }
> +        }
> +    }
> +
> +    if (!table_matched) {
> +        ds_truncate(&ctx->output, base_length);
> +    }
> +
> +    return NULL;
> +}
> +
>  /* 'shown' records the tables that has been displayed by the current
>   * command to avoid duplicated prints.
>   */
> -static void
> +static bool
>  cmd_show_row(struct ctl_context *ctx, const struct ovsdb_idl_row *row,
> -             int level, struct sset *shown)
> +             int level, struct sset *shown, struct shash *row_filters)
>  {
>      const struct cmd_show_table *show = cmd_show_find_table_by_row(row);
> +    size_t start_pos = ctx->output.length;
> +    bool has_matching_child = false;
>      size_t i;
>  
>      ds_put_char_multiple(&ctx->output, ' ', level * 4);
> @@ -2158,7 +2215,7 @@ cmd_show_row(struct ctl_context *ctx, const struct ovsdb_idl_row *row,
>      ds_put_char(&ctx->output, '\n');
>  
>      if (!show || sset_find(shown, show->table->name)) {
> -        return;
> +        goto filter_and_return;
>      }
>  
>      sset_add(shown, show->table->name);
> @@ -2186,7 +2243,9 @@ cmd_show_row(struct ctl_context *ctx, const struct ovsdb_idl_row *row,
>                                                           ref_show->table,
>                                                           &datum->keys[j].uuid);
>                      if (ref_row) {
> -                        cmd_show_row(ctx, ref_row, level + 1, shown);
> +                        bool matched = cmd_show_row(ctx, ref_row, level + 1,
> +                                                    shown, row_filters);
> +                        has_matching_child = has_matching_child || matched;
>                      }
>                  }
>                  continue;
> @@ -2241,6 +2300,81 @@ cmd_show_row(struct ctl_context *ctx, const struct ovsdb_idl_row *row,
>      }
>      cmd_show_weak_ref(ctx, show, row, level);
>      sset_find_and_delete_assert(shown, show->table->name);
> +
> +filter_and_return:
> +    if (has_matching_child) {
> +        return true;
> +    }
> +
> +    char *error = filter_row(ctx, show, row_filters, start_pos);
> +    if (error) {
> +        ctx->error = error;
> +        return false;
> +    }
> +
> +    return ctx->output.length > start_pos;
> +}
> +
> +
> +

1 empty line is enough.

> +static char * OVS_WARN_UNUSED_RESULT
> +init_filters(const char *filter_str,
> +             struct sset *table_filters,
> +             struct shash *row_filters)
> +{
> +    struct sset all_filters = SSET_INITIALIZER(&all_filters);
> +    sset_from_delimited_string(&all_filters, filter_str, ",");
> +
> +    const char *item;
> +    SSET_FOR_EACH (item, &all_filters) {
> +        const char *ptr = strchr(item, '(');
> +
> +        if (ptr && item[strlen(item) - 1] == ')') {
> +            char table[64];
> +            char values[256];
> +
> +            if (sscanf(item, "%63[^()](%255[^)])", table, values) == 2) {

The length limit here is sort of arbitrary, we can use xmemdup0 to get
the table name and the values, since we know exactly where they are inside
the item.  sscanf will have some stricter checks, but I'm not sure if we
need to actually perform those.

> +                struct sset *value_set = shash_find_data(row_filters, table);
> +                if (!value_set) {
> +                    value_set = xmalloc(sizeof *value_set);
> +                    sset_init(value_set);
> +                    shash_add(row_filters, table, value_set);
> +                }

As per the comment for the filter_row function, here we should instead get
the table and the lookup the real table->name, e.g.

  char *table_name = xmemdup0(item, ptr - item);
  const struct ovsdb_idl_table_class *table;

  error = get_table(table_name, &table);
  free(table_name);

  if (error) {
      return error;
  }

  struct sset *value_set = shash_find_data(row_filters, table->name);

And use table->name moving forward in this function.  This way we will
be able to lookup the show->table->name in the shash directly without
iterating over it.

> +
> +                struct sset parsed_values = SSET_INITIALIZER(&parsed_values);
> +                sset_from_delimited_string(&parsed_values, values, "|");
> +
> +                const char *value;
> +                SSET_FOR_EACH (value, &parsed_values) {
> +                    sset_add(value_set, value);
> +                }
> +
> +                sset_destroy(&parsed_values);
> +            }
> +        } else if (ptr && item[strlen(item) - 1] != ')') {
> +            return xasprintf("Warning: malformed filter "
> +                             "(missing closing ')'): %s\n", item);

The command will be aborted, so this isn't really a warning, it's an error.

> +        } else {
> +            sset_add(table_filters, item);
> +        }
> +    }
> +
> +    sset_destroy(&all_filters);
> +
> +    return NULL;
> +}
> +
> +static void
> +destroy_filters(struct sset *table_filters,
> +                struct shash *row_filters)
> +{
> +    struct shash_node *node;
> +    SHASH_FOR_EACH (node, row_filters) {
> +        sset_destroy(node->data);
> +        free(node->data);
> +    }
> +    shash_destroy(row_filters);
> +    sset_destroy(table_filters);
>  }
>  
>  static void
> @@ -2248,12 +2382,28 @@ cmd_show(struct ctl_context *ctx)
>  {
>      const struct ovsdb_idl_row *row;
>      struct sset shown = SSET_INITIALIZER(&shown);
> +    struct sset table_filters = SSET_INITIALIZER(&table_filters);
> +    struct shash row_filters = SHASH_INITIALIZER(&row_filters);
> +
> +    char *filter_str = shash_find_data(&ctx->options, "--filter");
> +    if (filter_str && *filter_str) {
> +        char *error = init_filters(filter_str, &table_filters, &row_filters);
> +        if (error) {
> +            ctx->error = error;
> +            return;
> +        }
> +    }
>  
>      for (row = ovsdb_idl_first_row(ctx->idl, cmd_show_tables[0].table);
>           row; row = ovsdb_idl_next_row(row)) {
> -        cmd_show_row(ctx, row, 0, &shown);
> +        size_t length_before = ctx->output.length;
> +        cmd_show_row(ctx, row, 0, &shown, &row_filters);
> +        if (!sset_is_empty(&table_filters)) {
> +            filter_output(ctx, &table_filters, length_before);
> +        }

We may avoid calling the filters here if we put the table_filters into
row_filters shash under the key cmd_show_tables[0].table->name, right?

Your init_filters function may be rearranged somewhat like this
(completely untested, just a sketch):

static char * OVS_WARN_UNUSED_RESULT
init_filters(const char *filter_str,
             const struct ovsdb_idl_table_class *top_table,
             struct shash *filters)
{
    struct sset all_filters = SSET_INITIALIZER(&all_filters);

    sset_from_delimited_string(&all_filters, filter_str, ",");

    const char *item;
    SSET_FOR_EACH (item, &all_filters) {
        const struct ovsdb_idl_table_class *table = top_table;
        const char *ptr = strchr(item, '(');
        size_t len = strlen(item);
        char *values = NULL;

        if (ptr && item[len - 1] == ')') {
            char *table_name = xmemdup0(item, ptr - item);

            error = get_table(table_name, &table);
            free(table_name);

            if (error) {
                return error;
            }

            values = xmemdup0(ptr + 1, len - (ptr - item + 2));
        } else if (ptr && item[len - 1] != ')') {
            return xasprintf("Malformed filter (missing closing ')'): %s\n",
                             item);
        } else {
            values = xmemdup0(item, len);
        }

        struct sset parsed_values = SSET_INITIALIZER(&parsed_values);

        sset_from_delimited_string(&parsed_values, values, "|");
        free(values)

        struct sset *value_set = shash_find_data(filters, table->name);

        if (!value_set) {
            shash_add(filters, table->name, parsed_values);
        } else {
            const char *value;
            SSET_FOR_EACH (value, &parsed_values) {
                sset_add(value_set, value);
            }
            sset_destroy(parsed_values);
        }
    }

    sset_destroy(&all_filters);

    return NULL;
}

And then called with:

    struct shash filters = SHASH_INITIALIZER(&filters);

    char *error = init_filters(filter_str,
                               cmd_show_tables[0].table, &filters);

This way there are no really two types of filters, just one, but if
the table name is not specified, then the filter is assigned to the
top level table by default.  Will that work for your use case?

>      }
>  
> +    destroy_filters(&table_filters, &row_filters);
>      ovs_assert(sset_is_empty(&shown));
>      sset_destroy(&shown);
>  }
> @@ -2548,7 +2698,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..133dd6691 100644
> --- a/tests/ovs-vsctl.at
> +++ b/tests/ovs-vsctl.at
> @@ -1831,3 +1831,45 @@ 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])
> +AT_KEYWORDS([ovs-vsctl filter option])
> +
> +OVS_VSWITCHD_START([dnl
> +   add-port br0 p1 -- set Interface p1 type=internal ofport_request=1 -- \
> +   add-port br0 tunnel_port \
> +    -- set Interface tunnel_port type=geneve \
> +           options:remote_ip=1.2.3.4 options:key=123 -- \
> +   add-port br0 tunnel_port2 \
> +    -- set Interface tunnel_port2 type=geneve \
> +           options:remote_ip=1.2.3.5 options:key=125
> +])
> +
> +AT_CHECK([ovs-vsctl --filter='interface(1.2.3.5)' show | uuidfilt], [0], [dnl
> +<0>
> +    Bridge br0
> +        fail_mode: secure
> +        datapath_type: dummy
> +        Port tunnel_port2
> +            Interface tunnel_port2
> +                type: geneve
> +                options: {key="125", remote_ip="1.2.3.5"}
> +])
> +
> +AT_CHECK([ovs-vsctl --filter='port(p1)' show | uuidfilt], [0], [dnl
> +<0>
> +    Bridge br0
> +        fail_mode: secure
> +        datapath_type: dummy
> +        Port p1
> +])
> +
> +AT_CHECK(
> +  [ovs-vsctl --filter="interface(geneve)" show | uuidfilt | grep "options:" | sort],
> +  [0],
> +  [dnl
> +                options: {key="123", remote_ip="1.2.3.4"}
> +                options: {key="125", remote_ip="1.2.3.5"}
> +])
> +

Would be great to have some more complex cases as well, e.g. more than one
filter, just to make sure that the parsing code is working as expected.
Maybe also a negative case with an incorrectly formatted filter to get an
error.

> +AT_CLEANUP
> diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
> index 575b7c0bf..e08b6c15c 100644
> --- a/utilities/ovs-vsctl.8.in
> +++ b/utilities/ovs-vsctl.8.in
> @@ -152,6 +152,21 @@ initialize the database without executing any other command.
>  .
>  .IP "\fBshow\fR"
>  Prints a brief overview of the database contents.
> +.br
> +.br
> +\fBshow\fR [\fB--filter\fR=\fIfilters\fR]

The filter goes before the 'show'.  Also, this should be merged with the
previous description of the 'show' command.

> +    Prints a brief overview of the database contents.
> +    The \fB--filter\fR option allows you to filter the output in two ways:

Remove the 'you'.  Generally speaking, documentation supposed to describe
things and not tell things to the reader.

> +.br
> +    1. Basic filtering: Comma-separated list of filters to apply to individual records.
> +       Example: \fBovs-vsctl --filter='filter1,filter2' show\fR

We should mention somewhere that the filter is a sub-string to look for in
the printed output for a row or table in this case.

> +.br
> +    2. Recursive filtering: Filter output by specifying a table and optional conditions.
> +       The syntax is \fItable\fR(\fIconditions\fR) where conditions can include nested filters.
> +       Example: \fBovs-vsctl --filter='interface(geneve)' show\fR
> +       This will recursively show only elements related to Geneve interfaces.
> +.br

And we shouldn't force formatting this way, for a list you may use something
similar to, e.g., syslog-method documentation in lib/vlog.man:

.IP "synopsys"
Description.  Options are:
.RS
.IP \(bu
\fBFirst\fR: ....
.IP \(bu
\fBSecond\fR: ....
.RE
.IP

Though with the code suggestions above we may not need to document two
separate types of filters.

Best regards, Ilya Maximets.