Re: [dpdk-dev] [PATCH v2 1/4] app/test: unit test for rx and tx cycles/packet

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> Sent: Tuesday, October 21, 2014 2:38 PM
> To: Richardson, Bruce
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 1/4] app/test: unit test for rx and tx cycles/packet
> 
> On Tue, Oct 21, 2014 at 10:43:03AM +0000, Richardson, Bruce wrote:
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> > > Sent: Tuesday, October 21, 2014 11:33 AM
> > > To: Liang, Cunming
> > > Cc: dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v2 1/4] app/test: unit test for rx and tx
> > > cycles/packet
> > >
> > > > >
> > > > > > +	if (count == 0)
> > > > > > +		return -1;
> > > > > > +
> > > > > > +	printf("%lu packet, %lu drop, %lu idle\n", count, drop, idle);
> > > > > > +	printf("Result: %ld cycles per packet\n", (cur_tsc - prev_tsc) / count);
> > > > > > +
> > > > > Bad math here.  Theres no guarantee that the tsc hasn't wrapped (potentially
> > > > > more than once) depending on your test length.  you need to check the tsc
> > > before
> > > > > and after each burst and record an average of deltas instead, accounting in
> > > each
> > > > > instance for the possibility of wrap.
> > > > [Liang, Cunming] I'm not sure catch your point correctly.
> > > > I think both cur_tsc and prev_tsc are 64 bits width.
> > > > For 3GHz, I think it won't wrapped so quick.
> > > > As it's uint64_t, so even get wrapped, the delta should still be correct.
> > > But theres no guarantee that the tsc starts at zero when you begin your test.
> > > The system may have been up for a long time and near wrapping already.
> > > Regardless, you need to account for the possibility that cur_tsc is smaller
> > > than prev_tsc, or this breaks.
> > >
> >
> > The tsc. is 64-bit and so only wraps around every couple of hundred years or so on a 2GHz machine, so I don't think it's necessary to
> handle that case.
> >
> But that presumes that no one has written the TSC via IA32_TIME_STAMP_COUNTER.

Then the test app would just print the wrong number :)
I suppose user will just repeat the test.
Again, imagine someone wrmsr(TSC), but set the value just much bigger than current.
Then your statistics will be incorrect, but you have no wait to figure that out. 
I think that trying to handle all such hypothetical situations is a pure overkill.

> Assuming that something will never wrap just seems like bad practice here.  We
> should have a general purpose macro to handle wrapping counters like this, if
> not for this case specficially, then in general.
> 
> Neil
> 
> > /Bruce
> >