All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net/af_xdp: add Rx metadata and dynamic timestamping support
@ 2026-06-23 21:53 Mark Blasko
  2026-06-23 22:06 ` Stephen Hemminger
  0 siblings, 1 reply; 8+ messages in thread
From: Mark Blasko @ 2026-06-23 21:53 UTC (permalink / raw)
  To: dev, Ciara Loftus, Maryam Tahhan
  Cc: Mark Blasko, Joshua Washington, Jasper Tran O'Leary

Enable dynamic RX timestamping in the AF_XDP Poll Mode Driver.
This extracts the ingress timestamp prepended to the packet
headroom by the XDP program and populates it in the mbuf.

Signed-off-by: Mark Blasko <blasko@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
Reviewed-by: Jasper Tran O'Leary <jtranoleary@google.com>
---
 doc/guides/rel_notes/release_26_07.rst |  5 +++
 drivers/net/af_xdp/rte_eth_af_xdp.c    | 56 +++++++++++++++++++++++++-
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index 6eba91a5e9..727442258f 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -63,6 +63,11 @@ New Features
     ``rte_eal_init`` and the application is responsible for probing each device,
   * ``--auto-probing`` enables the initial bus probing, which is the current default behavior.
 
+* **Updated AF_XDP ethernet driver.**
+
+  * Added support for dynamic RX metadata and timestamping offload
+    (``RTE_ETH_RX_OFFLOAD_TIMESTAMP``).
+
 * **Added LinkData sxe2 ethernet driver.**
 
   Added network driver for the LinkData network adapters.
diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 2cdb533276..c90e232d57 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -62,6 +62,13 @@
 #define PF_XDP AF_XDP
 #endif
 
+struct af_xdp_rx_metadata {
+	uint64_t rx_timestamp;
+};
+
+static int timestamp_dynfield_offset = -1;
+static uint64_t timestamp_dynflag;
+
 RTE_LOG_REGISTER_DEFAULT(af_xdp_logtype, NOTICE);
 #define RTE_LOGTYPE_NET_AF_XDP af_xdp_logtype
 
@@ -144,6 +151,7 @@ struct pkt_rx_queue {
 	struct pollfd fds[1];
 	int xsk_queue_idx;
 	int busy_budget;
+	bool rx_timestamp_enabled;
 };
 
 struct tx_stats {
@@ -398,6 +406,20 @@ af_xdp_rx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 
 		rte_pktmbuf_pkt_len(bufs[i]) = len;
 		rte_pktmbuf_data_len(bufs[i]) = len;
+
+		if (rxq->rx_timestamp_enabled &&
+		    timestamp_dynfield_offset >= 0) {
+			struct af_xdp_rx_metadata *meta;
+
+			meta = (struct af_xdp_rx_metadata *)
+				((char *)rte_pktmbuf_mtod(bufs[i], void *) -
+				 sizeof(struct af_xdp_rx_metadata));
+			*RTE_MBUF_DYNFIELD(bufs[i],
+					   timestamp_dynfield_offset,
+					   uint64_t *) = meta->rx_timestamp;
+			bufs[i]->ol_flags |= timestamp_dynflag;
+		}
+
 		rx_bytes += len;
 	}
 
@@ -457,6 +479,18 @@ af_xdp_rx_cp(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 		len = desc->len;
 		pkt = xsk_umem__get_data(rxq->umem->mz->addr, addr);
 
+		if (rxq->rx_timestamp_enabled &&
+		    timestamp_dynfield_offset >= 0) {
+			struct af_xdp_rx_metadata *meta;
+
+			meta = (struct af_xdp_rx_metadata *)((char *)pkt -
+				sizeof(struct af_xdp_rx_metadata));
+			*RTE_MBUF_DYNFIELD(mbufs[i],
+					   timestamp_dynfield_offset,
+					   uint64_t *) = meta->rx_timestamp;
+			mbufs[i]->ol_flags |= timestamp_dynflag;
+		}
+
 		rte_memcpy(rte_pktmbuf_mtod(mbufs[i], void *), pkt, len);
 		rte_ring_enqueue(umem->buf_ring, (void *)addr);
 		rte_pktmbuf_pkt_len(mbufs[i]) = len;
@@ -743,6 +777,23 @@ eth_dev_start(struct rte_eth_dev *dev)
 {
 	uint16_t i;
 
+	if (dev->data->dev_conf.rxmode.offloads &
+	    RTE_ETH_RX_OFFLOAD_TIMESTAMP) {
+		int rc;
+
+		rc = rte_mbuf_dyn_rx_timestamp_register(
+				&timestamp_dynfield_offset,
+				&timestamp_dynflag);
+		if (rc) {
+			AF_XDP_LOG_LINE(ERR,
+				"Failed to register mbuf timestamp field");
+			return rc;
+		}
+		AF_XDP_LOG_LINE(INFO,
+			"Registered mbuf timestamp field, offset: %d",
+			timestamp_dynfield_offset);
+	}
+
 	dev->data->dev_link.link_status = RTE_ETH_LINK_UP;
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED;
@@ -870,6 +921,8 @@ eth_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_rx_queues = internals->queue_cnt;
 	dev_info->max_tx_queues = internals->queue_cnt;
 
+	dev_info->rx_offload_capa = RTE_ETH_RX_OFFLOAD_TIMESTAMP;
+
 	dev_info->min_mtu = RTE_ETHER_MIN_MTU;
 #if defined(XDP_UMEM_UNALIGNED_CHUNK_FLAG)
 	dev_info->max_rx_pktlen = getpagesize() -
@@ -1873,7 +1926,8 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,
 	process_private->rxq_xsk_fds[rx_queue_id] = rxq->fds[0].fd;
 
 	rxq->port = dev->data->port_id;
-
+	rxq->rx_timestamp_enabled = !!(dev->data->dev_conf.rxmode.offloads &
+					RTE_ETH_RX_OFFLOAD_TIMESTAMP);
 	dev->data->rx_queues[rx_queue_id] = rxq;
 	return 0;
 
-- 
2.55.0.rc0.799.gd6f94ed593-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] net/af_xdp: add Rx metadata and dynamic timestamping support
  2026-06-23 21:53 [PATCH] net/af_xdp: add Rx metadata and dynamic timestamping support Mark Blasko
@ 2026-06-23 22:06 ` Stephen Hemminger
  2026-06-29  0:50   ` Mark Blasko
  0 siblings, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2026-06-23 22:06 UTC (permalink / raw)
  To: Mark Blasko
  Cc: dev, Ciara Loftus, Maryam Tahhan, Joshua Washington,
	Jasper Tran O'Leary

On Tue, 23 Jun 2026 21:53:24 +0000
Mark Blasko <blasko@google.com> wrote:

> +		if (rxq->rx_timestamp_enabled &&
> +		    timestamp_dynfield_offset >= 0) {
> +			struct af_xdp_rx_metadata *meta;
> +
> +			meta = (struct af_xdp_rx_metadata *)
> +				((char *)rte_pktmbuf_mtod(bufs[i], void *) -
> +				 sizeof(struct af_xdp_rx_metadata));
> +			*RTE_MBUF_DYNFIELD(bufs[i],
> +					   timestamp_dynfield_offset,
> +					   uint64_t *) = meta->rx_timestamp;
> +			bufs[i]->ol_flags |= timestamp_dynflag;
> +		}
> +

Why does XDP time stamp need to be different than how other drivers
already do timestamps. See AF_PACKET and TAP device?

Should not be driver specific here.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] net/af_xdp: add Rx metadata and dynamic timestamping support
  2026-06-23 22:06 ` Stephen Hemminger
