Re: [dpdk-dev] next releases

["Richardson, Bruce" <bruce.richardson@intel.com>]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Cyril Chemparathy
> Sent: Wednesday, August 27, 2014 6:05 PM
> To: Thomas Monjalon; dev@dpdk.org
> Subject: Re: [dpdk-dev] next releases
> 
> Hi Thomas,
> 
> On 8/25/2014 10:15 AM, Thomas Monjalon wrote:
> > Hello all,
> >
> > I am back from holidays; thanks for all the
> > patches/reviews/comments done during last weeks.
> >
> > I'd like to have a version 1.7.1, ideally at the end of this week.
> >
> > For the coming days,
> >     - first priority is to integrate bug fixes
> >     - some changes which do not imply API could be part of 1.7.1
> >     - please, do not send more features until 1.8.0-rc1
> >     - features that have been been *properly* reviewed or acked before
> >       end of august will be integrated in 1.8.0-rc1
> >     - all pending features which do not have any review will be postponed
> >       after 1.8.0-rc1
> >     - then rc2 will integrate new features if *properly* reviewed at that time
> >
> > I'd like to have some cleanups in version 1.8. Examples:
> >     - get rid of doxygen warnings
> >     - check if compile time options can be moved to run time
> >     - rename some options (CONFIG_RTE_LIBRTE_*_PMD ->
> CONFIG_RTE_LIBRTE_PMD_*)
> >     - merge common code between linux and bsd implementions
> >     - check secondary process rights
> >     - remove drivers lists from code for easy integration of new drivers
> >     - use rte_eth_dev_atomic_read_link_status in drivers
> >     - use librte_cfgfile instead of examples/qos_sched
> >     - add sysfs functions as eal services
> >     - replace printf calls by rte functions
> >     - use new assert macros for unit tests
> >     - remove kni traces from bsd
> >     - remove bare metal traces
> >     - compress test_lpm*_routes.h
> 
> Any thoughts on consolidating/cleaning up the timer interfaces?
> 
> Usage across rte_rdtsc(), rte_get_tsc_cycles(), and
> rte_get_timer_cycles() could use some rationalization, I think.
> 
> It looks like most code should use rte_get_timer_cycles() and generally
> honor user specified timer source selection.  The relatively few places
> that have a good cause to pin down on TSC should probably use
> rte_get_tsc_cycles() instead of rde_rdtsc().  On the other hand, if
> rte_rdtsc() is meant for direct use, why do we need the
> rte_get_tsc_cycles() wrapper?
> 
> Also, I'm not quite clear on the intended usage of rte_rdtsc_precise().
> I can't find uses of this function on master, and it is not quite clear
> to me if the intent is to replace rte_rdtsc() in some (or all?) places
> in the code.  Any insights on this?
> 
> Thanks
> -- Cyril.

No comments on any planned cleanup, but I can provide some background on the existing situation. (This is all to the best of my memory, since much of it dates back a few years and I'm not digging through old repos for the official logs :-) ) 

Originally there was an explicit rte_rdtsc() call to do timestamping and rough cycle counts, and an EAL provided timer api to do wall-time measurements using the HPET. Using the HPET proved more awkward than it needed to be (due to the fact that most distro kernels did not enable the MMAP_HPET option), and had less precision than using rdtsc. So we changed over to having the EAL timer use either HPET or TSC depending on what was available at boot. So we had two timer APIs both of which generally used the TSC, and no way to allow an application to explicitly request the HPET if that was the timesource we wanted. What was worse was that for compatibility across versions the EAL timers were accessed via APIs with HPET in the name - even when they returned values based off TSC.
So we cleaned things up a bit, so that we had a truly generic set of APIs that had a function to return the frequency and cycles for the system-default time source, as well as specific functions to return that info for both HPET (if available) and TSC. The names were made as consistent as possible - so we have rte_get_tsc_cycles() to match the rte_get_timer_cycles() and rte_get_hpet_cycles() functions. The original rte_rdtsc() function was also kept around for backward compatibility, as it was widely used, so both it and rte_get_tsc_cycles() are indeed identical. However, I believe these are the only two duplicate functions in the API, with the wrapper function being just one line long, so it's not a big overhead. I wouldn't look to remove the wrapper function as it would break the consistency of the API for the sake of removing a single-line function, and I definitely wouldn't remove rte_rdtsc() because it's so widely used. 

As for usage, my recommendation that anything wanting to work with wall-time uses rte_get_timer_cycles() (as you suggest), and that anything doing unit tests which wants to get approximate cycle counts uses rte_rdtsc() as existing unit test code does. As for rte_rdtsc_precise, I'm not sure about its origins, but I'm surprised to see that it does not correspond to the rdtscp instruction. Can anyone else comment on this one? I would assume it's designed to be used to get more accurate measurements of smaller blocks of code that we want to benchmark, since rdtsc works best when timing larger blocks (in terms of cycle counts, that is, not source lines :-) ).

Regards,
/Bruce