[PATCH] net/af_packet: add timestamp offloading support

[PATCH] net/af_packet: add timestamp offloading support

[Stefan Laesser]

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(-)

diff --git a/.mailmap b/.mailmap
index 4a508bafad..69314e34a0 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1414,6 +1414,7 @@ Steeven Lee <steeven@gmail.com>
 Steeven Li <steeven.li@broadcom.com>
 Stefan Baranoff <sbaranoff@gmail.com>
 Stefan Hajnoczi <stefanha@redhat.com>
+Stefan Laesser <stefan.laesser@omicronenergy.com>
 Stefan Puiu <stefan.puiu@gmail.com>
 Stefan Wegrzyn <stefan.wegrzyn@intel.com>
 Stepan Sojka <stepan.sojka@adaptivemobile.com>
diff --git a/doc/guides/nics/af_packet.rst b/doc/guides/nics/af_packet.rst
index 66b977e1a2..e5da625e09 100644
--- a/doc/guides/nics/af_packet.rst
+++ b/doc/guides/nics/af_packet.rst
@@ -69,6 +69,8 @@ framecnt=512):
 Features and Limitations
 ------------------------
 
-The PMD will re-insert the VLAN tag transparently to the packet if the kernel
-strips it, as long as the ``RTE_ETH_RX_OFFLOAD_VLAN_STRIP`` is not enabled by the
-application.
+*  The PMD will re-insert the VLAN tag transparently to the packet if the kernel
+   strips it, as long as the ``RTE_ETH_RX_OFFLOAD_VLAN_STRIP`` is not enabled by the
+   application.
+*  The PMD will add the kernel packet timestamp with nanoseconds resolution if
+   ``RTE_ETH_RX_OFFLOAD_TIMESTAMP`` is enabled.
diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
index 6b7b16f348..6a7d5d0ffb 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -40,6 +40,9 @@
 #define DFLT_FRAME_SIZE		(1 << 11)
 #define DFLT_FRAME_COUNT	(1 << 9)
 
+uint64_t af_packet_timestamp_dynflag;
+int af_packet_timestamp_dynfield_offset = -1;
+
 struct __rte_cache_aligned pkt_rx_queue {
 	int sockfd;
 
@@ -51,6 +54,7 @@ struct __rte_cache_aligned pkt_rx_queue {
 	struct rte_mempool *mb_pool;
 	uint16_t in_port;
 	uint8_t vlan_strip;
+	uint8_t timestamp_offloading;
 
 	volatile unsigned long rx_pkts;
 	volatile unsigned long rx_bytes;
@@ -82,6 +86,7 @@ struct pmd_internals {
 	struct pkt_rx_queue *rx_queue;
 	struct pkt_tx_queue *tx_queue;
 	uint8_t vlan_strip;
+	uint8_t timestamp_offloading;
 };
 
 static const char *valid_arguments[] = {
@@ -157,6 +162,16 @@ eth_af_packet_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 				PMD_LOG(ERR, "Failed to reinsert VLAN tag");
 		}
 
+		/* add kernel provided timestamp when offloading is enabled */
+		if (pkt_q->timestamp_offloading) {
+			/* since TPACKET_V2 timestamps are provided in nanoseconds resolution */
+			*RTE_MBUF_DYNFIELD(mbuf, af_packet_timestamp_dynfield_offset,
+				rte_mbuf_timestamp_t *) =
+					(uint64_t)ppd->tp_sec * 1000000000 + ppd->tp_nsec;
+
+			mbuf->ol_flags |= af_packet_timestamp_dynflag;
+		}
+
 		/* release incoming frame and advance ring buffer */
 		ppd->tp_status = TP_STATUS_KERNEL;
 		if (++framenum >= framecount)
@@ -315,13 +330,25 @@ static int
 eth_dev_start(struct rte_eth_dev *dev)
 {
 	struct pmd_internals *internals = dev->data->dev_private;
-	uint16_t i;
+
+	if (internals->timestamp_offloading) {
+		/* Register mbuf field and flag for Rx timestamp */
+		int rc = rte_mbuf_dyn_rx_timestamp_register(&af_packet_timestamp_dynfield_offset,
+				&af_packet_timestamp_dynflag);
+		if (rc) {
+			PMD_LOG(ERR, "Cannot register mbuf field/flag for timestamp");
+			return rc;
+		}
+	}
 
 	dev->data->dev_link.link_status = RTE_ETH_LINK_UP;
+
+	uint16_t i;
 	for (i = 0; i < internals->nb_queues; i++) {
 		dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED;
 		dev->data->tx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED;
 	}
+
 	return 0;
 }
 
@@ -365,6 +392,7 @@ eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
 	struct pmd_internals *internals = dev->data->dev_private;
 
 	internals->vlan_strip = !!(rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_STRIP);
+	internals->timestamp_offloading = !!(rxmode->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP);
 	return 0;
 }
 
@@ -381,7 +409,8 @@ eth_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->min_rx_bufsize = 0;
 	dev_info->tx_offload_capa = RTE_ETH_TX_OFFLOAD_MULTI_SEGS |
 		RTE_ETH_TX_OFFLOAD_VLAN_INSERT;
-	dev_info->rx_offload_capa = RTE_ETH_RX_OFFLOAD_VLAN_STRIP;
+	dev_info->rx_offload_capa = RTE_ETH_RX_OFFLOAD_VLAN_STRIP |
+		RTE_ETH_RX_OFFLOAD_TIMESTAMP;
 
 	return 0;
 }
