On 14.01.2020 16:41, 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.f...@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.
> ---
> NEWS | 2 ++
> lib/dpctl.c | 4 ++++
> lib/dpif-netdev.c | 43 +++++++++++++++++++++++++++++++++++++------
> lib/dpif.c | 9 +++++++++
> lib/dpif.h | 2 ++
> 5 files changed, 54 insertions(+), 6 deletions(-)
>
> 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..9242407 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.subtable) {
> + ds_put_cstr(ds, ", dp-extra-info:");
> + dpif_flow_attrs_format(&f->attrs, ds);
> + }
> }
>
> struct dump_types {
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 079bd1b..314d3cb 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -867,6 +867,8 @@ static inline bool
> pmd_perf_metrics_enabled(const struct dp_netdev_pmd_thread *pmd);
> static void queue_netdev_flow_del(struct dp_netdev_pmd_thread *pmd,
> struct dp_netdev_flow *flow);
> +static struct dpcls_subtable *get_subtable(struct dp_netdev_pmd_thread *,
> + const struct dp_netdev_flow *);
>
> static void
> emc_cache_init(struct emc_cache *flow_cache)
> @@ -3054,10 +3056,14 @@ get_dpif_flow_stats(const struct dp_netdev_flow
> *netdev_flow_,
> * 'mask_buf'. Actions will be returned without copying, by relying on RCU to
> * protect them. */
> static void
> -dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow *netdev_flow,
> +dp_netdev_flow_to_dpif_flow(struct dp_netdev *dp,
> + const struct dp_netdev_flow *netdev_flow,
> struct ofpbuf *key_buf, struct ofpbuf *mask_buf,
> struct dpif_flow *flow, bool terse)
> {
> + struct dp_netdev_pmd_thread *pmd_thread;
> + struct dpcls_subtable *subtable;
> +
> if (terse) {
> memset(flow, 0, sizeof *flow);
> } else {
> @@ -3101,6 +3107,10 @@ dp_netdev_flow_to_dpif_flow(const struct
> dp_netdev_flow *netdev_flow,
>
> flow->attrs.offloaded = false;
> flow->attrs.dp_layer = "ovs";
> +
> + pmd_thread = dp_netdev_get_pmd(dp, flow->pmd_id);
> + subtable = get_subtable(pmd_thread, netdev_flow);
netdev_flow contains miniflow. You just need to count bits inside of it.
Returning the pointer here sounds very dangerous.
> + flow->attrs.subtable = subtable;
I'd prefer this to be:
flow->attrs.dp_extra_info = <malloced string>;
The string should be freed by the caller, if provided.
To construct the string lib/dynamic-string.h could be used.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