@ 2026-06-29  0:50   ` Mark Blasko
  2026-06-29 17:38     ` Stephen Hemminger
  0 siblings, 1 reply; 8+ messages in thread
From: Mark Blasko @ 2026-06-29  0:50 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: dev, Ciara Loftus, Maryam Tahhan, Joshua Washington,
	Jasper Tran O'Leary

In AF_PACKET, the PMD reads the timestamp from socket ring headers
because the kernel stack processes the packet and allocates a socket
buffer. Since the kernel stack and socket buffer allocation are bypassed
in AF_XDP, UMEM frames are given directly to the PMD, which
means the kernel never generates these socket headers. Also, it
looks like the TAP PMD doesn't support RX timestamps.

Since the location of metadata in the UMEM headroom is determined
by the XDP program, the current driver implementation is coupled to
a specific XDP program layout. As an alternative, we could just
plumb the entire metadata headroom to the DPDK application and let
the application parse it based on whatever XDP program is in use.
That would couple the application and the XDP program, but would
at least decouple the driver and the XDP program.

On Tue, Jun 23, 2026 at 3:06 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Tue, 23 Jun 2026 21:53:24 +0000
> Mark Blasko <blasko@google.com> wrote:
>
> > +             if (rxq->rx_timestamp_enabled &&
> > +                 timestamp_dynfield_offset >= 0) {
> > +                     struct af_xdp_rx_metadata *meta;
> > +
> > +                     meta = (struct af_xdp_rx_metadata *)
> > +                             ((char *)rte_pktmbuf_mtod(bufs[i], void *) -
> > +                              sizeof(struct af_xdp_rx_metadata));
> > +                     *RTE_MBUF_DYNFIELD(bufs[i],
> > +                                        timestamp_dynfield_offset,
> > +                                        uint64_t *) = meta->rx_timestamp;
> > +                     bufs[i]->ol_flags |= timestamp_dynflag;
> > +             }
> > +
>
> Why does XDP time stamp need to be different than how other drivers
> already do timestamps. See AF_PACKET and TAP device?
>
> Should not be driver specific here.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] net/af_xdp: add Rx metadata and dynamic timestamping support
  2026-06-29  0:50   ` Mark Blasko
@ 2026-06-29 17:38     ` Stephen Hemminger
  2026-06-29 19:10       ` Joshua Washington
  0 siblings, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2026-06-29 17:38 UTC (permalink / raw)
  To: Mark Blasko
  Cc: dev, Ciara Loftus, Maryam Tahhan, Joshua Washington,
	Jasper Tran O'Leary

On Sun, 28 Jun 2026 17:50:03 -0700
Mark Blasko <blasko@google.com> wrote:

> In AF_PACKET, the PMD reads the timestamp from socket ring headers
> because the kernel stack processes the packet and allocates a socket
> buffer. Since the kernel stack and socket buffer allocation are bypassed
> in AF_XDP, UMEM frames are given directly to the PMD, which
> means the kernel never generates these socket headers. Also, it
> looks like the TAP PMD doesn't support RX timestamps.
> 
> Since the location of metadata in the UMEM headroom is determined
> by the XDP program, the current driver implementation is coupled to
> a specific XDP program layout. As an alternative, we could just
> plumb the entire metadata headroom to the DPDK application and let
> the application parse it based on whatever XDP program is in use.
> That would couple the application and the XDP program, but would
> at least decouple the driver and the XDP program.


Sorry if I was not clear enough before.
The DPDK PMD provides an abstraction to applications to avoid exposing
as many details as possible. Whenever possible a PMD should follow
precedent and implement functions in a manner similar to other drivers.

The method of expressing received timestamps was never well described
in DPDK documentation. The convention is:
  - A dynamic field in mbuf is used for the timestamp.
  - All drivers using timestamp should register the same field
    using rte_mbuf_dyn_rx_timestamp_register.
  - The mbuf field is filled inside the rx_burst processing.
  - The timestamp dynamic field is a 64 bit unsigned number rolling
    clock value.
  - A PMD providing timestamp, must also define a readclock ethdev dev ops
    so that application can compute the number of ticks in timestamp per
    time interval.
  - A PMD providing timestamp must advertise that in offload flags.
  - Timestamp should only be inserted if the Rx timestamp offload flag
    is set during configuration.

When I read this was a little confused about meta data in mbuf.
Would have been clearer with a simple helper:

static inline void
af_xdp_rx_timestamp(struct rte_mbuf *m)
{
	const struct af_xdp_rx_metadata *meta
		= rte_pktmbuf_mtod_offset(m, struct af_xdp_rx_metadata, -(int)sizeof(*meta));

	*RTE_MBUF_DYNFIELD(m, timestamp_dynfield_offset, uint64_t *) = meta->rx_timestamp;
	m->ol_flags |= timestamp_dynflag;
}

If you are going to do Rx timestamp then a simple readclock ethdev op
is also needed to tell the application what the units are.

But there are bigger issues with this patch:
1. The patch assumes metadata is always present in XDP receive but device
used by XDP may not implement it. There is no capability checking.
The DPDK PMD ends up advertising RTE_ETH_RX_OFFLOAD_TIMESTAMP
unconditionally even if underling kernel device doesn't do it.

2. The format of metadata is not a documented contract between kernel
device implementing XDP and the DPDK.

3. The XDP documentation says you need to check for metadata
in each frame.

4. There are no head bounds checks; must check that there is
packet headroom is configured with enough space for metadata.

I am sure AI will find several more things but need fixing
before ready to merge.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] net/af_xdp: add Rx metadata and dynamic timestamping support
  2026-06-29 17:38     ` Stephen Hemminger
