On Wed, Dec 7, 2022 at 7:34 PM Balazs Nemeth <[email protected]> wrote:
> push.n_{packets,bytes} contains the difference in the number of packets
> and bytes. Even if the counters are uint64_t, they can and do still
> overflow. In particular, this happens with high throughput traffic while
> running for a long enough time.
>
Wel the problem is not the overflow, the datapath sometimes reports wrong
values, but I have not been able to replicate this in a consistent way.
See comments on my similar patch from Ilya.
https://patchwork.ozlabs.org/project/openvswitch/patch/164362644549.2824752.16138250405530944551.stgit@ebuild/
//Eelco
> The old code used to just set n_{packets,bytes} to 0. Consequently,
> should_revalidate would return false (assuming the dump took longer
> than ofproto_max_revalidator / 2) since the metric would be incorrectly
> calculated to be too large.
>
> The module arithmatic behind a simple subtraction will take care of the
> overflow case. For that reason, no conditional is needed.
>
> Signed-off-by: Balazs Nemeth <[email protected]>
> ---
> ofproto/ofproto-dpif-upcall.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index ad9635496..f6584af11 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -2334,12 +2334,10 @@ 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);
> + /* In case lhs of the subtraction overflowed since the last check,
> + the subtraction is still correct */
> + 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.37.3
>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
