[PATCH bpf 0/3] xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len

[PATCH bpf 0/3] xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len

[Stanislav Fomichev]

Julian reports that commit 341ac980eab9 ("xsk: Support tx_metadata_len")
can break existing use cases which don't zero-initialize xdp_umem_reg
padding. Fix it (while still breaking a minority of new users of tx
metadata), update the docs, update the selftest and sprinkle some
BUILD_BUG_ONs to hopefully catch similar issues in the future.

Thank you Julian for the report and for helping to chase it down!

Reported-by: Julian Schindel <mail@arctic-alpaca.de>
Cc: Magnus Karlsson <magnus.karlsson@gmail.com>

Stanislav Fomichev (3):
  xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len
  selftests/bpf: Add XDP_UMEM_TX_METADATA_LEN to XSK TX metadata test
  xsk: Try to make xdp_umem_reg extension a bit more future-proof

 Documentation/networking/xsk-tx-metadata.rst  | 16 ++++++++-----
 include/uapi/linux/if_xdp.h                   |  4 ++++
 net/xdp/xdp_umem.c                            |  9 +++++---
 net/xdp/xsk.c                                 | 23 ++++++++++---------
 tools/include/uapi/linux/if_xdp.h             |  4 ++++
 .../selftests/bpf/prog_tests/xdp_metadata.c   |  3 ++-
 6 files changed, 38 insertions(+), 21 deletions(-)

-- 
2.45.2

[PATCH bpf 1/3] xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len

[Stanislav Fomichev]

Julian reports that commit 341ac980eab9 ("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: 341ac980eab9 ("xsk: Support tx_metadata_len")
Reported-by: Julian Schindel <mail@arctic-alpaca.de>
Cc: Magnus Karlsson <magnus.karlsson@gmail.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 Documentation/networking/xsk-tx-metadata.rst | 16 ++++++++++------
 include/uapi/linux/if_xdp.h                  |  4 ++++
 net/xdp/xdp_umem.c                           |  9 ++++++---
 3 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/Documentation/networking/xsk-tx-metadata.rst b/Documentation/networking/xsk-tx-metadata.rst
index bd033fe95cca..e76b0cfc32f7 100644
--- a/Documentation/networking/xsk-tx-metadata.rst
+++ b/Documentation/networking/xsk-tx-metadata.rst
@@ -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
diff --git a/include/uapi/linux/if_xdp.h b/include/uapi/linux/if_xdp.h
index d31698410410..42ec5ddaab8d 100644
--- a/include/uapi/linux/if_xdp.h
+++ b/include/uapi/linux/if_xdp.h
@@ -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;
diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index caa340134b0e..9f76ca591d54 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -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->tx_metadata_len >= 256 || mr->tx_metadata_len % 8)
-		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);
-- 
2.45.2

[PATCH bpf 2/3] selftests/bpf: Add XDP_UMEM_TX_METADATA_LEN to XSK TX metadata test

[Stanislav Fomichev]

This flag is now required to use tx_metadata_len.

Fixes: 40808a237d9c ("selftests/bpf: Add TX side to xdp_metadata")
Reported-by: Julian Schindel <mail@arctic-alpaca.de>
Cc: Magnus Karlsson <magnus.karlsson@gmail.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 tools/include/uapi/linux/if_xdp.h                     | 4 ++++
 tools/testing/selftests/bpf/prog_tests/xdp_metadata.c | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/include/uapi/linux/if_xdp.h b/tools/include/uapi/linux/if_xdp.h
index 638c606dfa74..2f082b01ff22 100644
--- a/tools/include/uapi/linux/if_xdp.h
+++ b/tools/include/uapi/linux/if_xdp.h
@@ -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;
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
index f76b5d67a3ee..c87ee2bf558c 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
@@ -68,7 +68,8 @@ static int open_xsk(int ifindex, struct xsk *xsk)
 		.fill_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
 		.comp_size = XSK_RING_CONS__DEFAULT_NUM_DESCS,
 		.frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE,
-		.flags = XDP_UMEM_UNALIGNED_CHUNK_FLAG | XDP_UMEM_TX_SW_CSUM,
+		.flags = XDP_UMEM_UNALIGNED_CHUNK_FLAG | XDP_UMEM_TX_SW_CSUM |
+			 XDP_UMEM_TX_METADATA_LEN,
 		.tx_metadata_len = sizeof(struct xsk_tx_metadata),
 	};
 	__u32 idx;
