[ovs-dev] [PATCH V2] Add offload packets statistics

Ben Pfaff 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
ovs-vswitchd(8).

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().

Thanks,

Ben.


More information about the dev mailing list