@ 2026-06-29 19:10       ` Joshua Washington
  2026-06-29 20:02         ` Stephen Hemminger
  2026-06-29 20:03         ` Stephen Hemminger
  0 siblings, 2 replies; 8+ messages in thread
From: Joshua Washington @ 2026-06-29 19:10 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Mark Blasko, dev, Ciara Loftus, Maryam Tahhan,
	Jasper Tran O'Leary

On Mon, Jun 29, 2026 at 10:38 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Sun, 28 Jun 2026 17:50:03 -0700
> Mark Blasko <blasko@google.com> wrote:
>
> > In AF_PACKET, the PMD reads the timestamp from socket ring headers
> > because the kernel stack processes the packet and allocates a socket
> > buffer. Since the kernel stack and socket buffer allocation are bypassed
> > in AF_XDP, UMEM frames are given directly to the PMD, which
> > means the kernel never generates these socket headers. Also, it
> > looks like the TAP PMD doesn't support RX timestamps.
> >
> > Since the location of metadata in the UMEM headroom is determined
> > by the XDP program, the current driver implementation is coupled to
> > a specific XDP program layout. As an alternative, we could just
> > plumb the entire metadata headroom to the DPDK application and let
> > the application parse it based on whatever XDP program is in use.
> > That would couple the application and the XDP program, but would
> > at least decouple the driver and the XDP program.
>
>
> Sorry if I was not clear enough before.
> The DPDK PMD provides an abstraction to applications to avoid exposing
> as many details as possible. Whenever possible a PMD should follow
> precedent and implement functions in a manner similar to other drivers.
>
> The method of expressing received timestamps was never well described
> in DPDK documentation. The convention is:
>   - A dynamic field in mbuf is used for the timestamp.
>   - All drivers using timestamp should register the same field
>     using rte_mbuf_dyn_rx_timestamp_register.
>   - The mbuf field is filled inside the rx_burst processing.
>   - The timestamp dynamic field is a 64 bit unsigned number rolling
>     clock value.
>   - A PMD providing timestamp, must also define a readclock ethdev dev ops
>     so that application can compute the number of ticks in timestamp per
>     time interval.
>   - A PMD providing timestamp must advertise that in offload flags.
>   - Timestamp should only be inserted if the Rx timestamp offload flag
>     is set during configuration.

These are all very good points for ensuring an airtight abstraction.
However, I'd like to note that neither of the kernel-bound PMD
interfaces (pcap, AF_PACKET) have support for the `read_clock` op. I
am not certain of the reason for this, but I suspect it's because
ethtool IOCTLs already cover that functionality. This patch's
implementation follows the other implementations pretty closely,
differing only in how the timestamp is extracted.

>
> When I read this was a little confused about meta data in mbuf.
> Would have been clearer with a simple helper:
>
> static inline void
> af_xdp_rx_timestamp(struct rte_mbuf *m)
> {
>         const struct af_xdp_rx_metadata *meta
>                 = rte_pktmbuf_mtod_offset(m, struct af_xdp_rx_metadata, -(int)sizeof(*meta));
>
>         *RTE_MBUF_DYNFIELD(m, timestamp_dynfield_offset, uint64_t *) = meta->rx_timestamp;
>         m->ol_flags |= timestamp_dynflag;
> }
>
> If you are going to do Rx timestamp then a simple readclock ethdev op
> is also needed to tell the application what the units are.
>
> But there are bigger issues with this patch:
> 1. The patch assumes metadata is always present in XDP receive but device
> used by XDP may not implement it. There is no capability checking.
> The DPDK PMD ends up advertising RTE_ETH_RX_OFFLOAD_TIMESTAMP
> unconditionally even if underling kernel device doesn't do it.

The patch as submitted is incorrect; my apologies for that. I missed
it in review. The layout of XDP metadata is something which is
determined by the XDP program and consumed by an AF_XDP application
tailor-made for the XDP program. This means that the PMD itself would
not be able to process the RX timestamp from an XDP program on its
own; that functionality would have to be handled by the DPDK
application.

>
> 2. The format of metadata is not a documented contract between kernel
> device implementing XDP and the DPDK.

This is true. The contract lies between the XDP program and the AF_XDP
application, which can both be controlled by the DPDK application and
its invocation.

>
> 3. The XDP documentation says you need to check for metadata
> in each frame.
>
> 4. There are no head bounds checks; must check that there is
> packet headroom is configured with enough space for metadata.
>
> I am sure AI will find several more things but need fixing
> before ready to merge.
>

Unfortunately, I think that some amount of abstraction leakage is
unavoidable by the very nature of AF_XDP. What I propose is as
follows:

Introduce a new PMD capability for processing XDP metadata. The mbuf
dyn_fields can store a pointer to the metadata and the size of the
metadata. The DPDK application processes the metadata as it sees fit.
As mentioned before, both the reader and the writer of the XDP
metadata are both controllable, and this would add the flexiblity for
one to use pre-existing DPDK mbuf infrastructure with minor
modifications to support this functionality, rather than introducing a
full rewrite using an AF_XDP application.

Would this be acceptable?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] net/af_xdp: add Rx metadata and dynamic timestamping support
  2026-06-29 19:10       ` Joshua Washington
