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
> >> 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.
> >>
> >> 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?
Thanks,
Marcelo
>
> Thanks,
>
> Eelco
>
> >> ofproto/ofproto-dpif-upcall.c | 8 ++------
> >> 1 file changed, 2 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> >> index ad9635496..9117a1aef 100644
> >> --- a/ofproto/ofproto-dpif-upcall.c
> >> +++ b/ofproto/ofproto-dpif-upcall.c
> >> @@ -2334,12 +2334,8 @@ revalidate_ukey(struct udpif *udpif, struct
> >> udpif_key *ukey,
> >>
> >> push.used = stats->used;
> >> push.tcp_flags = stats->tcp_flags;
> >> - push.n_packets = (stats->n_packets > ukey->stats.n_packets
> >> - ? stats->n_packets - ukey->stats.n_packets
> >> - : 0);
> >> - push.n_bytes = (stats->n_bytes > ukey->stats.n_bytes
> >> - ? stats->n_bytes - ukey->stats.n_bytes
> >> - : 0);
> >> + push.n_packets = stats->n_packets - ukey->stats.n_packets;
> >> + push.n_bytes = stats->n_bytes - ukey->stats.n_bytes;
> >>
> >> if (need_revalidate) {
> >> if (should_revalidate(udpif, push.n_packets, ukey->stats.used)) {
> >> --
> >> 2.38.1
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
