From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 35AD3458FA; Tue, 3 Sep 2024 23:11:07 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E501E402C5; Tue, 3 Sep 2024 23:11:06 +0200 (CEST) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id CBA2E402BE for ; Tue, 3 Sep 2024 23:11:05 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id 8A402210AF; Tue, 3 Sep 2024 23:11:05 +0200 (CEST) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH] net/af_packet: add timestamp offloading support Date: Tue, 3 Sep 2024 23:11:00 +0200 X-MimeOLE: Produced By Microsoft Exchange V6.5 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9F698@smartserver.smartshare.dk> In-Reply-To: <20240903092138.0e071924@hermes.local> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH] net/af_packet: add timestamp offloading support Thread-Index: Adr+HV+TEWID16g1SyqrgZV8lvf5hwAIMNlA References: <20240903114306.2336633-1-stefan.laesser@omicronenergy.com> <20240903092138.0e071924@hermes.local> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Stephen Hemminger" , "Stefan Laesser" Cc: "Thomas Monjalon" , "John W. Linville" , X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Tuesday, 3 September 2024 18.22 >=20 > On Tue, 3 Sep 2024 13:43:06 +0200 > Stefan Laesser wrote: >=20 > > 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 > > --- > > .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(-) >=20 > 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. >=20 > 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?