@ 2026-06-29 20:02         ` Stephen Hemminger
  2026-06-29 20:03         ` Stephen Hemminger
  1 sibling, 0 replies; 8+ messages in thread
From: Stephen Hemminger @ 2026-06-29 20:02 UTC (permalink / raw)
  To: Joshua Washington
  Cc: Mark Blasko, dev, Ciara Loftus, Maryam Tahhan,
	Jasper Tran O'Leary

On Mon, 29 Jun 2026 12:10:27 -0700
Joshua Washington <joshwash@google.com> wrote:

> These are all very good points for ensuring an airtight abstraction.
> However, I'd like to note that neither of the kernel-bound PMD
> interfaces (pcap, AF_PACKET) have support for the `read_clock` op. I
> am not certain of the reason for this, but I suspect it's because
> ethtool IOCTLs already cover that functionality. This patch's
> implementation follows the other implementations pretty closely,
> differing only in how the timestamp is extracted.

Yes, that needs to be fixed :-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] net/af_xdp: add Rx metadata and dynamic timestamping support
  2026-06-29 19:10       ` Joshua Washington
  2026-06-29 20:02         ` Stephen Hemminger
@ 2026-06-29 20:03         ` Stephen Hemminger
  2026-06-30  0:41           ` Joshua Washington
  1 sibling, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2026-06-29 20:03 UTC (permalink / raw)
  To: Joshua Washington
  Cc: Mark Blasko, dev, Ciara Loftus, Maryam Tahhan,
	Jasper Tran O'Leary

