[ovs-dev] [v3] dpif-netdev: Modified ovs-appctl dpctl/dump-flows command

Ilya Maximets i.maximets at ovn.org
Wed Jan 15 17:27:05 UTC 2020


On 15.01.2020 17:19, Emma Finn wrote:
> Modified ovs-appctl dpctl/dump-flows command to output
> the miniflow bits for a given flow when -m option is passed.
> 
> $ ovs-appctl dpctl/dump-flows -m
> 
> Signed-off-by: Emma Finn <emma.finn at intel.com>
> 
> ---
> 
> RFC -> v1
> 
> * Changed revision from RFC to v1
> * Reformatted based on comments
> * Fixed same classifier being dumped multiple times
>   flagged by Ilya
> * Fixed print of subtables flagged by William
> * Updated print count of bits as well as bits themselves
> 
> ---
> 
> v1 -> v2
> 
> * Reformatted based on comments
> * Refactored code to make output part of ovs-appctl
>   dpctl/dump-flows -m command.
> 
> ---
> 
> v2 -> v3
> 
> * Added attribute dp_extra_info to dpif_flow_attrs struct
>   to store miniflow bits as a string
> ---
>  NEWS              | 2 ++
>  lib/dpctl.c       | 4 ++++
>  lib/dpif-netdev.c | 8 ++++++++
>  lib/dpif.h        | 1 +
>  4 files changed, 15 insertions(+)
> 
> diff --git a/NEWS b/NEWS
> index 965faca..1c9d2db 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -8,6 +8,8 @@ Post-v2.12.0
>       * Add option to enable, disable and query TCP sequence checking in
>         conntrack.
>       * Add support for conntrack zone limits.
> +     * Command "ovs-appctl dpctl/dump-flows" refactored to show subtable
> +       miniflow bits for userspace datapath.
>     - AF_XDP:
>       * New option 'use-need-wakeup' for netdev-afxdp to control enabling
>         of corresponding 'need_wakeup' flag in AF_XDP rings.  Enabled by default
> diff --git a/lib/dpctl.c b/lib/dpctl.c
> index a1ea25b..bd33ac4 100644
> --- a/lib/dpctl.c
> +++ b/lib/dpctl.c
> @@ -825,6 +825,10 @@ format_dpif_flow(struct ds *ds, const struct dpif_flow *f, struct hmap *ports,
>      }
>      ds_put_cstr(ds, ", actions:");
>      format_odp_actions(ds, f->actions, f->actions_len, ports);
> +    if (dpctl_p->verbosity && f->attrs.dp_extra_info) {
> +        ds_put_format(ds, ", dp-extra-info:miniflow_bits%s",
> +                      f->attrs.dp_extra_info);

Please, make 'miniflow_bits' part of f->attrs.dp_extra_info.
This doesn't make sense for other datapaths.

And the string leaked here.

> +    }
>  }
>  
>  struct dump_types {
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 079bd1b..51def10 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -3101,6 +3101,14 @@ dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow *netdev_flow,
>  
>      flow->attrs.offloaded = false;
>      flow->attrs.dp_layer = "ovs";
> +
> +    struct ds extra_info = DS_EMPTY_INITIALIZER;
> +
> +    ds_put_format(&extra_info, "(0x%X,0x%X)",
> +                  count_1bits(netdev_flow->cr.mask->mf.map.bits[0]),
> +                  count_1bits(netdev_flow->cr.mask->mf.map.bits[1]));
> +    flow->attrs.dp_extra_info = ds_steal_cstr(&extra_info);
> +    ds_destroy(&extra_info);
>  }
>  
>  static int
> diff --git a/lib/dpif.h b/lib/dpif.h
> index c21e897..064f884 100644
> --- a/lib/dpif.h
> +++ b/lib/dpif.h
> @@ -513,6 +513,7 @@ struct dpif_flow_detailed_stats {
>  struct dpif_flow_attrs {
>      bool offloaded;         /* True if flow is offloaded to HW. */
>      const char *dp_layer;   /* DP layer the flow is handled in. */
> +    char *dp_extra_info;

Comment required.  Keep it generic, please, i.e. no mentioning of 'miniflow_bits'.

>  };
>  
>  struct dpif_flow_dump_types {
> 


More information about the dev mailing list