On Thu, May 25, 2023 at 10:58:36AM +0200, 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)
> +{
> + 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));
Hi Balazs,
Clang seems a bit upset about this bit:
../../ofproto/ofproto-dpif-upcall.c:2358:37: error: reading variable 'stats'
requires holding any mutex [-Werror,-Wthread-safety-analysis]
stats->n_packets, ukey->stats.n_packets, ds_cstr(&ds));
^
Link:
https://github.com/ovsrobot/ovs/actions/runs/5078235117/jobs/9122656937#step:11:2833
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
