[ovs-dev] [PATCH 2/2] dpif-linux: collect and display mega flow mask stats

[Ben Pfaff]

On Fri, Oct 11, 2013 at 03:55:07PM -0700, Andy Zhou wrote:
> 
> Signed-off-by: Andy Zhou <azhou at nicira.com>

It would be nice to add something to ovs-dpctl(8) to explain these new
stats (as well as the old ones, for that matter).

I would usually wrap this:
    struct ovs_dp_megaflow_stats
            megaflow_stats;            /* OVS_DP_ATTR_MEGAFLOW_STATS. */
as:
    struct ovs_dp_megaflow_stats megaflow_stats;
                                       /* OVS_DP_ATTR_MEGAFLOW_STATS. */

In dpif_linux_dp_from_ofpbuf(), here we might repeat the same comment
as for the OVS_DP_ATTR_STATS case immediately above it, just to hammer
the point home:
> +    if (a[OVS_DP_ATTR_MEGAFLOW_STATS]) {
> +        memcpy(&dp->megaflow_stats, nl_attr_get(a[OVS_DP_ATTR_MEGAFLOW_STATS]),
> +               sizeof dp->megaflow_stats);
> +    }
> +
>      return 0;
>  }

Here we test n_masks against 0 and UINT64_MAX, presumably in case the
datapath doesn't support megaflows, but I didn't notice code that
initializes n_masks to UINT64_MAX (or to 0) if the attribute is
missing (maybe I just missed it):
> +        if (stats.n_masks && stats.n_masks != UINT64_MAX) {
> +            uint64_t n_pkts = stats.n_hit + stats.n_missed;
> +            double avg = (double) stats.n_mask_hit / n_pkts;
> +
> +            printf("\tmasks: hit:%"PRIu64" total:%"PRIu64" hit/pkt:%.2f\n",
> +                   stats.n_mask_hit, stats.n_masks, avg);
> +        }
>      }

I don't see an update to dpif-netdev.  I guess that at least
dpif_netdev_get_stats() should set the new members to 0 or UINT64_MAX.

Thanks,

Ben.