[ovs-dev] [PATCH] dpif-netdev-perf: Fix double update of perf histograms.

[Jan Scheurich]

Hi Ilya,

Thanks for spotting this. I believe your fix is correct.

BR, Jan

> -----Original Message-----
> From: Ilya Maximets <i.maximets at samsung.com>
> Sent: Monday, 18 March, 2019 14:01
> To: ovs-dev at openvswitch.org; Ian Stokes <ian.stokes at intel.com>
> Cc: Kevin Traynor <ktraynor at redhat.com>; Ilya Maximets
> <i.maximets at samsung.com>; Jan Scheurich <jan.scheurich at ericsson.com>
> Subject: [PATCH] dpif-netdev-perf: Fix double update of perf histograms.
> 
> Real values of 'packets per batch' and 'cycles per upcall' already added to
> histograms in 'dpif-netdev' on receive. Adding the averages makes statistics
> wrong. We should not add to histograms values that never really appeared.
> 
> For exmaple, in current code following situation is possible:
> 
>   pmd thread numa_id 0 core_id 5:
>   ...
>     Rx packets:                  83  (0 Kpps, 13873 cycles/pkt)
>     ...
>     - Upcalls:                    3  (  3.6 %, 248.6 us/upcall)
> 
>   Histograms
>     packets/it      pkts/batch       upcalls/it     cycles/upcall
>     1         83    1         166    1         3    ...
>                                                     15848     2
>                                                     19952     2
>                                                     ...
>                                                     50118     2
> 
> i.e. all the packets counted twice in 'pkts/batch' column and all the upcalls
> counted twice in 'cycles/upcall' column.
> 
> CC: Jan Scheurich <jan.scheurich at ericsson.com>
> Fixes: 79f368756ce8 ("dpif-netdev: Detailed performance stats for PMDs")
> Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
> ---
>  lib/dpif-netdev-perf.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/lib/dpif-netdev-perf.c b/lib/dpif-netdev-perf.c index
> 8f0c9bc4f..52324858d 100644
> --- a/lib/dpif-netdev-perf.c
> +++ b/lib/dpif-netdev-perf.c
> @@ -498,15 +498,7 @@ pmd_perf_end_iteration(struct pmd_perf_stats *s, int
> rx_packets,
>          cycles_per_pkt = cycles / rx_packets;
>          histogram_add_sample(&s->cycles_per_pkt, cycles_per_pkt);
>      }
> -    if (s->current.batches > 0) {
> -        histogram_add_sample(&s->pkts_per_batch,
> -                             rx_packets / s->current.batches);
> -    }
>      histogram_add_sample(&s->upcalls, s->current.upcalls);
> -    if (s->current.upcalls > 0) {
> -        histogram_add_sample(&s->cycles_per_upcall,
> -                             s->current.upcall_cycles / s->current.upcalls);
> -    }
>      histogram_add_sample(&s->max_vhost_qfill, s->current.max_vhost_qfill);
> 
>      /* Add iteration samples to millisecond stats. */
> --
> 2.17.1