On 3/10/25 11:24 AM, Alexandra Rukomoinikova wrote:
> This commit improves the `cmd_show` functionality in the
> OVS database control module to support filtering rows
> based on command-line arguments.
> This is relevant for ovn-sbctl show.
> 
> Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud>
> ---
>  lib/db-ctl-base.c     | 118 ++++++++++++++++++++++++++++++++++++++++--
>  lib/db-ctl-base.h     |   1 +
>  utilities/ovs-vsctl.c |   8 ++-
>  vtep/vtep-ctl.c       |   7 ++-
>  4 files changed, 127 insertions(+), 7 deletions(-)

Hi, Alexandra.  Thanks for the patch and sorry for the late reply.

I see you went with a different approach to ones I suggested in the
previous OVN patch and it may be fine, but I still think we can make
this more generic, so the same user API can be re-used for other
commands as well, not only for the 'show'.

The approach in this patch assumes the filtering on columns of a
string type, so it can't be used for UUIDs or integers or sets.

One of my previous suggestions was to instead format the thing into
a string and then compare with the filter.  This way we could filter
on any type (sort of a JS approach of type comparison by converting
everything to strings).

This can be implemented by adding a separate flag to the command, e.g.
'ovs-vsctl --filter="str1|str2|str3" show', and then format the row,
produce the output, find the filter string in the output and discard
it if not found, e.g.:

  length = ctx->output.length
  cmd_show_row(ctx, ...);

  filter-match = false
  for filter in filters {
    if strstr(&ctx->output[length], filter) {
      filter-match = true
      break
    }
  }
  if not filter-match {
    ds_truncate(ctx->output, length)
  }

This should work well for recursive calls, entire recursive blocks
will be discarded if nothing inside matches the filter.

Advantages of this method:

1. Easier to implement (I think).
2. We can filter on or even inside any printable column type
   (e.g. maps and sets).
3. No need for complex type checking.
4. We can extend support for the --filter argument to other ctl
   commands.
5. Can be extended to support wildcards by replacing strstr with
   something more clever in the future.
6. Should be simple enough to implement even for custom 'show'
   commands like 'ovn-nbctl show'.

Disadvantage is that simple strstr may return partial matches if
some things have the same prefix.  But this can be fixed later if
needed by replacing strstr with something a touch smarter, e.g.
by introducing a common '\<filter\>' syntax for exact word matches.

What do you think?

Also, we'll need some tests for the functionality inside OVS, so we
can actually verify that the library works correctly.  For this,
it would be good to implement some filtering in ovs-vsctl and add
tests for it.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to