@@ -508,6 +537,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,
 	dev->data->rx_queues[rx_queue_id] = pkt_q;
 	pkt_q->in_port = dev->data->port_id;
 	pkt_q->vlan_strip = internals->vlan_strip;
+	pkt_q->timestamp_offloading = internals->timestamp_offloading;
 
 	return 0;
 }
-- 
2.34.1

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

[Stephen Hemminger]

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.

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.  Also, having
standard helpers in ethdev for the conversion from TSC to NS would
help.

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

[Morten Brørup]

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

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

[Stefan Lässer]

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

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

[Morten Brørup]

> From: Stefan Lässer [mailto:stefan.laesser@omicronenergy.com]
> Sent: Friday, 6 September 2024 08.23
> 
> > > 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?

That would be nice, but not a requirement. :-)

Please do it as a series of patches, maybe three:
1. This patch.
2. A patch to generally introduce TIMESTAMP_NS RX offload and capability flags.
3. A patch to implement TIMESTAMP_NS in af_packet.

The new TIMESTAMP_NS feature might trigger some discussions, and you don't want this patch caught up too much in that discussion.

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

There's no need for different timestamp accuracy if TIMESTAMP_NS is not enabled.
RTE_ETH_RX_OFFLOAD_TIMESTAMP means that a timestamp is present, with driver dependent clock and base.
The driver is allowed to use nanoseconds as clock and UNIX origo as base, regardless.

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

[Stefan Lässer]

> > From: Stefan Lässer [mailto:stefan.laesser@omicronenergy.com]
> > Sent: Friday, 6 September 2024 08.23
> >
> > > > 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?
> 
> That would be nice, but not a requirement. :-)
> 
> Please do it as a series of patches, maybe three:
> 1. This patch.
> 2. A patch to generally introduce TIMESTAMP_NS RX offload and capability
> flags.
> 3. A patch to implement TIMESTAMP_NS in af_packet.

I will give it a try. :-)

> The new TIMESTAMP_NS feature might trigger some discussions, and you don't
> want this patch caught up too much in that discussion.
> 
> > 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.
> 
> There's no need for different timestamp accuracy if TIMESTAMP_NS is not
> enabled.
> RTE_ETH_RX_OFFLOAD_TIMESTAMP means that a timestamp is present, with
> driver dependent clock and base.
> The driver is allowed to use nanoseconds as clock and UNIX origo as base,
> regardless.
> 

Okay - so the purpose of RTE_ETH_RX_OFFLOAD_TIMESTAMP_NS is just to signal that the
PMD delivers timestamps with nanoseconds resolution - I see.

What about this patch: are the changes valid / good enough to become part of the mainline version?

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

[Morten Brørup]

> From: Stefan Laesser [mailto:stefan.laesser@omicronenergy.com]
> Sent: Tuesday, 3 September 2024 13.43
> 
> 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.

Suggest adding: and UNIX origo, i.e. time since 1-JAN-1970 UTC.

> 
> Signed-off-by: Stefan Laesser <stefan.laesser@omicronenergy.com>
> ---


