On 4/25/2024 10:26 AM, Mattias Rönnblom wrote: > On 2024-04-25 01:55, Stephen Hemminger wrote: >> On Thu, 25 Apr 2024 00:27:36 +0200 >> Mattias Rönnblom <hof...@lysator.liu.se> wrote: >> >>> On 2024-04-24 21:13, Stephen Hemminger wrote: >>>> On Wed, 24 Apr 2024 18:50:50 +0100 >>>> Ferruh Yigit <ferruh.yi...@amd.com> wrote: >>>> >>>>>> I don't know how slow af_packet is, but if you care about >>>>>> performance, >>>>>> you don't want to use atomic add for statistics. >>>>>> >>>>> >>>>> There are a few soft drivers already using atomics adds for >>>>> updating stats. >>>>> If we document expectations from 'rte_eth_stats_reset()', we can >>>>> update >>>>> those usages. >>>> >>>> Using atomic add is lots of extra overhead. The statistics are not >>>> guaranteed >>>> to be perfect. If nothing else, the bytes and packets can be skewed. >>>> >>> >>> The sad thing here is that in case the counters are reset within the >>> load-modify-store cycle of the lcore counter update, the reset may end >>> up being a nop. So, it's not like you missed a packet or two, or suffer >>> some transient inconsistency, but you completed and permanently ignored >>> the reset request. >> >> That is one of the many reasons the Linux kernel intentionally did not >> implement a reset statistics operation. > > DPDK should deprecate statistics reset, it seems to me. > > The current API is broken anyway, if you care about correctness. A reset > function must return the current state of the counters, at the point > them being reset. Otherwise, a higher layer may miss counter updates. > > The af_packet PMD should return -ENOTSUP on reset (which is allowed by > the API). > > Maintaining an offset-since-last-reset for counters is a control plane > thing, and shouldn't be in PMDs. (If MT-safe reset for SW-managed > counters are to be expected from the PMDs, we should have some helper > API to facilitate its efficient & correct-enough implementation.) >
statistics reset works for HW devices, instead of removing statics reset I am for documenting API that it may be not reliable, I can send a patch for it. With above change, we can be more relax on stats update specially for soft drivers, and can convert atomic_add stats updates to "atomic load + add + atomic store". Does this plan make sense?
