Commit d5e726d9 authored by Stanislav Fomichev's avatar Stanislav Fomichev Committed by Daniel Borkmann
Browse files

xsk: Require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len 



Julian reports that commit 341ac980 ("xsk: Support tx_metadata_len")
can break existing use cases which don't zero-initialize xdp_umem_reg
padding. Introduce new XDP_UMEM_TX_METADATA_LEN to make sure we
interpret the padding as tx_metadata_len only when being explicitly
asked.

Fixes: 341ac980 ("xsk: Support tx_metadata_len")
Reported-by: default avatarJulian Schindel <mail@arctic-alpaca.de>
Signed-off-by: default avatarStanislav Fomichev <sdf@fomichev.me>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Reviewed-by: default avatarMaciej Fijalkowski <maciej.fijalkowski@intel.com>
Link: https://lore.kernel.org/bpf/20240713015253.121248-2-sdf@fomichev.me
parent fa5ef655

Documentation/networking/xsk-tx-metadata.rst

+10 −6
Original line number Diff line number Diff line
@@ -11,12 +11,16 @@ metadata on the receive side. 
General Design

==============



The headroom for the metadata is reserved via ``tx_metadata_len`` in

``struct xdp_umem_reg``. The metadata length is therefore the same for

every socket that shares the same umem. The metadata layout is a fixed UAPI,

refer to ``union xsk_tx_metadata`` in ``include/uapi/linux/if_xdp.h``.

Thus, generally, the ``tx_metadata_len`` field above should contain

``sizeof(union xsk_tx_metadata)``.

The headroom for the metadata is reserved via ``tx_metadata_len`` and

``XDP_UMEM_TX_METADATA_LEN`` flag in ``struct xdp_umem_reg``. The metadata

length is therefore the same for every socket that shares the same umem.

The metadata layout is a fixed UAPI, refer to ``union xsk_tx_metadata`` in

``include/uapi/linux/if_xdp.h``. Thus, generally, the ``tx_metadata_len``

field above should contain ``sizeof(union xsk_tx_metadata)``.



Note that in the original implementation the ``XDP_UMEM_TX_METADATA_LEN``

flag was not required. Applications might attempt to create a umem

with a flag first and if it fails, do another attempt without a flag.



The headroom and the metadata itself should be located right before

``xdp_desc->addr`` in the umem frame. Within a frame, the metadata

include/uapi/linux/if_xdp.h

+4 −0
Original line number Diff line number Diff line
@@ -41,6 +41,10 @@ 
 */

#define XDP_UMEM_TX_SW_CSUM		(1 << 1)



/* Request to reserve tx_metadata_len bytes of per-chunk metadata.

 */

#define XDP_UMEM_TX_METADATA_LEN	(1 << 2)



struct sockaddr_xdp {

	__u16 sxdp_family;

	__u16 sxdp_flags;

net/xdp/xdp_umem.c

+6 −3
Original line number Diff line number Diff line
@@ -151,6 +151,7 @@ static int xdp_umem_account_pages(struct xdp_umem *umem) 
#define XDP_UMEM_FLAGS_VALID ( \

		XDP_UMEM_UNALIGNED_CHUNK_FLAG | \

		XDP_UMEM_TX_SW_CSUM | \

		XDP_UMEM_TX_METADATA_LEN | \

	0)



static int xdp_umem_reg(struct xdp_umem *umem, struct xdp_umem_reg *mr)
@@ -204,8 +205,11 @@ static int xdp_umem_reg(struct xdp_umem *umem, struct xdp_umem_reg *mr) 
	if (headroom >= chunk_size - XDP_PACKET_HEADROOM)

		return -EINVAL;



	if (mr->flags & XDP_UMEM_TX_METADATA_LEN) {

		if (mr->tx_metadata_len >= 256 || mr->tx_metadata_len % 8)

			return -EINVAL;

		umem->tx_metadata_len = mr->tx_metadata_len;

	}



	umem->size = size;

	umem->headroom = headroom;
@@ -215,7 +219,6 @@ static int xdp_umem_reg(struct xdp_umem *umem, struct xdp_umem_reg *mr) 
	umem->pgs = NULL;

	umem->user = NULL;

	umem->flags = mr->flags;

	umem->tx_metadata_len = mr->tx_metadata_len;



	INIT_LIST_HEAD(&umem->xsk_dma_list);

	refcount_set(&umem->users, 1);