From: Stephen Hemminger <stephen@networkplumber.org>
To: Mark Blasko <blasko@google.com>
Cc: dev@dpdk.org, Ciara Loftus <ciara.loftus@intel.com>,
Maryam Tahhan <mtahhan@redhat.com>,
Joshua Washington <joshwash@google.com>,
"Jasper Tran O'Leary" <jtranoleary@google.com>
Subject: Re: [PATCH] net/af_xdp: add Rx metadata and dynamic timestamping support
Date: Mon, 29 Jun 2026 10:38:04 -0700 [thread overview]
Message-ID: <20260629103804.3d4154bb@phoenix.local> (raw)
In-Reply-To: <CANULgnK9-P5dQtTPayiFD7PLor=1qV=7du-AtRuWGPd4F5SmAQ@mail.gmail.com>
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.
next prev parent reply other threads:[~2026-06-29 17:38 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260629103804.3d4154bb@phoenix.local \
--to=stephen@networkplumber.org \
--cc=blasko@google.com \
--cc=ciara.loftus@intel.com \
--cc=dev@dpdk.org \
--cc=joshwash@google.com \
--cc=jtranoleary@google.com \
--cc=mtahhan@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.