On Mon, 29 Jun 2026 12:10:27 -0700
Joshua Washington <joshwash@google.com> wrote:

> On Mon, Jun 29, 2026 at 10:38 AM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Sun, 28 Jun 2026 17:50:03 -0700
> > Mark Blasko <blasko@google.com> wrote:
> >  
> > > In AF_PACKET, the PMD reads the timestamp from socket ring headers
> > > because the kernel stack processes the packet and allocates a socket
> > > buffer. Since the kernel stack and socket buffer allocation are bypassed
> > > in AF_XDP, UMEM frames are given directly to the PMD, which
> > > means the kernel never generates these socket headers. Also, it
> > > looks like the TAP PMD doesn't support RX timestamps.
> > >
> > > Since the location of metadata in the UMEM headroom is determined
> > > by the XDP program, the current driver implementation is coupled to
> > > a specific XDP program layout. As an alternative, we could just
> > > plumb the entire metadata headroom to the DPDK application and let
> > > the application parse it based on whatever XDP program is in use.
> > > That would couple the application and the XDP program, but would
> > > at least decouple the driver and the XDP program.  
> >
> >
> > Sorry if I was not clear enough before.
> > The DPDK PMD provides an abstraction to applications to avoid exposing
> > as many details as possible. Whenever possible a PMD should follow
> > precedent and implement functions in a manner similar to other drivers.
> >
> > The method of expressing received timestamps was never well described
> > in DPDK documentation. The convention is:
> >   - A dynamic field in mbuf is used for the timestamp.
> >   - All drivers using timestamp should register the same field
> >     using rte_mbuf_dyn_rx_timestamp_register.
> >   - The mbuf field is filled inside the rx_burst processing.
> >   - The timestamp dynamic field is a 64 bit unsigned number rolling
> >     clock value.
> >   - A PMD providing timestamp, must also define a readclock ethdev dev ops
> >     so that application can compute the number of ticks in timestamp per
> >     time interval.
> >   - A PMD providing timestamp must advertise that in offload flags.
> >   - Timestamp should only be inserted if the Rx timestamp offload flag
> >     is set during configuration.  
> 
> These are all very good points for ensuring an airtight abstraction.
> However, I'd like to note that neither of the kernel-bound PMD
> interfaces (pcap, AF_PACKET) have support for the `read_clock` op. I
> am not certain of the reason for this, but I suspect it's because
> ethtool IOCTLs already cover that functionality. This patch's
> implementation follows the other implementations pretty closely,
> differing only in how the timestamp is extracted.
> 
> >
> > When I read this was a little confused about meta data in mbuf.
> > Would have been clearer with a simple helper:
> >
> > static inline void
> > af_xdp_rx_timestamp(struct rte_mbuf *m)
> > {
> >         const struct af_xdp_rx_metadata *meta
> >                 = rte_pktmbuf_mtod_offset(m, struct af_xdp_rx_metadata, -(int)sizeof(*meta));
> >
> >         *RTE_MBUF_DYNFIELD(m, timestamp_dynfield_offset, uint64_t *) = meta->rx_timestamp;
> >         m->ol_flags |= timestamp_dynflag;
> > }
> >
> > If you are going to do Rx timestamp then a simple readclock ethdev op
> > is also needed to tell the application what the units are.
> >
> > But there are bigger issues with this patch:
> > 1. The patch assumes metadata is always present in XDP receive but device
> > used by XDP may not implement it. There is no capability checking.
> > The DPDK PMD ends up advertising RTE_ETH_RX_OFFLOAD_TIMESTAMP
> > unconditionally even if underling kernel device doesn't do it.  
> 
> The patch as submitted is incorrect; my apologies for that. I missed
> it in review. The layout of XDP metadata is something which is
> determined by the XDP program and consumed by an AF_XDP application
> tailor-made for the XDP program. This means that the PMD itself would
> not be able to process the RX timestamp from an XDP program on its
> own; that functionality would have to be handled by the DPDK
> application.
> 
> >
> > 2. The format of metadata is not a documented contract between kernel
> > device implementing XDP and the DPDK.  
> 
> This is true. The contract lies between the XDP program and the AF_XDP
> application, which can both be controlled by the DPDK application and
> its invocation.
> 
> >
> > 3. The XDP documentation says you need to check for metadata
> > in each frame.
> >
> > 4. There are no head bounds checks; must check that there is
> > packet headroom is configured with enough space for metadata.
> >
> > I am sure AI will find several more things but need fixing
> > before ready to merge.
> >  
> 
> Unfortunately, I think that some amount of abstraction leakage is
> unavoidable by the very nature of AF_XDP. What I propose is as
> follows:
> 
> Introduce a new PMD capability for processing XDP metadata. The mbuf
> dyn_fields can store a pointer to the metadata and the size of the
> metadata. The DPDK application processes the metadata as it sees fit.
> As mentioned before, both the reader and the writer of the XDP
> metadata are both controllable, and this would add the flexiblity for
> one to use pre-existing DPDK mbuf infrastructure with minor
> modifications to support this functionality, rather than introducing a
> full rewrite using an AF_XDP application.
> 
> Would this be acceptable?

