[dpdk-dev] [PATCH v2] baseband/turbo_sw: offload cost measurement test

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Mokhtar, Amr
> Sent: Tuesday, April 24, 2018 8:09 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch at intel.com>; Chalupnik, KamilX
> <kamilx.chalupnik at intel.com>; dev at dpdk.org
> Cc: Chalupnik, KamilX <kamilx.chalupnik at intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2] baseband/turbo_sw: offload cost
> measurement test
> 
> 
> 
> > -----Original Message-----
> > From: De Lara Guarch, Pablo
> > Sent: Tuesday 24 April 2018 18:45
> > To: Chalupnik, KamilX <kamilx.chalupnik at intel.com>; dev at dpdk.org
> > Cc: Mokhtar, Amr <amr.mokhtar at intel.com>; Chalupnik, KamilX
> > <kamilx.chalupnik at intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v2] baseband/turbo_sw: offload cost
> > measurement test
> >
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of KamilX
> > > Chalupnik
> > > Sent: Tuesday, April 17, 2018 3:27 PM
> > > To: dev at dpdk.org
> > > Cc: Mokhtar, Amr <amr.mokhtar at intel.com>; Chalupnik, KamilX
> > > <kamilx.chalupnik at intel.com>
> > > Subject: [dpdk-dev] [PATCH v2] baseband/turbo_sw: offload cost
> > measurement
> > > test
> > >
> > > New test created to measure offload cost.
> > > Changes were introduced in API, turbo software driver and test
> > application.
> > >
> >
> > Shouldn't this be generic to bbdev/baseband drivers in general and not
> > just turbo?
> >
> 
> Yes, it is generic.
> But the only driver we have right now is the turbo_sw driver. Future drivers will
> have a similar support.

Right, then the title should be something like "bbdev: measure offload cost",
since this is affecting multiple components in bbdev.

> 
> > > Signed-off-by: KamilX Chalupnik <kamilx.chalupnik at intel.com>
> >
> > ...
> >
> > > --- a/lib/librte_bbdev/rte_bbdev.h
> > > +++ b/lib/librte_bbdev/rte_bbdev.h
> > > @@ -239,6 +239,10 @@ struct rte_bbdev_stats {
> > >  	uint64_t enqueue_err_count;
> > >  	/** Total error count on operations dequeued */
> > >  	uint64_t dequeue_err_count;
> > > +#ifdef RTE_TEST_BBDEV
> > > +	/** It stores turbo decoder/encoder working time. */
> > > +	uint64_t turbo_perf_time;
> > > +#endif
> >
> > I don't think it is a good idea to use RTE_TEST_BBDEV here.
> > This macro is used to enable/disable the compilation of the bbdev test
> > app, so I think it should not be used in the API/PMDs.
> >
> > Also, this looks too specific for the Turbo SW PMD to be exposed as a
> > generic statistic.
> 
> Well, it should be generic. Probably 'turbo' is a bad comment and name.
> It's intention is to feedback execution time/cycles back to test app in order to
> collect the offload cost of the bbdev driver.
> What is meant by the offload cost is the cycles consumed from the moment of
> enqueue till the moment the request is put on the acceleration engine inbound
> sw ring (software) or MMIO operation (hardware).

I understand. Then yes, probably you should find a better name, maybe offload_time?

Thanks,
Pablo