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

["Stefan Lässer" <stefan.laesser@omicronenergy.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.

Yes, I use the generic timestamp dynamic field as used in all the other PMDs I have looked at.

> 
> >
> > 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?

Should I extend my patch to include the new RTE_ETH_RX_OFFLOAD_TIMESTAMP_NS capability?
What happens if the user only enables RTE_ETH_RX_OFFLOAD_TIMESTAMP in the AF_PACKET PMD?
I would suggest that in this case the timestamp will have microsecond accuracy and only if RTE_ETH_RX_OFFLOAD_TIMESTAMP_NS is also enabled, then the timestamp will have nanosecond accuracy.