There needs to be an API that XDP PMD can call to check if underlying
driver will add metadata; and another API to check that buffer has
valid timestamp

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] net/af_xdp: add Rx metadata and dynamic timestamping support
  2026-06-29 20:03         ` Stephen Hemminger
@ 2026-06-30  0:41           ` Joshua Washington
  0 siblings, 0 replies; 8+ messages in thread
From: Joshua Washington @ 2026-06-30  0:41 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Mark Blasko, dev, Ciara Loftus, Maryam Tahhan,
	Jasper Tran O'Leary

On Mon, Jun 29, 2026 at 1:03 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 29 Jun 2026 12:10:27 -0700
> Joshua Washington <joshwash@google.com> wrote:
>
> > On Mon, Jun 29, 2026 at 10:38 AM Stephen Hemminger
> > <stephen@networkplumber.org> wrote:
> > >
> > > On Sun, 28 Jun 2026 17:50:03 -0700
> > > Mark Blasko <blasko@google.com> wrote:
> > >
> > > > In AF_PACKET, the PMD reads the timestamp from socket ring headers
> > > > because the kernel stack processes the packet and allocates a socket
> > > > buffer. Since the kernel stack and socket buffer allocation are bypassed
> > > > in AF_XDP, UMEM frames are given directly to the PMD, which
> > > > means the kernel never generates these socket headers. Also, it
> > > > looks like the TAP PMD doesn't support RX timestamps.
> > > >
> > > > Since the location of metadata in the UMEM headroom is determined
> > > > by the XDP program, the current driver implementation is coupled to
> > > > a specific XDP program layout. As an alternative, we could just
> > > > plumb the entire metadata headroom to the DPDK application and let
> > > > the application parse it based on whatever XDP program is in use.
> > > > That would couple the application and the XDP program, but would
> > > > at least decouple the driver and the XDP program.
> > >
> > >
> > > Sorry if I was not clear enough before.
> > > The DPDK PMD provides an abstraction to applications to avoid exposing
> > > as many details as possible. Whenever possible a PMD should follow
> > > precedent and implement functions in a manner similar to other drivers.
> > >
> > > The method of expressing received timestamps was never well described
> > > in DPDK documentation. The convention is:
> > >   - A dynamic field in mbuf is used for the timestamp.
> > >   - All drivers using timestamp should register the same field
> > >     using rte_mbuf_dyn_rx_timestamp_register.
> > >   - The mbuf field is filled inside the rx_burst processing.
> > >   - The timestamp dynamic field is a 64 bit unsigned number rolling
> > >     clock value.
> > >   - A PMD providing timestamp, must also define a readclock ethdev dev ops
> > >     so that application can compute the number of ticks in timestamp per
> > >     time interval.
> > >   - A PMD providing timestamp must advertise that in offload flags.
> > >   - Timestamp should only be inserted if the Rx timestamp offload flag
> > >     is set during configuration.
> >
> > These are all very good points for ensuring an airtight abstraction.
> > However, I'd like to note that neither of the kernel-bound PMD
> > interfaces (pcap, AF_PACKET) have support for the `read_clock` op. I
> > am not certain of the reason for this, but I suspect it's because
> > ethtool IOCTLs already cover that functionality. This patch's
> > implementation follows the other implementations pretty closely,
> > differing only in how the timestamp is extracted.
> >
> > >
> > > When I read this was a little confused about meta data in mbuf.
> > > Would have been clearer with a simple helper:
> > >
> > > static inline void
> > > af_xdp_rx_timestamp(struct rte_mbuf *m)
> > > {
> > >         const struct af_xdp_rx_metadata *meta
> > >                 = rte_pktmbuf_mtod_offset(m, struct af_xdp_rx_metadata, -(int)sizeof(*meta));
> > >
> > >         *RTE_MBUF_DYNFIELD(m, timestamp_dynfield_offset, uint64_t *) = meta->rx_timestamp;
> > >         m->ol_flags |= timestamp_dynflag;
> > > }
> > >
> > > If you are going to do Rx timestamp then a simple readclock ethdev op
> > > is also needed to tell the application what the units are.
> > >
> > > But there are bigger issues with this patch:
> > > 1. The patch assumes metadata is always present in XDP receive but device
> > > used by XDP may not implement it. There is no capability checking.
> > > The DPDK PMD ends up advertising RTE_ETH_RX_OFFLOAD_TIMESTAMP
> > > unconditionally even if underling kernel device doesn't do it.
> >
> > The patch as submitted is incorrect; my apologies for that. I missed
> > it in review. The layout of XDP metadata is something which is
> > determined by the XDP program and consumed by an AF_XDP application
> > tailor-made for the XDP program. This means that the PMD itself would
> > not be able to process the RX timestamp from an XDP program on its
> > own; that functionality would have to be handled by the DPDK
> > application.
> >
> > >
> > > 2. The format of metadata is not a documented contract between kernel
> > > device implementing XDP and the DPDK.
> >
> > This is true. The contract lies between the XDP program and the AF_XDP
> > application, which can both be controlled by the DPDK application and
> > its invocation.
> >
> > >
> > > 3. The XDP documentation says you need to check for metadata
> > > in each frame.
> > >
> > > 4. There are no head bounds checks; must check that there is
> > > packet headroom is configured with enough space for metadata.
> > >
> > > I am sure AI will find several more things but need fixing
> > > before ready to merge.
> > >
> >
> > Unfortunately, I think that some amount of abstraction leakage is
> > unavoidable by the very nature of AF_XDP. What I propose is as
> > follows:
> >
> > Introduce a new PMD capability for processing XDP metadata. The mbuf
> > dyn_fields can store a pointer to the metadata and the size of the
> > metadata. The DPDK application processes the metadata as it sees fit.
> > As mentioned before, both the reader and the writer of the XDP
> > metadata are both controllable, and this would add the flexiblity for
> > one to use pre-existing DPDK mbuf infrastructure with minor
> > modifications to support this functionality, rather than introducing a
> > full rewrite using an AF_XDP application.
> >
> > Would this be acceptable?
>
> There needs to be an API that XDP PMD can call to check if underlying
> driver will add metadata; and another API to check that buffer has
> valid timestamp

