On Fri, Oct 11, 2013 at 03:55:07PM -0700, Andy Zhou wrote: > > Signed-off-by: Andy Zhou <az...@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. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev