Hi Ilya, Thanks for spotting this. I believe your fix is correct.
BR, Jan > -----Original Message----- > From: Ilya Maximets <[email protected]> > Sent: Monday, 18 March, 2019 14:01 > To: [email protected]; Ian Stokes <[email protected]> > Cc: Kevin Traynor <[email protected]>; Ilya Maximets > <[email protected]>; Jan Scheurich <[email protected]> > 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 <[email protected]> > Fixes: 79f368756ce8 ("dpif-netdev: Detailed performance stats for PMDs") > Signed-off-by: Ilya Maximets <[email protected]> > --- > 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
