On Fri, Mar 17, 2023 at 09:47:36PM +0100, Eelco Chaudron wrote: > > > Send from my phone > > > Op 17 mrt. 2023 om 21:11 heeft Marcelo Ricardo Leitner > > <[email protected]> het volgende geschreven: > > > > On Thu, Mar 16, 2023 at 09:51:34AM +0100, Eelco Chaudron wrote: > >> > >> > >>> On 22 Dec 2022, at 13:32, Eelco Chaudron wrote: > >>> > >>>> On 22 Dec 2022, at 10:26, Balazs Nemeth wrote: > >>> > >>>> The only way that stats->{n_packets,n_bytes} would decrease is due to an
nit: s/way/ways/ > >>>> overflow, or if there are bugs in how statistics are handled. In the > >>>> past, there were multiple bugs that caused a jump backward. A > >>>> workaround was in place to set the statistics to 0 in that case. When > >>>> this happened while the revalidator was under heavy load, the workaround > >>>> had an unintended side effect where should_revalidate returned false > >>>> causing the flow to be removed because the metric it calculated was > >>>> based on a bogus value. Since many of those bugs have now been > >>>> identified and resolved, there is no need to set the statistics to 0. In > >>>> addition, the (unlikely) overflow still needs to be handled > >>>> appropriately. 1. Perhaps it would be prudent to log/count if/when this occurs 2. I take it that the overflow handling would be follow-up work, is that correct? > >>>> Signed-off-by: Balazs Nemeth <[email protected]> > >>>> --- > >>>> > >>>> Depends on: > >>>> - netdev-offload-tc: Preserve tc statistics when flow gets modified. > >>>> - tc: Add support for TCA_STATS_PKT64 > >>>> - ofproto-dpif-upcall: Wait for valid hw flow stats before applying > >>>> min-revalidate-pps > >>>> - ofproto-dpif-upcall: Use last known stats ukey stats on revalidate > >>>> missed dp flows. > >>> > >>> Did a lot of tests with this patch applied, but as mentioned it does need > >>> all four patches mentioned above applied first. > >>> > >>> Acked-by: Eelco Chaudron <[email protected]> > >> > >> Now that all the dependent patches have been applied and backported to > >> 2.17, can we have another look at this patch? > > > > Not sure what you mean here, Eelco. You mean, just a patch review or > > you mean something else, like evaluating if the patch is still needed > > at all? > > Guess I was not clear enough :) what I meant was for the maintainers to > have another look at this patch and if no further objections apply it. There seems to now be minor fuzz when applying this patch (locally). Perhaps a rebase is in order? _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
