On 21 Feb 2022, at 12:29, Eelco Chaudron wrote:
> On 17 Feb 2022, at 14:10, Ilya Maximets wrote: > >> On 1/31/22 11:54, Eelco Chaudron wrote: >>> Make sure to only update packet and byte counters when valid, >>> or else this could lead to "temporarily/occasionally" >>> out-of-sync flow counters. >> >> There was already the same patch submitted here: >> https://patchwork.ozlabs.org/project/openvswitch/patch/20200602075036.78112-1-zhaozha...@163.com/ >> >> And I'm still not comfortable with the change, because it seems >> like it only hides the underlying datapath problem. Do you know >> why exactly datapath stats become lower than previously reported? >> >> If it's some kind of a statistics flush, that will mean that flow >> statistics will not be updated until new stats will catch up to the >> old value leading to the flow revalidation and incorrect flow stats >> anyway. > > This was very hard to reproduce, only once out of 20-30 runs if I remember > correctly. > > Without the patch, it would sometimes show very high numbers and then got > updated after a while with the correct numbers. > At least this is what I remember, as I did not take any notes, and my brain > wanted to forget this patchset :) > > > Guess the fix is needed anyway as this behavior was there since day one in > revalidate_ukey(), e79a6c833. > > I also see that you got no reply to your comments, so I’ll take another stab > to make sure this patch is really fixing the problem, or fixing a problem, > and hiding another TC problem. > >> We fixed incorrect stats on flow modification for dpdk offload provider >> previously here: >> https://patchwork.ozlabs.org/project/openvswitch/patch/20201012142735.5304-1-el...@nvidia.com/ >> >> Do we need something similar for tc? > > I’ll take a close look at the DPDK patch, once I get my reproducer going. Unfortunately, after an afternoon and night or running tests, I can not replicate the problem with the weird counters :( So for now I’ll drop this patch from the set, and hopefully, it will resurface when I’m integrating the system-traffic tests into the hardware offload set. <SNIP> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev