Re: [dpdk-dev] [PATCH v2 0/3] Add rte_eth_read_clock API

["Wiles, Keith" <keith.wiles@intel.com>]

> On Mar 27, 2019, at 9:41 AM, Stephen Hemminger <stephen@networkplumber.org> wrote:
> 
> On Wed, 27 Mar 2019 07:19:32 +0100
> Tom Barbette <barbette@kth.se> wrote:
> 
>> Some NICs allow to timestamp packets, but do not support the full
>> PTP synchronization process. Hence, the value set in the mbuf
>> timestamp field is only the raw value of an internal clock.
>> 
>> To make sense of this value, one at least needs to be able to query
>> the current hardware clock value. As with the TSC, from there
>> a frequency can be derieved by querying multiple time the current value of the
>> internal clock with some known delay between the queries (example
>> provided in the API doc).
>> 
>> This patch series adds support for MLX5.
>> 
>> An example app is provided in the rxtx_callback application.
>> It has been updated to display, on top of the software latency
>> in cycles, the total latency since the packet was received in hardware.
>> The API is used to compute a delta in the TX callback. The raw amount of
>> ticks is converted to cycles using a variation of the technique describe above.
>> 
>> Aside from offloading timestamping, which relieve the
>> software from a few operations, this allows to get much more precision
>> when studying the source of the latency in a system.
>> Eg. in our 100G, CX5 setup the rxtx callback application shows
>> SW latency is around 74 cycles (TSC is 3.2Ghz), but the latency
>> including NIC processing, PCIe, and queuing is around 196 cycles.
>> 
>> One may think at first this API is overlapping with te_eth_timesync_read_time.
>> rte_eth_timesync_read_time is clearly identified as part of a set of functions
>> to use PTP synchronization.
>> The device raw clock is not "sync" in any way. More importantly, the returned
>> value is not a timeval, but an amount of ticks. We could have a cast-based
>> solution, but on top of being an ugly solution, some people seeing the timeval
>> type of rte_eth_timesync_read_time could use it blindly.
>> 
>> Change in v2:
>>  - Rebase on current master
>> 
>> Tom Barbette (3):
>>  rte_ethdev: Add API function to read dev clock
>>  mlx5: Implement support for read_clock
>>  rxtx_callbacks: Add support for HW timestamp
>> 
>> doc/guides/nics/features.rst                |  1 +
>> doc/guides/sample_app_ug/rxtx_callbacks.rst |  9 ++-
>> drivers/net/mlx5/mlx5.c                     |  1 +
>> drivers/net/mlx5/mlx5.h                     |  1 +
>> drivers/net/mlx5/mlx5_ethdev.c              | 29 +++++++
>> drivers/net/mlx5/mlx5_glue.c                |  8 ++
>> drivers/net/mlx5/mlx5_glue.h                |  2 +
>> examples/rxtx_callbacks/Makefile            |  2 +
>> examples/rxtx_callbacks/main.c              | 86 ++++++++++++++++++++-
>> examples/rxtx_callbacks/meson.build         |  1 +
>> lib/librte_ethdev/rte_ethdev.c              | 13 ++++
>> lib/librte_ethdev/rte_ethdev.h              | 44 +++++++++++
>> lib/librte_ethdev/rte_ethdev_core.h         |  6 ++
>> lib/librte_ethdev/rte_ethdev_version.map    |  1 +
>> lib/librte_mbuf/rte_mbuf.h                  |  2 +
>> 15 files changed, 201 insertions(+), 5 deletions(-)
> 
> 
> I like this approach but would like to see the same API supported
> on multiple devices.
> 
> The current timestamp API is a mess because not all devices behave the
> same way. Trying to write an application that uses timestamping is therefore
> very difficult.

Another question is this an optional API for a PMD? I assume it is.

I know the API rte_eht_read_clock() is attempting to read the NIC for this timestamp, but if the PMD does not support this request can we just default to the rte_rdtsc() as a return value?

Regards,
Keith