-- 
2.45.2

[PATCH bpf 3/3] xsk: Try to make xdp_umem_reg extension a bit more future-proof

[Stanislav Fomichev]

Add a couple of things:
1. Remove xdp_umem_reg_v2 since its sizeof is the same as xdp_umem_reg
2. Add BUILD_BUG_ON that checks that the size of xdp_umem_reg_v1 is less
   than xdp_umem_reg; presumably, when we get to v2, there is gonna
   be a similar line to enforce that sizeof(v2) > sizeof(v1)
3. Add BUILD_BUG_ON to make sure the last field plus its size matches
   the overall struct size. The intent is to demonstrate that we don't
   have any lingering padding.

Reported-by: Julian Schindel <mail@arctic-alpaca.de>
Cc: Magnus Karlsson <magnus.karlsson@gmail.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 net/xdp/xsk.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 7d1c0986f9bb..1d951d7e3797 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -1331,14 +1331,6 @@ struct xdp_umem_reg_v1 {
 	__u32 headroom;
 };
 
-struct xdp_umem_reg_v2 {
-	__u64 addr; /* Start of packet data area */
-	__u64 len; /* Length of packet data area */
-	__u32 chunk_size;
-	__u32 headroom;
-	__u32 flags;
-};
-
 static int xsk_setsockopt(struct socket *sock, int level, int optname,
 			  sockptr_t optval, unsigned int optlen)
 {
@@ -1382,10 +1374,19 @@ static int xsk_setsockopt(struct socket *sock, int level, int optname,
 
 		if (optlen < sizeof(struct xdp_umem_reg_v1))
 			return -EINVAL;
-		else if (optlen < sizeof(struct xdp_umem_reg_v2))
-			mr_size = sizeof(struct xdp_umem_reg_v1);
 		else if (optlen < sizeof(mr))
-			mr_size = sizeof(struct xdp_umem_reg_v2);
+			mr_size = sizeof(struct xdp_umem_reg_v1);
+
+		BUILD_BUG_ON(sizeof(struct xdp_umem_reg_v1) >= sizeof(struct xdp_umem_reg));
+
+		/* Make sure the last field of the struct doesn't have
+		 * uninitialized padding. All padding has to be explicit
+		 * and has to be set to zero by the userspace to make
+		 * struct xdp_umem_reg extensible in the future.
+		 */
+		BUILD_BUG_ON(offsetof(struct xdp_umem_reg, tx_metadata_len) +
+			     sizeof_field(struct xdp_umem_reg, tx_metadata_len) !=
+			     sizeof(struct xdp_umem_reg));
 
 		if (copy_from_sockptr(&mr, optval, mr_size))
 			return -EFAULT;
-- 
2.45.2

Re: [PATCH bpf 3/3] xsk: Try to make xdp_umem_reg extension a bit more future-proof

[Maciej Fijalkowski]

On Fri, Jul 12, 2024 at 06:52:53PM -0700, Stanislav Fomichev wrote:
> Add a couple of things:
> 1. Remove xdp_umem_reg_v2 since its sizeof is the same as xdp_umem_reg

So thing here is that adding __attribute__((packed)) on kernel side
wouldn't help because we wouldn't fix old uapi with this, correct? old
uapi would still yield 32 bytes for xdp_umem_reg without tx_metadata_len.

Just explaining here to myself.

> 2. Add BUILD_BUG_ON that checks that the size of xdp_umem_reg_v1 is less
>    than xdp_umem_reg; presumably, when we get to v2, there is gonna
>    be a similar line to enforce that sizeof(v2) > sizeof(v1)
> 3. Add BUILD_BUG_ON to make sure the last field plus its size matches
>    the overall struct size. The intent is to demonstrate that we don't
>    have any lingering padding.

This is good stuff but I wonder wouldn't it be more feasible to squash
this with 1/3 ? And have it backported. Regarding the patch logistics, you
did not provide fixes tag here for some reason, but still include the
patch routed via bpf tree.

> 
> Reported-by: Julian Schindel <mail@arctic-alpaca.de>
> Cc: Magnus Karlsson <magnus.karlsson@gmail.com>
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> ---
>  net/xdp/xsk.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 7d1c0986f9bb..1d951d7e3797 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -1331,14 +1331,6 @@ struct xdp_umem_reg_v1 {
>  	__u32 headroom;
>  };
>  
> -struct xdp_umem_reg_v2 {
> -	__u64 addr; /* Start of packet data area */
> -	__u64 len; /* Length of packet data area */
> -	__u32 chunk_size;
> -	__u32 headroom;
> -	__u32 flags;
> -};
> -
>  static int xsk_setsockopt(struct socket *sock, int level, int optname,
>  			  sockptr_t optval, unsigned int optlen)
>  {
> @@ -1382,10 +1374,19 @@ static int xsk_setsockopt(struct socket *sock, int level, int optname,
>  
>  		if (optlen < sizeof(struct xdp_umem_reg_v1))
>  			return -EINVAL;
> -		else if (optlen < sizeof(struct xdp_umem_reg_v2))
> -			mr_size = sizeof(struct xdp_umem_reg_v1);
>  		else if (optlen < sizeof(mr))
> -			mr_size = sizeof(struct xdp_umem_reg_v2);
> +			mr_size = sizeof(struct xdp_umem_reg_v1);
> +
> +		BUILD_BUG_ON(sizeof(struct xdp_umem_reg_v1) >= sizeof(struct xdp_umem_reg));
> +
> +		/* Make sure the last field of the struct doesn't have
> +		 * uninitialized padding. All padding has to be explicit
> +		 * and has to be set to zero by the userspace to make
> +		 * struct xdp_umem_reg extensible in the future.
> +		 */
> +		BUILD_BUG_ON(offsetof(struct xdp_umem_reg, tx_metadata_len) +
> +			     sizeof_field(struct xdp_umem_reg, tx_metadata_len) !=
> +			     sizeof(struct xdp_umem_reg));
>  
>  		if (copy_from_sockptr(&mr, optval, mr_size))
>  			return -EFAULT;
> -- 
> 2.45.2
> 
> 

Re: [PATCH bpf 3/3] xsk: Try to make xdp_umem_reg extension a bit more future-proof

[Stanislav Fomichev]

On 07/24, Maciej Fijalkowski wrote:
> On Fri, Jul 12, 2024 at 06:52:53PM -0700, Stanislav Fomichev wrote:
> > Add a couple of things:
> > 1. Remove xdp_umem_reg_v2 since its sizeof is the same as xdp_umem_reg
> 
> So thing here is that adding __attribute__((packed)) on kernel side
> wouldn't help because we wouldn't fix old uapi with this, correct? old
> uapi would still yield 32 bytes for xdp_umem_reg without tx_metadata_len.
> 
> Just explaining here to myself.

Yea :-(

> > 2. Add BUILD_BUG_ON that checks that the size of xdp_umem_reg_v1 is less
> >    than xdp_umem_reg; presumably, when we get to v2, there is gonna
> >    be a similar line to enforce that sizeof(v2) > sizeof(v1)
> > 3. Add BUILD_BUG_ON to make sure the last field plus its size matches
> >    the overall struct size. The intent is to demonstrate that we don't
> >    have any lingering padding.
> 
> This is good stuff but I wonder wouldn't it be more feasible to squash
> this with 1/3 ? And have it backported. Regarding the patch logistics, you
> did not provide fixes tag here for some reason, but still include the
> patch routed via bpf tree.

SG, will resend this against bpf-next.

Re: [PATCH bpf 0/3] xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len

[Daniel Borkmann]

On 7/13/24 3:52 AM, Stanislav Fomichev wrote:
> Julian reports that commit 341ac980eab9 ("xsk: Support tx_metadata_len")
> can break existing use cases which don't zero-initialize xdp_umem_reg
> padding. Fix it (while still breaking a minority of new users of tx
> metadata), update the docs, update the selftest and sprinkle some
> BUILD_BUG_ONs to hopefully catch similar issues in the future.
> 
> Thank you Julian for the report and for helping to chase it down!
> 
> Reported-by: Julian Schindel <mail@arctic-alpaca.de>
> Cc: Magnus Karlsson <magnus.karlsson@gmail.com>
> 
> Stanislav Fomichev (3):
>    xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len
>    selftests/bpf: Add XDP_UMEM_TX_METADATA_LEN to XSK TX metadata test
>    xsk: Try to make xdp_umem_reg extension a bit more future-proof
> 
>   Documentation/networking/xsk-tx-metadata.rst  | 16 ++++++++-----
>   include/uapi/linux/if_xdp.h                   |  4 ++++
>   net/xdp/xdp_umem.c                            |  9 +++++---
>   net/xdp/xsk.c                                 | 23 ++++++++++---------
>   tools/include/uapi/linux/if_xdp.h             |  4 ++++
>   .../selftests/bpf/prog_tests/xdp_metadata.c   |  3 ++-
>   6 files changed, 38 insertions(+), 21 deletions(-)
> 

Magnus or Maciej, ptal when you get a chance.

Thanks,
Daniel

RE: [PATCH bpf 0/3] xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len

[Fijalkowski, Maciej]

> On 7/13/24 3:52 AM, Stanislav Fomichev wrote:
> > Julian reports that commit 341ac980eab9 ("xsk: Support tx_metadata_len")
> > can break existing use cases which don't zero-initialize xdp_umem_reg
> > padding. Fix it (while still breaking a minority of new users of tx
> > metadata), update the docs, update the selftest and sprinkle some
> > BUILD_BUG_ONs to hopefully catch similar issues in the future.
> >
> > Thank you Julian for the report and for helping to chase it down!
> >
> > Reported-by: Julian Schindel <mail@arctic-alpaca.de>
> > Cc: Magnus Karlsson <magnus.karlsson@gmail.com>
> >
> > Stanislav Fomichev (3):
> >    xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len
> >    selftests/bpf: Add XDP_UMEM_TX_METADATA_LEN to XSK TX metadata test
> >    xsk: Try to make xdp_umem_reg extension a bit more future-proof
> >
> >   Documentation/networking/xsk-tx-metadata.rst  | 16 ++++++++-----
> >   include/uapi/linux/if_xdp.h                   |  4 ++++
> >   net/xdp/xdp_umem.c                            |  9 +++++---
> >   net/xdp/xsk.c                                 | 23 ++++++++++---------
> >   tools/include/uapi/linux/if_xdp.h             |  4 ++++
> >   .../selftests/bpf/prog_tests/xdp_metadata.c   |  3 ++-
> >   6 files changed, 38 insertions(+), 21 deletions(-)
> >
> 
> Magnus or Maciej, ptal when you get a chance.

I'll do so on Monday as I'll be back from vacation, Magnus will be out for
yet another week. Hope it works for you?

> 
> Thanks,
> Daniel

Re: [PATCH bpf 0/3] xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len

[Daniel Borkmann]

On 7/19/24 5:29 PM, Fijalkowski, Maciej wrote:
>> On 7/13/24 3:52 AM, Stanislav Fomichev wrote:
>>> Julian reports that commit 341ac980eab9 ("xsk: Support tx_metadata_len")
>>> can break existing use cases which don't zero-initialize xdp_umem_reg
>>> padding. Fix it (while still breaking a minority of new users of tx
>>> metadata), update the docs, update the selftest and sprinkle some
>>> BUILD_BUG_ONs to hopefully catch similar issues in the future.
>>>
>>> Thank you Julian for the report and for helping to chase it down!
>>>
>>> Reported-by: Julian Schindel <mail@arctic-alpaca.de>
>>> Cc: Magnus Karlsson <magnus.karlsson@gmail.com>
>>>
>>> Stanislav Fomichev (3):
>>>     xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len
>>>     selftests/bpf: Add XDP_UMEM_TX_METADATA_LEN to XSK TX metadata test
>>>     xsk: Try to make xdp_umem_reg extension a bit more future-proof
>>>
>>>    Documentation/networking/xsk-tx-metadata.rst  | 16 ++++++++-----
>>>    include/uapi/linux/if_xdp.h                   |  4 ++++
>>>    net/xdp/xdp_umem.c                            |  9 +++++---
>>>    net/xdp/xsk.c                                 | 23 ++++++++++---------
>>>    tools/include/uapi/linux/if_xdp.h             |  4 ++++
>>>    .../selftests/bpf/prog_tests/xdp_metadata.c   |  3 ++-
>>>    6 files changed, 38 insertions(+), 21 deletions(-)
>>>
>>
>> Magnus or Maciej, ptal when you get a chance.
> 
> I'll do so on Monday as I'll be back from vacation, Magnus will be out for
> yet another week. Hope it works for you?

Sounds good, just making sure this doesn't fall off the radar. :)

Thanks Maciej!

Re: [PATCH bpf 0/3] xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len

[Maciej Fijalkowski]

On Fri, Jul 12, 2024 at 06:52:50PM -0700, Stanislav Fomichev wrote:
> Julian reports that commit 341ac980eab9 ("xsk: Support tx_metadata_len")
> can break existing use cases which don't zero-initialize xdp_umem_reg
> padding. Fix it (while still breaking a minority of new users of tx
> metadata), update the docs, update the selftest and sprinkle some
> BUILD_BUG_ONs to hopefully catch similar issues in the future.
> 
> Thank you Julian for the report and for helping to chase it down!
> 
> Reported-by: Julian Schindel <mail@arctic-alpaca.de>
> Cc: Magnus Karlsson <magnus.karlsson@gmail.com>

For the content series,

Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

However I was not sure about handling patch 3/3.

Thanks!

> 
> Stanislav Fomichev (3):
>   xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len
>   selftests/bpf: Add XDP_UMEM_TX_METADATA_LEN to XSK TX metadata test
>   xsk: Try to make xdp_umem_reg extension a bit more future-proof
> 
>  Documentation/networking/xsk-tx-metadata.rst  | 16 ++++++++-----
>  include/uapi/linux/if_xdp.h                   |  4 ++++
>  net/xdp/xdp_umem.c                            |  9 +++++---
>  net/xdp/xsk.c                                 | 23 ++++++++++---------
>  tools/include/uapi/linux/if_xdp.h             |  4 ++++
>  .../selftests/bpf/prog_tests/xdp_metadata.c   |  3 ++-
>  6 files changed, 38 insertions(+), 21 deletions(-)
> 
> -- 
> 2.45.2
> 
> 

Re: [PATCH bpf 0/3] xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len

[Daniel Borkmann]

On 7/24/24 5:23 PM, Maciej Fijalkowski wrote:
> On Fri, Jul 12, 2024 at 06:52:50PM -0700, Stanislav Fomichev wrote:
>> Julian reports that commit 341ac980eab9 ("xsk: Support tx_metadata_len")
>> can break existing use cases which don't zero-initialize xdp_umem_reg
>> padding. Fix it (while still breaking a minority of new users of tx
>> metadata), update the docs, update the selftest and sprinkle some
>> BUILD_BUG_ONs to hopefully catch similar issues in the future.
>>
>> Thank you Julian for the report and for helping to chase it down!
>>
>> Reported-by: Julian Schindel <mail@arctic-alpaca.de>
>> Cc: Magnus Karlsson <magnus.karlsson@gmail.com>
> 
> For the content series,
> 
> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> 
> However I was not sure about handling patch 3/3.

Ok, then I'm taking in the first two for now as they seem to actually
address the fix and the 3rd seems like an improvement which could also
get routed via bpf-next. In either case, if one of you could follow-up
on the latter, that would be great.

Thanks,
Daniel

Re: [PATCH bpf 0/3] xsk: require XDP_UMEM_TX_METADATA_LEN to actuate tx_metadata_len

[Maciej Fijalkowski]

On Thu, Jul 25, 2024 at 12:06:22PM +0200, Daniel Borkmann wrote:
> On 7/24/24 5:23 PM, Maciej Fijalkowski wrote:
> > On Fri, Jul 12, 2024 at 06:52:50PM -0700, Stanislav Fomichev wrote:
> > > Julian reports that commit 341ac980eab9 ("xsk: Support tx_metadata_len")
> > > can break existing use cases which don't zero-initialize xdp_umem_reg
> > > padding. Fix it (while still breaking a minority of new users of tx
> > > metadata), update the docs, update the selftest and sprinkle some
> > > BUILD_BUG_ONs to hopefully catch similar issues in the future.
> > > 
> > > Thank you Julian for the report and for helping to chase it down!
> > > 
> > > Reported-by: Julian Schindel <mail@arctic-alpaca.de>
> > > Cc: Magnus Karlsson <magnus.karlsson@gmail.com>
> > 
> > For the content series,
> > 
> > Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > 
> > However I was not sure about handling patch 3/3.
> 
> Ok, then I'm taking in the first two for now as they seem to actually
> address the fix and the 3rd seems like an improvement which could also
> get routed via bpf-next. In either case, if one of you could follow-up
> on the latter, that would be great.

My first thought was about squashing 1 and 3 but I hope Stan doesn't mind
sending 3 solely to bpf-next...

> 
> Thanks,
> Daniel