> +uint64_t af_packet_timestamp_dynflag;
> +int af_packet_timestamp_dynfield_offset = -1;

No need to expose these publicly, they should be static.
This also means that you can remove the af_packet_ prefix.


>  eth_dev_start(struct rte_eth_dev *dev)
>  {
>  	struct pmd_internals *internals = dev->data->dev_private;
> -	uint16_t i;
> +
> +	if (internals->timestamp_offloading) {
> +		/* Register mbuf field and flag for Rx timestamp */
> +		int rc =
> rte_mbuf_dyn_rx_timestamp_register(&af_packet_timestamp_dynfield_offset,
> +				&af_packet_timestamp_dynflag);
> +		if (rc) {
> +			PMD_LOG(ERR, "Cannot register mbuf field/flag for
> timestamp");
> +			return rc;
> +		}
> +	}
> 
>  	dev->data->dev_link.link_status = RTE_ETH_LINK_UP;
> +
> +	uint16_t i;

No need to move this down here. Please leave it where it was.

Alternatively, minimize its scope by moving it inside the for loop:
-	for (i = 0; i < internals->nb_queues; i++) {
+	for (uint16_t i = 0; i < internals->nb_queues; i++) {


With the two variables made local,

Acked-by: Morten Brørup <mb@smartsharesystems.com>

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

[Stefan Lässer]

> > +uint64_t af_packet_timestamp_dynflag; int
> > +af_packet_timestamp_dynfield_offset = -1;
> 
> No need to expose these publicly, they should be static.
> This also means that you can remove the af_packet_ prefix.
> >  eth_dev_start(struct rte_eth_dev *dev)  {
> >  	struct pmd_internals *internals = dev->data->dev_private;
> > -	uint16_t i;
> > +
> > +	if (internals->timestamp_offloading) {
> > +		/* Register mbuf field and flag for Rx timestamp */
> > +		int rc =
> >
> rte_mbuf_dyn_rx_timestamp_register(&af_packet_timestamp_dynfield_offs
> e
> > t,
> > +				&af_packet_timestamp_dynflag);
> > +		if (rc) {
> > +			PMD_LOG(ERR, "Cannot register mbuf field/flag for
> > timestamp");
> > +			return rc;
> > +		}
> > +	}
> >
> >  	dev->data->dev_link.link_status = RTE_ETH_LINK_UP;
> > +
> > +	uint16_t i;
> 
> No need to move this down here. Please leave it where it was.
> 
> Alternatively, minimize its scope by moving it inside the for loop:
> -	for (i = 0; i < internals->nb_queues; i++) {
> +	for (uint16_t i = 0; i < internals->nb_queues; i++) {
> 
> 
> With the two variables made local,
> 
> Acked-by: Morten Brørup <mb@smartsharesystems.com>

Thank you very much for your feedback! I have submitted a patch including your recommendations.

I will start with the other patches now.

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

[Stefan Laesser]

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
and UNIX origo, i.e. time since 1-JAN-1970 UTC.

Signed-off-by: Stefan Laesser <stefan.laesser@omicronenergy.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---

v2:
* mentioned that timestamp is since 1-JAN-1970 UTC
* made the two timestamp_dynflag variables private and removed prefix
* undo moving of counter variable in eth_dev_start() to reduce code changes

 .mailmap                                  |  1 +
 doc/guides/nics/af_packet.rst             |  8 +++---
 drivers/net/af_packet/rte_eth_af_packet.c | 30 ++++++++++++++++++++++-
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/.mailmap b/.mailmap
index 4a508bafad..69314e34a0 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1414,6 +1414,7 @@ Steeven Lee <steeven@gmail.com>
 Steeven Li <steeven.li@broadcom.com>
 Stefan Baranoff <sbaranoff@gmail.com>
 Stefan Hajnoczi <stefanha@redhat.com>
+Stefan Laesser <stefan.laesser@omicronenergy.com>
 Stefan Puiu <stefan.puiu@gmail.com>
 Stefan Wegrzyn <stefan.wegrzyn@intel.com>
 Stepan Sojka <stepan.sojka@adaptivemobile.com>
diff --git a/doc/guides/nics/af_packet.rst b/doc/guides/nics/af_packet.rst
index 66b977e1a2..a343d3a961 100644
--- a/doc/guides/nics/af_packet.rst
+++ b/doc/guides/nics/af_packet.rst
@@ -69,6 +69,8 @@ framecnt=512):
 Features and Limitations
 ------------------------
 
-The PMD will re-insert the VLAN tag transparently to the packet if the kernel
-strips it, as long as the ``RTE_ETH_RX_OFFLOAD_VLAN_STRIP`` is not enabled by the
-application.
+*  The PMD will re-insert the VLAN tag transparently to the packet if the kernel
+   strips it, as long as the ``RTE_ETH_RX_OFFLOAD_VLAN_STRIP`` is not enabled by the
+   application.
+*  The PMD will add the kernel packet timestamp with nanoseconds resolution and
+   UNIX origo, i.e. time since 1-JAN-1970 UTC, if ``RTE_ETH_RX_OFFLOAD_TIMESTAMP`` is enabled.
diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
index 6b7b16f348..380ce15ca3 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -40,6 +40,9 @@
 #define DFLT_FRAME_SIZE		(1 << 11)
 #define DFLT_FRAME_COUNT	(1 << 9)
 
+static uint64_t timestamp_dynflag;
+static int timestamp_dynfield_offset = -1;
+
 struct __rte_cache_aligned pkt_rx_queue {
 	int sockfd;
 
@@ -51,6 +54,7 @@ struct __rte_cache_aligned pkt_rx_queue {
 	struct rte_mempool *mb_pool;
 	uint16_t in_port;
 	uint8_t vlan_strip;
+	uint8_t timestamp_offloading;
 
 	volatile unsigned long rx_pkts;
 	volatile unsigned long rx_bytes;
@@ -82,6 +86,7 @@ struct pmd_internals {
 	struct pkt_rx_queue *rx_queue;
 	struct pkt_tx_queue *tx_queue;
 	uint8_t vlan_strip;
+	uint8_t timestamp_offloading;
 };
 
 static const char *valid_arguments[] = {
@@ -157,6 +162,16 @@ eth_af_packet_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 				PMD_LOG(ERR, "Failed to reinsert VLAN tag");
 		}
 
+		/* add kernel provided timestamp when offloading is enabled */
+		if (pkt_q->timestamp_offloading) {
+			/* since TPACKET_V2 timestamps are provided in nanoseconds resolution */
+			*RTE_MBUF_DYNFIELD(mbuf, af_packet_timestamp_dynfield_offset,
+				rte_mbuf_timestamp_t *) =
+					(uint64_t)ppd->tp_sec * 1000000000 + ppd->tp_nsec;
+
+			mbuf->ol_flags |= af_packet_timestamp_dynflag;
+		}
+
 		/* release incoming frame and advance ring buffer */
 		ppd->tp_status = TP_STATUS_KERNEL;
 		if (++framenum >= framecount)
@@ -317,6 +332,16 @@ eth_dev_start(struct rte_eth_dev *dev)
 	struct pmd_internals *internals = dev->data->dev_private;
 	uint16_t i;
 
+	if (internals->timestamp_offloading) {
+		/* Register mbuf field and flag for Rx timestamp */
+		int rc = rte_mbuf_dyn_rx_timestamp_register(&af_packet_timestamp_dynfield_offset,
+				&af_packet_timestamp_dynflag);
+		if (rc) {
+			PMD_LOG(ERR, "Cannot register mbuf field/flag for timestamp");
+			return rc;
+		}
+	}
+
 	dev->data->dev_link.link_status = RTE_ETH_LINK_UP;
 	for (i = 0; i < internals->nb_queues; i++) {
 		dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED;
@@ -365,6 +390,7 @@ eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
 	struct pmd_internals *internals = dev->data->dev_private;
 
 	internals->vlan_strip = !!(rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_STRIP);
+	internals->timestamp_offloading = !!(rxmode->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP);
 	return 0;
 }
 
@@ -381,7 +407,8 @@ eth_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->min_rx_bufsize = 0;
 	dev_info->tx_offload_capa = RTE_ETH_TX_OFFLOAD_MULTI_SEGS |
 		RTE_ETH_TX_OFFLOAD_VLAN_INSERT;
-	dev_info->rx_offload_capa = RTE_ETH_RX_OFFLOAD_VLAN_STRIP;
+	dev_info->rx_offload_capa = RTE_ETH_RX_OFFLOAD_VLAN_STRIP |
+		RTE_ETH_RX_OFFLOAD_TIMESTAMP;
 
 	return 0;
 }
@@ -508,6 +535,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,
 	dev->data->rx_queues[rx_queue_id] = pkt_q;
 	pkt_q->in_port = dev->data->port_id;
 	pkt_q->vlan_strip = internals->vlan_strip;
+	pkt_q->timestamp_offloading = internals->timestamp_offloading;
 
 	return 0;
 }
-- 
2.34.1

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

[Stefan Laesser]

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
and UNIX origo, i.e. time since 1-JAN-1970 UTC.

Signed-off-by: Stefan Laesser <stefan.laesser@omicronenergy.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---

v3:
* fixed compilation errors

v2:
* mentioned that timestamp is since 1-JAN-1970 UTC
* made the two timestamp_dynflag variables private and removed prefix
* undo moving of counter variable in eth_dev_start() to reduce code changes

 .mailmap                                  |  1 +
 doc/guides/nics/af_packet.rst             |  8 +++---
 drivers/net/af_packet/rte_eth_af_packet.c | 30 ++++++++++++++++++++++-
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/.mailmap b/.mailmap
index 4a508bafad..69314e34a0 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1414,6 +1414,7 @@ Steeven Lee <steeven@gmail.com>
 Steeven Li <steeven.li@broadcom.com>
 Stefan Baranoff <sbaranoff@gmail.com>
 Stefan Hajnoczi <stefanha@redhat.com>
+Stefan Laesser <stefan.laesser@omicronenergy.com>
 Stefan Puiu <stefan.puiu@gmail.com>
 Stefan Wegrzyn <stefan.wegrzyn@intel.com>
 Stepan Sojka <stepan.sojka@adaptivemobile.com>
diff --git a/doc/guides/nics/af_packet.rst b/doc/guides/nics/af_packet.rst
index 66b977e1a2..a343d3a961 100644
--- a/doc/guides/nics/af_packet.rst
+++ b/doc/guides/nics/af_packet.rst
@@ -69,6 +69,8 @@ framecnt=512):
 Features and Limitations
 ------------------------
 
-The PMD will re-insert the VLAN tag transparently to the packet if the kernel
-strips it, as long as the ``RTE_ETH_RX_OFFLOAD_VLAN_STRIP`` is not enabled by the
-application.
+*  The PMD will re-insert the VLAN tag transparently to the packet if the kernel
+   strips it, as long as the ``RTE_ETH_RX_OFFLOAD_VLAN_STRIP`` is not enabled by the
+   application.
+*  The PMD will add the kernel packet timestamp with nanoseconds resolution and
+   UNIX origo, i.e. time since 1-JAN-1970 UTC, if ``RTE_ETH_RX_OFFLOAD_TIMESTAMP`` is enabled.
diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
index 6b7b16f348..d532105745 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -40,6 +40,9 @@
 #define DFLT_FRAME_SIZE		(1 << 11)
 #define DFLT_FRAME_COUNT	(1 << 9)
 
+static uint64_t timestamp_dynflag;
+static int timestamp_dynfield_offset = -1;
+
 struct __rte_cache_aligned pkt_rx_queue {
 	int sockfd;
 
@@ -51,6 +54,7 @@ struct __rte_cache_aligned pkt_rx_queue {
 	struct rte_mempool *mb_pool;
 	uint16_t in_port;
 	uint8_t vlan_strip;
+	uint8_t timestamp_offloading;
 
 	volatile unsigned long rx_pkts;
 	volatile unsigned long rx_bytes;
@@ -82,6 +86,7 @@ struct pmd_internals {
 	struct pkt_rx_queue *rx_queue;
 	struct pkt_tx_queue *tx_queue;
 	uint8_t vlan_strip;
+	uint8_t timestamp_offloading;
 };
 
 static const char *valid_arguments[] = {
@@ -157,6 +162,16 @@ eth_af_packet_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 				PMD_LOG(ERR, "Failed to reinsert VLAN tag");
 		}
 
+		/* add kernel provided timestamp when offloading is enabled */
+		if (pkt_q->timestamp_offloading) {
+			/* since TPACKET_V2 timestamps are provided in nanoseconds resolution */
+			*RTE_MBUF_DYNFIELD(mbuf, timestamp_dynfield_offset,
+				rte_mbuf_timestamp_t *) =
+					(uint64_t)ppd->tp_sec * 1000000000 + ppd->tp_nsec;
+
+			mbuf->ol_flags |= timestamp_dynflag;
+		}
+
 		/* release incoming frame and advance ring buffer */
 		ppd->tp_status = TP_STATUS_KERNEL;
 		if (++framenum >= framecount)
@@ -317,6 +332,16 @@ eth_dev_start(struct rte_eth_dev *dev)
 	struct pmd_internals *internals = dev->data->dev_private;
 	uint16_t i;
 
+	if (internals->timestamp_offloading) {
+		/* Register mbuf field and flag for Rx timestamp */
+		int rc = rte_mbuf_dyn_rx_timestamp_register(&timestamp_dynfield_offset,
+				&timestamp_dynflag);
+		if (rc) {
+			PMD_LOG(ERR, "Cannot register mbuf field/flag for timestamp");
+			return rc;
+		}
+	}
+
 	dev->data->dev_link.link_status = RTE_ETH_LINK_UP;
 	for (i = 0; i < internals->nb_queues; i++) {
 		dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED;
@@ -365,6 +390,7 @@ eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
 	struct pmd_internals *internals = dev->data->dev_private;
 
 	internals->vlan_strip = !!(rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_STRIP);
+	internals->timestamp_offloading = !!(rxmode->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP);
 	return 0;
 }
 
@@ -381,7 +407,8 @@ eth_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->min_rx_bufsize = 0;
 	dev_info->tx_offload_capa = RTE_ETH_TX_OFFLOAD_MULTI_SEGS |
 		RTE_ETH_TX_OFFLOAD_VLAN_INSERT;
-	dev_info->rx_offload_capa = RTE_ETH_RX_OFFLOAD_VLAN_STRIP;
+	dev_info->rx_offload_capa = RTE_ETH_RX_OFFLOAD_VLAN_STRIP |
+		RTE_ETH_RX_OFFLOAD_TIMESTAMP;
 
 	return 0;
 }
@@ -508,6 +535,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,
 	dev->data->rx_queues[rx_queue_id] = pkt_q;
 	pkt_q->in_port = dev->data->port_id;
 	pkt_q->vlan_strip = internals->vlan_strip;
+	pkt_q->timestamp_offloading = internals->timestamp_offloading;
 
 	return 0;
 }
-- 
2.34.1

Re: [PATCH v3] net/af_packet: add timestamp offloading support

[Stephen Hemminger]

On Fri, 20 Sep 2024 09:14:11 +0200
Stefan Laesser <stefan.laesser@omicronenergy.com> wrote:

> +			*RTE_MBUF_DYNFIELD(mbuf, timestamp_dynfield_offset,
> +				rte_mbuf_timestamp_t *) =
> +					(uint64_t)ppd->tp_sec * 1000000000 + ppd->tp_nsec;

Maybe the should be rte_timespec_to_timestamp() inline in rte_mbuf?

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

[Morten Brørup]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Friday, 20 September 2024 16.30
> 
> On Fri, 20 Sep 2024 09:14:11 +0200
> Stefan Laesser <stefan.laesser@omicronenergy.com> wrote:
> 
> > +			*RTE_MBUF_DYNFIELD(mbuf, timestamp_dynfield_offset,
> > +				rte_mbuf_timestamp_t *) =
> > +					(uint64_t)ppd->tp_sec * 1000000000 + ppd-
> >tp_nsec;
> 
> Maybe the should be rte_timespec_to_timestamp() inline in rte_mbuf?

Yes, when/if timestamp gets standardized to NS after UNIX origo.
And the scope of the time conversion function could be broader than mbufs, so it should reside in some time related header in EAL.

Re: [PATCH v3] net/af_packet: add timestamp offloading support

[Ferruh Yigit]

On 9/20/2024 8:14 AM, Stefan Laesser 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
> and UNIX origo, i.e. time since 1-JAN-1970 UTC.
> 
> Signed-off-by: Stefan Laesser <stefan.laesser@omicronenergy.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>

Applied to dpdk-next-net/main, thanks.