On 5/25/23 15:37, 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 issues 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. If an unexpected jump does happen, just log it as a > warning. > > Signed-off-by: Balazs Nemeth <bnem...@redhat.com> > --- > ofproto/ofproto-dpif-upcall.c | 29 +++++++++++++++++++++++------ > 1 file changed, 23 insertions(+), 6 deletions(-) > > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c > index cd57fdbd9..4c34d695e 100644 > --- a/ofproto/ofproto-dpif-upcall.c > +++ b/ofproto/ofproto-dpif-upcall.c > @@ -2339,6 +2339,26 @@ exit: > return result; > } > > +static void > +log_unexpected_stats_jump(struct udpif_key *ukey, > + const struct dpif_flow_stats *stats)
We need to add a thread-safety annotation here to make clang happy: OVS_REQUIRES(ukey->mutex) > +{ > + static struct vlog_rate_limit rll = VLOG_RATE_LIMIT_INIT(1, 5); > + struct ds ds = DS_EMPTY_INITIALIZER; > + struct ofpbuf *actions; > + > + odp_format_ufid(&ukey->ufid, &ds); > + ds_put_cstr(&ds, " "); > + odp_flow_key_format(ukey->key, ukey->key_len, &ds); > + ds_put_cstr(&ds, ", actions:"); > + actions = ovsrcu_get(struct ofpbuf *, &ukey->actions); > + format_odp_actions(&ds, actions->data, actions->size, NULL); > + VLOG_WARN_RL(&rll, "Unexpected jump in packet stats from %"PRIu64 > + " to %"PRIu64" when handling ukey %s", > + stats->n_packets, ukey->stats.n_packets, ds_cstr(&ds)); And stats should be in the opposite order. The jump is from ukey->stats.n_packets to stats->n_packets, not the other way around. Sorry I missed that before. Best regards, Ilya Maximets. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev