RE: [PATCH] net/af_packet: add timestamp offloading support

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Tuesday, 3 September 2024 18.22
> 
> On Tue, 3 Sep 2024 13:43:06 +0200
> Stefan Laesser <stefan.laesser@omicronenergy.com> wrote:
> 
> > Add the packet timestamp from TPACKET_V2 to the mbuf
> > dynamic rx timestamp register if offload RTE_ETH_RX_OFFLOAD_TIMESTAMP
> > is enabled.
> >
> > TPACKET_V2 provides the timestamp with nanosecond resolution.
> >
> > Signed-off-by: Stefan Laesser <stefan.laesser@omicronenergy.com>
> > ---
> >  .mailmap                                  |  1 +
> >  doc/guides/nics/af_packet.rst             |  8 ++++--
> >  drivers/net/af_packet/rte_eth_af_packet.c | 34 +++++++++++++++++++++-
> -
> >  3 files changed, 38 insertions(+), 5 deletions(-)
> 
> Adding timestamp is good, but it would be better if the timestamp
> field was generic. The pcap PMD also has a timestamp, and pdump API
> could/should use timestamp as well.

As far as I can see, this patch does use the existing cross-driver/generic timestamp dynamic field, like the pcap driver.

> 
> What makes sense is for there to be a standard dynamic field for
> nanosecond resolution timestamp, and add a make sure that all drivers
> use the same base  1/1/1970 same as Linux/Unix.

Yes, standardizing on nanosecond resolution and a common base might have been a better choice than using driver-specific units for the generic timestamp dynamic field.
If the driver can use the NIC's native clock system, the driver doesn't need to convert to nanoseconds, which has a performance cost.
However, I suppose any application using timestamps needs to do this conversion in the application instead, so the total performance is the same as if the drivers did it. I.e. from a performance perspective, the drivers might as well do the conversion, and from a usability perspective, it would be easier with a standard unit and base.

We should define a roadmap towards dynamic mbuf field timestamps using fixed unit and base (instead of driver-specific) and migrate towards it.

Perhaps start by adding an ethdev capability flag, RTE_ETH_RX_OFFLOAD_TIMESTAMP_NS used together with RTE_ETH_RX_OFFLOAD_TIMESTAMP to indicate that the timestamp unit and base follows a common standard, i.e. nanoseconds since UNIX epoch.

There may be other considerations, though: The NIC's clock may drift compared to the CPU's clock, and compared to the clock of other NICs in the same system. So the "base" and "nanoseconds" will still be using the NIC's clock as reference, and it might be way out of sync with the CPU's clock.

> Also, having
> standard helpers in ethdev for the conversion from TSC to NS would
> help.

Helpers to convert from CPU TSC to nanoseconds have broader scope than ethdev and belong in the EAL, perhaps in /lib/eal/include/generic/rte_cycles.h?