[ovs-dev] [PATCH] netdev: Custom statistics.

[Ben Pfaff]

On Tue, Nov 28, 2017 at 01:46:05PM +0000, Michal Weglicki wrote:
> - New get_custom_stats interface function is added to netdev. It
>   allows particular netdev implementation to expose custom
>   counters in dictionary format (counter name/counter value).
> - New statistics are retrieved using experimenter code and
>   are printed as a result to ofctl dump-ports.
> - New counters are available for OpenFlow 1.4+.
> - New statistics are printed to output via ofctl only if those
>   are present in reply message.
> - New statistics definition is added to include/openflow/intel-ext.h.
> - Custom statistics are implemented only for dpdk-physical
>   port type.
> - DPDK-physical implementation uses xstats to collect statistics.
>   Only dropped and error counters are exposed.
> 
> Signed-off-by: Michal Weglicki <michalx.weglicki at intel.com>

This is pretty cool.  I like it.

There is one new compiler warning:

    ../lib/string.c:49:1: error: no previous prototype for function 'string_ends_with' [-Werror,-Wmissing-prototypes]

and new "sparse" warnings:

    ../lib/ofp-util.c:8100:40: warning: incorrect type in assignment (different base types)
    ../lib/ofp-util.c:8100:40:    expected restricted ovs_be16 [usertype] stats_array_size
    ../lib/ofp-util.c:8100:40:    got unsigned int const [unsigned] [usertype] size
    ../lib/ofp-util.c:8288:28: warning: incorrect type in assignment (different base types)
    ../lib/ofp-util.c:8288:28:    expected unsigned int [unsigned] [usertype] size
    ../lib/ofp-util.c:8288:28:    got restricted ovs_be16 const [usertype] stats_array_size
    ../lib/ofp-util.c:8297:34: warning: restricted ovs_be16 degrades to integer

It is inconvenient, in ofputil_append_ofp14_port_stats(), to make the
custom stats code know the size of the stats to be appended before
actually appending them.  I think that it would lead to nicer code if we
dropped netdev_get_custom_stats_size(), etc., and just used
ofpmp_postappend() instead of ofpmp_reserve().  The latter is slightly
more efficient but it requires knowing the size in advance.  Before this
patch that was easy, after this patch it is not, so a change in strategy
makes sense.

ofputil_append_ofp14_port_stats() and parse_intel_port_custom_property()
don't seem to be doing network byte order conversion for the counters.

parse_intel_port_custom_property() doesn't seem to be careful not to
overrun the data buffer, or to make sure that the strings are of
reasonable lengths.

Please add an item to NEWS.

What's an appropriate place to document the new statistics?  Is the set
of extended statistics fairly stable?  If so, then maybe they could be
documented directly in OVS, otherwise I'd hope at least for a pointer to
wherever the real documentation is, from an appropriate place in the OVS
documentation.

Thanks,

Ben.