DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Joshua Washington <joshwash@google.com>
Cc: Mark Blasko <blasko@google.com>,
	dev@dpdk.org, Ciara Loftus <ciara.loftus@intel.com>,
	Maryam Tahhan <mtahhan@redhat.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 13:03:30 -0700	[thread overview]
Message-ID: <20260629130330.2bd6a3cf@phoenix.local> (raw)
In-Reply-To: <CALuQH+W4wPA5ygZ63pcQrCD-Uz_K1AsN9n+fzX+ESarmaHnmrA@mail.gmail.com>

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

  parent reply	other threads:[~2026-06-29 20:03 UTC|newest]

Thread overview: 11+ 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
2026-06-29 19:10       ` Joshua Washington
2026-06-29 20:02         ` Stephen Hemminger
2026-06-29 20:03         ` Stephen Hemminger [this message]
2026-06-30  0:41           ` Joshua Washington
2026-07-10 22:10 ` [PATCH v2 0/2] net/af_xdp: add Rx timestamping and read_clock support Mark Blasko
2026-07-10 22:10   ` [PATCH v2 1/2] net/af_xdp: add af_xdp rx metadata and dynamic timestamping support Mark Blasko
2026-07-10 22:10   ` [PATCH v2 2/2] net/af_xdp: add read_clock support to AF_XDP PMD Mark Blasko

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=20260629130330.2bd6a3cf@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox