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@xxxxxxxxxx>
> > > +/* 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?
> > > +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.
