[ovs-dev] [PATCH V2] Add offload packets statistics
blp at ovn.org
Fri Oct 25 00:53:35 UTC 2019
On Tue, Oct 15, 2019 at 02:56:23PM +0800, zhaozhanxu wrote:
> Add argument '-m' for command ovs-appctl bridge/dump-flows
> to display the offloaded packets statistics.
Thanks for the updated patch.
Are n_offload_packets a subset of n_packets, or in addition to them? If
they are a subset, then n_offload_packets should always be less than or
equal to n_packets; if they are in addition, then there is no
relationship between the two. Either way, this should be explained in
comments on struct dpif_flow_stats.
I guess that "subset" makes more sense--after all, offloaded packets are
still packets--but then we need to update a few places, like
rule_dpif_credit_stats__(). If we take the "additional" choice, then we
need to change other places: I think most places that n_packets is
referenced, we need to write n_packets + n_offload_packets instead.
Please also update the documentation for bridge/dump-flows in
get_dpif_flow_stats(), in lib/dpif-netdev.c, should initialize '*stats'.
I don't think it initializes the new members, but it should.
dpif_netdev_flow_put() adds together dpif_flow_stats. Currently I think
those dpif_flow_stats always have zero in n_offload_*, but it might be a
good idea to add them together anyway.
Actually dpif_netdev_flow_del() does the same thing. Probably that
means it would be a good idea to factor out the code that adds these
things together into a new helper function since there's already
duplicate code and this adds more of it.
dpif_netlink_flow_get_stats() needs to initialize the new fields.
dpif_flow_stats_extract() needs to initialize the new fields.
I imagine that dpif_flow_stats_format() should display the new
fields--perhaps only if they are nonzero?
Should parse_tc_flower_to_match() set n_offload_* instead of packets and
bytes? (Or both of them to the same values, if n_offload_* is a subset
of the regular values.)
Ditto for netdev_tc_flow_del().
upcall_xlate() should initialize n_offload_*.
Ditto for revalidate_ukey() and push_dp_ops().
More information about the dev