XDP hardware hints discussion mail archive
 help / color / mirror / Atom feed
From: Stanislav Fomichev <sdf@google.com>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
	andrii@kernel.org, martin.lau@linux.dev, song@kernel.org,
	yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org,
	haoluo@google.com, jolsa@kernel.org, kuba@kernel.org,
	toke@kernel.org, willemb@google.com, dsahern@kernel.org,
	magnus.karlsson@intel.com, bjorn@kernel.org,
	maciej.fijalkowski@intel.com, hawk@kernel.org,
	netdev@vger.kernel.org, xdp-hints@xdp-project.net
Subject: [xdp-hints] Re: [RFC net-next v4 2/8] xsk: add TX timestamp and TX checksum offload support
Date: Tue, 25 Jul 2023 16:48:20 -0700	[thread overview]
Message-ID: <ZMBfRPxMk0F45a/s@google.com> (raw)
In-Reply-To: <64c056686b527_3a4d294e6@willemb.c.googlers.com.notmuch>

On 07/25, Willem de Bruijn wrote:
> Stanislav Fomichev wrote:
> > On 07/25, Willem de Bruijn wrote:
> > > Stanislav Fomichev wrote:
> > > > This change actually defines the (initial) metadata layout
> > > > that should be used by AF_XDP userspace (xsk_tx_metadata).
> > > > The first field is flags which requests appropriate offloads,
> > > > followed by the offload-specific fields. The supported per-device
> > > > offloads are exported via netlink (new xsk-flags).
> > > > 
> > > > The offloads themselves are still implemented in a bit of a
> > > > framework-y fashion that's left from my initial kfunc attempt.
> > > > I'm introducing new xsk_tx_metadata_ops which drivers are
> > > > supposed to implement. The drivers are also supposed
> > > > to call xsk_tx_metadata_request/xsk_tx_metadata_complete in
> > > > the right places. Since xsk_tx_metadata_{request,_complete}
> > > > are static inline, we don't incur any extra overhead doing
> > > > indirect calls.
> > > > 
> > > > The benefit of this scheme is as follows:
> > > > - keeps all metadata layout parsing away from driver code
> > > > - makes it easy to grep and see which drivers implement what
> > > > - don't need any extra flags to maintain to keep track of that
> > > >   offloads are implemented; if the callback is implemented - the offload
> > > >   is supported (used by netlink reporting code)
> > > > 
> > > > Two offloads are defined right now:
> > > > 1. XDP_TX_METADATA_CHECKSUM: skb-style csum_start+csum_offset
> > > > 2. XDP_TX_METADATA_TIMESTAMP: writes TX timestamp back into metadata
> > > >    area upon completion (tx_timestamp field)
> > > > 
> > > > The offloads are also implemented for copy mode:
> > > > 1. Extra XDP_TX_METADATA_CHECKSUM_SW to trigger skb_checksum_help; this
> > > >    might be useful as a reference implementation and for testing
> > > > 2. XDP_TX_METADATA_TIMESTAMP writes SW timestamp from the skb
> > > >    destructor (note I'm reusing hwtstamps to pass metadata pointer)
> > > > 
> > > > The struct is forward-compatible and can be extended in the future
> > > > by appending more fields.
> > > > 
> > > > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> 
> > > > +/* Request transmit checksum offload. Checksum start position and offset
> > > > + * are communicated via csum_start and csum_offset fields of struct
> > > > + * xsk_tx_metadata.
> > > > + */
> > > > +#define XDP_TX_METADATA_CHECKSUM		(1 << 1)
> > > > +
> > > > +/* Force checksum calculation in software. Can be used for testing or
> > > > + * working around potential HW issues. This option causes performance
> > > > + * degradation and only works in XDP_COPY mode.
> > > > + */
> > > > +#define XDP_TX_METADATA_CHECKSUM_SW		(1 << 2)
> > > 
> > > Not sure how useful this is, especially if only for copy mode.
> > 
> > Seems useful at least as a reference implementation? But I'm happy
> > to drop. It's used only in the tests for now. I was using it to
> > verify csum_offset/start field values.
> 
> If testing over veth, does anything even look at the checksum?

My receiver in the xdp_metadata test looks at it and compares to the
fixed (verified) value:

	ASSERT_EQ(udph->check, 0x1c72, "csum");

The packet is always the same (and macs are fixed), so we are able
to do that.
 
> > > > +struct xsk_tx_metadata {
> > > > +	__u32 flags;
> > > > +
> > > > +	/* XDP_TX_METADATA_CHECKSUM */
> > > > +
> > > > +	/* Offset from desc->addr where checksumming should start. */
> > > > +	__u16 csum_start;
> > > > +	/* Offset from csum_start where checksum should be stored. */
> > > > +	__u16 csum_offset;
> > > > +
> > > > +	/* XDP_TX_METADATA_TIMESTAMP */
> > > > +
> > > > +	__u64 tx_timestamp;
> > > > +};
> > > 
> > > Is this structure easily extensible for future offloads,
> > > such as USO?
> > 
> > We can append more field. What do we need for USO? Something akin
> > to gso_size/gso_segs/gso_type ?
> 
> Yes, a bit to set the feature (gso_type) and a field to store the
> segment size (gso_size).
> 
> Pacing offload is the other feature that comes to mind. That could
> conceivably use the tx_timestamp field.

Right, so we can append to this struct and add more XDP_TX_METADATA_$(FLAG)s
to signal various features. Jakub mentioned that it might be handy
to pass l2_offset/l3_offset/l4_offset, but I'm not sure whether
he was talking about xSO offloads or something else.

  reply	other threads:[~2023-07-25 23:48 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-24 23:59 [xdp-hints] [RFC net-next v4 0/8] xsk: TX metadata Stanislav Fomichev
2023-07-24 23:59 ` [xdp-hints] [RFC net-next v4 1/8] xsk: Support XDP_TX_METADATA_LEN Stanislav Fomichev
2023-07-24 23:59 ` [xdp-hints] [RFC net-next v4 2/8] xsk: add TX timestamp and TX checksum offload support Stanislav Fomichev
2023-07-25 19:28   ` [xdp-hints] " Simon Horman
2023-07-25 20:30     ` Stanislav Fomichev
2023-07-25 21:28       ` Jakub Kicinski
2023-07-25 22:40         ` Stanislav Fomichev
2023-07-26  7:47           ` Simon Horman
2023-07-25 20:54   ` Willem de Bruijn
2023-07-25 22:39     ` Stanislav Fomichev
2023-07-25 23:10       ` Willem de Bruijn
2023-07-25 23:48         ` Stanislav Fomichev [this message]
2023-07-27 14:11         ` Jesper Dangaard Brouer
2023-07-27 16:37           ` Stanislav Fomichev
2023-07-25 20:58   ` Willem de Bruijn
2023-07-28 11:56     ` Jesper Dangaard Brouer
2023-07-28 13:19       ` Willem de Bruijn
2023-07-26 19:25   ` Jesper Dangaard Brouer
2023-07-26 21:25     ` Stanislav Fomichev
2023-07-27 13:50       ` Jesper Dangaard Brouer
2023-07-27 16:34         ` Stanislav Fomichev
2023-07-24 23:59 ` [xdp-hints] [RFC net-next v4 3/8] net/mlx5e: Implement AF_XDP TX timestamp and checksum offload Stanislav Fomichev
2023-07-24 23:59 ` [xdp-hints] [RFC net-next v4 4/8] tools: ynl: update netdev sample to dump xsk-features Stanislav Fomichev
2023-07-24 23:59 ` [xdp-hints] [RFC net-next v4 5/8] selftests/xsk: Support XDP_TX_METADATA_LEN Stanislav Fomichev
2023-07-24 23:59 ` [xdp-hints] [RFC net-next v4 6/8] selftests/bpf: Add csum helpers Stanislav Fomichev
2023-07-24 23:59 ` [xdp-hints] [RFC net-next v4 7/8] selftests/bpf: Add TX side to xdp_metadata Stanislav Fomichev
2023-07-24 23:59 ` [xdp-hints] [RFC net-next v4 8/8] selftests/bpf: Add TX side to xdp_hw_metadata Stanislav Fomichev
2023-07-25 20:59   ` [xdp-hints] " Willem de Bruijn
2023-07-25 22:36     ` Stanislav Fomichev
2023-07-25 22:55       ` Willem de Bruijn

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

  List information: https://lists.xdp-project.net/postorius/lists/xdp-hints.xdp-project.net/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZMBfRPxMk0F45a/s@google.com \
    --to=sdf@google.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bjorn@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=dsahern@kernel.org \
    --cc=haoluo@google.com \
    --cc=hawk@kernel.org \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=kuba@kernel.org \
    --cc=maciej.fijalkowski@intel.com \
    --cc=magnus.karlsson@intel.com \
    --cc=martin.lau@linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=song@kernel.org \
    --cc=toke@kernel.org \
    --cc=willemb@google.com \
    --cc=willemdebruijn.kernel@gmail.com \
    --cc=xdp-hints@xdp-project.net \
    --cc=yhs@fb.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