I don't think the API to check if the driver has metadata support
exists, unfortunately. bpf_xdp_adjust_meta() is used to by XDP
programs to adjust the metadata pointer on a packet.

If xdp_md->data_meta > xdp_md->data then metadata isn't supported, and
the adjust call will fail. Otherwise, drivers have up to 256B of
metadata. What this means for the PMD is that it can read data_meta
and data pointers to decide if the metadata is valid on a per-packet
basis, and pass the metadata to the application when appropriate.

We were thinking to implement this via the vdev parameters passed in
as part of invoking the application. This solution would introduce
three new parameters:

1. `xdp_meta_valid_hint_offset` (byte offset of RX validity flag(s))
2. `xdp_meta_rx_ts_valid_mask` (mask to read if the RX timesamp is
valid. Mask covers a single byte.)
3. `xdp_meta_rx_ts_offset` (offset of RX timestamp in XDP metadata. RX
timestamp is assumed to be 8 bytes by kernel convention.)

modeled after xdp_metadata.h in the kernel. This solution would be
quite verbose, so I'm open to suggested improvements. If the valid
mask/hint offset are not passed in the invocation parameters, the
timestamp would be assumed valid if the metadata is valid and has
enough bytes available to feasibly read the timestamp value at the
intended offset.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-06-30  0:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-23 21:53 [PATCH] net/af_xdp: add Rx metadata and dynamic timestamping support Mark Blasko
2026-06-23 22:06 ` Stephen Hemminger
2026-06-29  0:50   ` Mark Blasko
2026-06-29 17:38     ` Stephen Hemminger
2026-06-29 19:10       ` Joshua Washington
2026-06-29 20:02         ` Stephen Hemminger
2026-06-29 20:03         ` Stephen Hemminger
2026-06-30  0:41           ` Joshua Washington

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.