On 6/27/25 2:48 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 <[email protected]>
> ---
> v3 --> v4: fixed NEWS
> moved the logic of finding the table to the
> initialization of filters
> removed two different types of filters, changed
> it to common storage in hashmap
> ---
> NEWS | 3 +-
> lib/db-ctl-base.c | 197 +++++++++++++++++++++++++++++++++++++--
> tests/ovs-vsctl.at | 49 ++++++++++
> utilities/ovs-vsctl.8.in | 25 +++++
> 4 files changed, 267 insertions(+), 7 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index d7231fabc..b824f3e65 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -15,6 +15,8 @@ Post-v3.5.0
> core file size.
> - ovs-appctl:
> * Added JSON output support to the 'ovs/route/show' command.
> + - ovs-vsctl:
> + * Added '--filter' option to the 'show' command.
> - SSL/TLS:
> * Support for deprecated TLSv1 and TLSv1.1 protocols on OpenFlow and
> database connections is now removed.
> @@ -30,7 +32,6 @@ Post-v3.5.0
> any supported versions of OVS. The remaining documentation of this
> kernel module relates to topics for older releases of OVS.
>
> -
> v3.5.0 - 17 Feb 2025
> --------------------
> - The limit on the number of fields for address prefix tracking in flow
> diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
> index 5b741b1d4..4bf990ec6 100644
> --- a/lib/db-ctl-base.c
> +++ b/lib/db-ctl-base.c
> @@ -2141,14 +2141,79 @@ 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 void
> +wildcard_filter(struct ctl_context *ctx,
> + const struct shash *filters,
> + size_t base_length)
> +{
> + if (shash_is_empty(filters)) {
> + return;
> + }
> +
> + struct sset *wildcard_filter = shash_find_data(filters, "");
> +
> + if (wildcard_filter) {
> + filter_output(ctx, wildcard_filter, base_length);
> + }
> +}
> +
> +static void
> +table_filter(struct ctl_context *ctx,
> + const struct cmd_show_table *show,
> + const struct shash *filters,
> + size_t base_length)
> +{
> + if (shash_is_empty(filters)) {
> + return;
> + }
> +
> + const char *table_name =
> + show && show->table ? show->table->name : NULL;
> + struct sset *table_filter =
> + table_name ? shash_find_data(filters, table_name) : NULL;
> +
> + if (table_filter && filter_output(ctx, table_filter, base_length)) {
> + return;
> + }
> +
> + struct shash_node *node;
> + SHASH_FOR_EACH (node, filters) {
> + if (node->name[0] != '\0') {
> + ds_truncate(&ctx->output, base_length);
> + break;
> + }
> + }
Hmm, what is this loop for?
> +}
> +
> /* '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 *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);
> @@ -2164,7 +2229,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;
> }
>
> sset_add(shown, show->table->name);
> @@ -2192,7 +2257,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, filters);
> + has_matching_child |= matched;
We should not use bit operations on booleans, because numerical representation
varies between systems. C doesn't have ||= operation, so this should be A = A
|| B,
or just if (cmd_show_row(...)) { has_matching_child = true; }. May also
collapse
the two if conditions into one in this case, i.e. if (ref_row &&
cmd_show_row(...)).
But, also, do we actually need this has_matching_child check? See below.
> }
> }
> continue;
> @@ -2247,6 +2314,111 @@ 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:
> + if (has_matching_child) {
> + if (!level) {
> + wildcard_filter(ctx, filters, start_pos);
> + }
Why these filters are applied only if there was a matching child?
> + return true;
This makes us not apply the filter for the current table, which is probably not
what we want. E.g. if I do this:
ovs-vsctl add-br br0
ovs-vsctl add-br br1
ovs-vsctl add-port br0 tunnel_port -- set Interface tunnel_port type=geneve \
options:remote_ip=flow options:key=123 options:remote_cert=abc3
ovs-vsctl add-port br0 tunnel_port2 -- set Interface tunnel_port2 type=geneve \
options:remote_ip=flow options:key=125 options:remote_cert=qwe2
ovs-vsctl add-port br0 p1
I can filter on the tunnel key on the interface level:
$ ovs-vsctl --filter='interface(key="123")' show
96630246-b431-408e-9882-6e683471fa48
Bridge br0
Port tunnel_port
Interface tunnel_port
type: geneve
options: {key="123", remote_cert=abc3, remote_ip=flow}
But I can't do the same on the Port level:
$ ovs-vsctl --filter='port(key="123")' show
<no output>
If I get red of the has_matching_child check, the loop in the table_filter(),
then I can filter by the tunnel key on any level I want:
$ ovs-vsctl --filter='interface(key="123")' show
3b7dfe59-53ae-42a4-90b4-f46d6f0e47ec
Bridge br1
Port br1
Bridge br0
Port tunnel_port2
Port tunnel_port
Interface tunnel_port
type: geneve
options: {key="123", remote_cert=abc3, remote_ip=flow}
Port p1
Port br0
$ ovs-vsctl --filter='port(key="123")' show
3b7dfe59-53ae-42a4-90b4-f46d6f0e47ec
Bridge br1
Bridge br0
Port tunnel_port
Interface tunnel_port
type: geneve
options: {key="123", remote_cert=abc3, remote_ip=flow}
$ ovs-vsctl --filter='bridge(key="123")' show
3b7dfe59-53ae-42a4-90b4-f46d6f0e47ec
Bridge br0
Port tunnel_port2
Interface tunnel_port2
type: geneve
options: {key="125", remote_cert=qwe2, remote_ip=flow}
Port tunnel_port
Interface tunnel_port
type: geneve
options: {key="123", remote_cert=abc3, remote_ip=flow}
Port p1
Interface p1
Port br0
Interface br0
type: internal
> + }
> +
> + table_filter(ctx, show, filters, start_pos);
> +
> + return ctx->output.length > start_pos;
> +}
> +
> +static char * OVS_WARN_UNUSED_RESULT
> +init_filters(const char *filter_str,
> + const struct ovsdb_idl_table_class *top_table,
This variable is not actually used in the code below. The idea was to store
the "global" filters under the top_table->name key instead of "", so we don't
have to have any special treatment for them, i.e. can drop the wildcard_filter()
function and only use the table_filter(). See the suggested code for v3.
This assumes we can drop this strange truncation loop in the table_filter().
> + struct shash *filters)
> +{
> + struct sset initialized_filters;
> + sset_from_delimited_string(&initialized_filters, filter_str, ",");
> +
> + struct sset *global_set = xzalloc(sizeof *global_set);
> + sset_init(global_set);
> + char *error = NULL;
> +
> + const char *item;
> + SSET_FOR_EACH (item, &initialized_filters) {
> + const struct ovsdb_idl_table_class *table = top_table;
> + const char *ptr = strchr(item, '(');
> + size_t len = strlen(item);
> +
> + char *values = NULL;
> + char *table_name = NULL;
> + struct sset parsed_values;
Reverse x-mass tree.
> +
> + if (ptr && item[len - 1] == ')') {
> + table_name = xmemdup0(item, ptr - item);
> + error = get_table(table_name, &table);
> + if (error) {
> + free(table_name);
> + goto cleanup;
> + }
> +
> + values = xmemdup0(ptr + 1, len - (ptr - item + 2));
> + sset_from_delimited_string(&parsed_values, values, "|");
> +
> + struct sset *value_set = shash_find_data(filters, table_name);
> + if (!value_set) {
> + value_set = xzalloc(sizeof *value_set);
> + sset_init(value_set);
> + sset_clone(value_set, &parsed_values);
> + shash_add(filters, table->name, value_set);
> + } else {
> + const char *v;
> + SSET_FOR_EACH (v, &parsed_values) {
> + sset_add(value_set, v);
> + }
> + }
> +
> + free(table_name);
> + } else if (ptr && item[len - 1] != ')') {
> + error = xasprintf("Malformed filter: missing closing ')'");
> + goto cleanup;
> +
Unnecessary empty line.
> + } else {
> + values = xstrdup(item);
> + sset_from_delimited_string(&parsed_values, values, "|");
> +
> + const char *v;
> + SSET_FOR_EACH (v, &parsed_values) {
> + sset_add(global_set, v);
> + }
> + }
> +
> + sset_destroy(&parsed_values);
> + free(values);
> + }
> +
> + if (!sset_is_empty(global_set)) {
> + shash_add(filters, "", global_set);
> + } else {
> + sset_destroy(global_set);
> + free(global_set);
> + }
> +
> +cleanup:
> + if (error) {
> + sset_destroy(global_set);
> + free(global_set);
> + }
> + sset_destroy(&initialized_filters);
> + return error;
> +}
> +
> +static void
> +destroy_filters(struct shash *filters)
> +{
> + struct shash_node *node;
> + SHASH_FOR_EACH (node, filters) {
> + struct sset *value_set = node->data;
> + sset_destroy(value_set);
> + free(value_set);
> + }
> + shash_destroy(filters);
> }
>
> static void
> @@ -2254,12 +2426,25 @@ cmd_show(struct ctl_context *ctx)
> {
> const struct ovsdb_idl_row *row;
> struct sset shown = SSET_INITIALIZER(&shown);
> + struct shash filters = SHASH_INITIALIZER(&filters);
> +
> + char *filter_str = shash_find_data(&ctx->options, "--filter");
> + if (filter_str && *filter_str) {
> + char *error = init_filters(filter_str,
> + cmd_show_tables[0].table, &filters);
> + if (error) {
> + destroy_filters(&filters);
> + 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);
> + cmd_show_row(ctx, row, 0, &shown, &filters);
> }
>
> + destroy_filters(&filters);
> ovs_assert(sset_is_empty(&shown));
> sset_destroy(&shown);
> }
> @@ -2554,7 +2739,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..e6f034aac 100644
> --- a/tests/ovs-vsctl.at
> +++ b/tests/ovs-vsctl.at
> @@ -1831,3 +1831,52 @@ 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='geneve,interface(1.2.3.5)' show | grep Port |
> sort], [0], [dnl
> + Port tunnel_port2
> +])
> +
> +AT_CHECK([ovs-vsctl --filter='geneve' show | grep Port | sort], [0], [dnl
> + Port br0
> + Port p1
> + Port tunnel_port
> + Port tunnel_port2
> +])
> +
> +AT_CHECK([ovs-vsctl --filter='interface(geneve)' show | grep Port | sort],
> [0], [dnl
> + Port tunnel_port
> + Port tunnel_port2
> +])
> +
> +AT_CHECK([ovs-vsctl --filter='interface(p1)' show | uuidfilt], [0], [dnl
> +<0>
> + Bridge br0
> + fail_mode: secure
> + datapath_type: dummy
> + Port p1
> + Interface p1
> + type: internal
> +])
Since we're filtering on the interface level, we should still see all the
other ports, but not their interfaces. The above outputs should be seen
when we filter on the port table, i.e. 'port(p1)' or 'port(geneve)'.
> +
> +AT_CHECK([ovs-vsctl --filter=$'interface\x28p1' show 2>&1 | uuidfilt], [0],
> [dnl
> +ovs-vsctl: Malformed filter: missing closing ')'
> +])
> +
> +AT_CHECK([ovs-vsctl --filter='interface1(p1)' show 2>&1 | uuidfilt], [0],
> [dnl
> +ovs-vsctl: unknown table "interface1"
> +])
> +
> +AT_CLEANUP
> diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
> index 575b7c0bf..6bc1b55ef 100644
> --- a/utilities/ovs-vsctl.8.in
> +++ b/utilities/ovs-vsctl.8.in
> @@ -152,6 +152,31 @@ initialize the database without executing any other
> command.
> .
> .IP "\fBshow\fR"
> Prints a brief overview of the database contents.
> +.br
> +.TP
> +\fB--filter\fR=\fIfilter-rules\fR
This should be combined with the main .IP "\fBshow\fR" line above, see how flags
for other commands are documented. Also, the summary should show that it's a
list
of filter rules. This way we'll only need to explain how a single rule looks
like,
instead of also stating that it's a comma-separated list. See the --column
option
for the find command in lib/db-ctl-base.man as an example. E.g.:
.IP "[\fB\-\-filter=\fIfilter\-rule\fR[\fB,\fIfilter\-rule\fR]...] \fBshow\fR"
> +Filters the output using either direct record names or table-based patterns:
> +.br
> +.RS
> +.IP \(bu 3
> +\fBRecord names\fR: Comma-separated list of specific records to display
nit: Periods at the end of the sentence. Here and below.
The 'Record names' is a bit confusing. Record names are typically a content of
a 'name' or similar column in the database, and that's not what we're filtering.
The idea of having just one type of a filter was to also document them this way,
i.e. it's always a filter for the printed representation of a particular table.
The difference is that if the table name is not specified, then the filter is
applied at the top level table.
> +.br
> +Example: \fB--filter='geneve,p1'\fR
> +.IP \(bu 3
> +\fBTable patterns\fR: \fItable(conditions)\fR filters records within table
> +.br
> +Examples:
> +.br
> +\fB--filter='Port'\fR (all ports)
> +.br
> +\fB--filter='Interface(type="internal")'\fR (filtered interfaces)
> +.IP \(bu 3
> +\fBCombinations\fR: Mix of record names and table patterns
> +.br
> +Example: \fB--filter='geneve,Port(name="p1")'\fR
> +.RE
> +.br
> +.
> .
> .IP "\fBemer\-reset\fR"
> Reset the configuration into a clean state. It deconfigures OpenFlow
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
