[PATCH bpf v2 0/9] Address XDP frags having negative tailroom

[PATCH bpf v2 0/9] Address XDP frags having negative tailroom

[Larysa Zaremba]

Aside from the issue described below, tailroom calculation does not account
for pages being split between frags, e.g. in i40e, enetc and
AF_XDP ZC with smaller chunks. These series address the problem by
calculating modulo (skb_frag_off() % rxq->frag_size) in order to get
data offset within a smaller block of memory. Please note, xskxceiver
tail grow test passes without modulo e.g. in xdpdrv mode on i40e,
because there is not enough descriptors to get to flipped buffers.

Many ethernet drivers report xdp Rx queue frag size as being the same as
DMA write size. However, the only user of this field, namely
bpf_xdp_frags_increase_tail(), clearly expects a truesize.

Such difference leads to unspecific memory corruption issues under certain
circumstances, e.g. in ixgbevf maximum DMA write size is 3 KB, so when
running xskxceiver's XDP_ADJUST_TAIL_GROW_MULTI_BUFF, 6K packet fully uses
all DMA-writable space in 2 buffers. This would be fine, if only
rxq->frag_size was properly set to 4K, but value of 3K results in a
negative tailroom, because there is a non-zero page offset.

We are supposed to return -EINVAL and be done with it in such case,
but due to tailroom being stored as an unsigned int, it is reported to be
somewhere near UINT_MAX, resulting in a tail being grown, even if the
requested offset is too much(it is around 2K in the abovementioned test).
This later leads to all kinds of unspecific calltraces.

[ 7340.337579] xskxceiver[1440]: segfault at 1da718 ip 00007f4161aeac9d sp 00007f41615a6a00 error 6
[ 7340.338040] xskxceiver[1441]: segfault at 7f410000000b ip 00000000004042b5 sp 00007f415bffecf0 error 4
[ 7340.338179]  in libc.so.6[61c9d,7f4161aaf000+160000]
[ 7340.339230]  in xskxceiver[42b5,400000+69000]
[ 7340.340300]  likely on CPU 6 (core 0, socket 6)
[ 7340.340302] Code: ff ff 01 e9 f4 fe ff ff 0f 1f 44 00 00 4c 39 f0 74 73 31 c0 ba 01 00 00 00 f0 0f b1 17 0f 85 ba 00 00 00 49 8b 87 88 00 00 00 <4c> 89 70 08 eb cc 0f 1f 44 00 00 48 8d bd f0 fe ff ff 89 85 ec fe
[ 7340.340888]  likely on CPU 3 (core 0, socket 3)
[ 7340.345088] Code: 00 00 00 ba 00 00 00 00 be 00 00 00 00 89 c7 e8 31 ca ff ff 89 45 ec 8b 45 ec 85 c0 78 07 b8 00 00 00 00 eb 46 e8 0b c8 ff ff <8b> 00 83 f8 69 74 24 e8 ff c7 ff ff 8b 00 83 f8 0b 74 18 e8 f3 c7
[ 7340.404334] Oops: general protection fault, probably for non-canonical address 0x6d255010bdffc: 0000 [#1] SMP NOPTI
[ 7340.405972] CPU: 7 UID: 0 PID: 1439 Comm: xskxceiver Not tainted 6.19.0-rc1+ #21 PREEMPT(lazy)
[ 7340.408006] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.17.0-5.fc42 04/01/2014
[ 7340.409716] RIP: 0010:lookup_swap_cgroup_id+0x44/0x80
[ 7340.410455] Code: 83 f8 1c 73 39 48 ba ff ff ff ff ff ff ff 03 48 8b 04 c5 20 55 fa bd 48 21 d1 48 89 ca 83 e1 01 48 d1 ea c1 e1 04 48 8d 04 90 <8b> 00 48 83 c4 10 d3 e8 c3 cc cc cc cc 31 c0 e9 98 b7 dd 00 48 89
[ 7340.412787] RSP: 0018:ffffcc5c04f7f6d0 EFLAGS: 00010202
[ 7340.413494] RAX: 0006d255010bdffc RBX: ffff891f477895a8 RCX: 0000000000000010
[ 7340.414431] RDX: 0001c17e3fffffff RSI: 00fa070000000000 RDI: 000382fc7fffffff
[ 7340.415354] RBP: 00fa070000000000 R08: ffffcc5c04f7f8f8 R09: ffffcc5c04f7f7d0
[ 7340.416283] R10: ffff891f4c1a7000 R11: ffffcc5c04f7f9c8 R12: ffffcc5c04f7f7d0
[ 7340.417218] R13: 03ffffffffffffff R14: 00fa06fffffffe00 R15: ffff891f47789500
[ 7340.418229] FS:  0000000000000000(0000) GS:ffff891ffdfaa000(0000) knlGS:0000000000000000
[ 7340.419489] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 7340.420286] CR2: 00007f415bfffd58 CR3: 0000000103f03002 CR4: 0000000000772ef0
[ 7340.421237] PKRU: 55555554
[ 7340.421623] Call Trace:
[ 7340.421987]  <TASK>
[ 7340.422309]  ? softleaf_from_pte+0x77/0xa0
[ 7340.422855]  swap_pte_batch+0xa7/0x290
[ 7340.423363]  zap_nonpresent_ptes.constprop.0.isra.0+0xd1/0x270
[ 7340.424102]  zap_pte_range+0x281/0x580
[ 7340.424607]  zap_pmd_range.isra.0+0xc9/0x240
[ 7340.425177]  unmap_page_range+0x24d/0x420
[ 7340.425714]  unmap_vmas+0xa1/0x180
[ 7340.426185]  exit_mmap+0xe1/0x3b0
[ 7340.426644]  __mmput+0x41/0x150
[ 7340.427098]  exit_mm+0xb1/0x110
[ 7340.427539]  do_exit+0x1b2/0x460
[ 7340.427992]  do_group_exit+0x2d/0xc0
[ 7340.428477]  get_signal+0x79d/0x7e0
[ 7340.428957]  arch_do_signal_or_restart+0x34/0x100
[ 7340.429571]  exit_to_user_mode_loop+0x8e/0x4c0
[ 7340.430159]  do_syscall_64+0x188/0x6b0
[ 7340.430672]  ? __do_sys_clone3+0xd9/0x120
[ 7340.431212]  ? switch_fpu_return+0x4e/0xd0
[ 7340.431761]  ? arch_exit_to_user_mode_prepare.isra.0+0xa1/0xc0
[ 7340.432498]  ? do_syscall_64+0xbb/0x6b0
[ 7340.433015]  ? __handle_mm_fault+0x445/0x690
[ 7340.433582]  ? count_memcg_events+0xd6/0x210
[ 7340.434151]  ? handle_mm_fault+0x212/0x340
[ 7340.434697]  ? do_user_addr_fault+0x2b4/0x7b0
[ 7340.435271]  ? clear_bhb_loop+0x30/0x80
[ 7340.435788]  ? clear_bhb_loop+0x30/0x80
[ 7340.436299]  ? clear_bhb_loop+0x30/0x80
[ 7340.436812]  ? clear_bhb_loop+0x30/0x80
[ 7340.437323]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 7340.437973] RIP: 0033:0x7f4161b14169
[ 7340.438468] Code: Unable to access opcode bytes at 0x7f4161b1413f.
[ 7340.439242] RSP: 002b:00007ffc6ebfa770 EFLAGS: 00000246 ORIG_RAX: 00000000000000ca
[ 7340.440173] RAX: fffffffffffffe00 RBX: 00000000000005a1 RCX: 00007f4161b14169
[ 7340.441061] RDX: 00000000000005a1 RSI: 0000000000000109 RDI: 00007f415bfff990
[ 7340.441943] RBP: 00007ffc6ebfa7a0 R08: 0000000000000000 R09: 00000000ffffffff
[ 7340.442824] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
[ 7340.443707] R13: 0000000000000000 R14: 00007f415bfff990 R15: 00007f415bfff6c0
[ 7340.444586]  </TASK>
[ 7340.444922] Modules linked in: rfkill intel_rapl_msr intel_rapl_common intel_uncore_frequency_common skx_edac_common nfit libnvdimm kvm_intel vfat fat kvm snd_pcm irqbypass rapl iTCO_wdt snd_timer intel_pmc_bxt iTCO_vendor_support snd ixgbevf virtio_net soundcore i2c_i801 pcspkr libeth_xdp net_failover i2c_smbus lpc_ich failover libeth virtio_balloon joydev 9p fuse loop zram lz4hc_compress lz4_compress 9pnet_virtio 9pnet netfs ghash_clmulni_intel serio_raw qemu_fw_cfg
[ 7340.449650] ---[ end trace 0000000000000000 ]---

The issue can be fixed in all in-tree drivers, but we cannot just trust OOT
drivers to not do this. Therefore, make tailroom a signed int and produce a
warning when it is negative to prevent such mistakes in the future.

The issue can also be easily reproduced with ice driver, by applying
the following diff to xskxceiver and enjoying a kernel panic in xdpdrv mode:

diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
index 5af28f359cfd..042d587fa7ef 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
@@ -2541,8 +2541,8 @@ int testapp_adjust_tail_grow_mb(struct test_spec *test)
 {
        test->mtu = MAX_ETH_JUMBO_SIZE;
        /* Grow by (frag_size - last_frag_Size) - 1 to stay inside the last fragment */
-       return testapp_adjust_tail(test, (XSK_UMEM__MAX_FRAME_SIZE / 2) - 1,
-                                  XSK_UMEM__LARGE_FRAME_SIZE * 2);
+       return testapp_adjust_tail(test, XSK_UMEM__MAX_FRAME_SIZE * 100,
+                                  6912);
 }

 int testapp_tx_queue_consumer(struct test_spec *test)

If we print out the values involved in the tailroom calculation:

tailroom = rxq->frag_size - skb_frag_size(frag) - skb_frag_off(frag);

4294967040 = 3456 - 3456 - 256

I personally reproduced and verified the issue in ice and i40e,
aside from WiP ixgbevf implementation.

v1->v2:
* add modulo to calculate offset within chunk
* add helper for AF_XDP ZC queues
* fix the problem in ZC mode in i40e, ice and idpf
* verify solution in i40e
* fix RxQ info registering in i40e
* fix splitq handling in idpf
* do not use word truesize unless the value used is named trusize

Larysa Zaremba (9):
  xdp: use modulo operation to calculate XDP frag tailroom
  xsk: introduce helper to determine rxq->frag_size
  ice: fix rxq info registering in mbuf packets
  ice: change XDP RxQ frag_size from DMA write length to xdp.frame_sz
  i40e: fix registering XDP RxQ info
  i40e: use xdp.frame_sz as XDP RxQ info frag_size
  idpf: use truesize as XDP RxQ info frag_size
  net: enetc: use truesize as XDP RxQ info frag_size
  xdp: produce a warning when calculated tailroom is negative

 drivers/net/ethernet/freescale/enetc/enetc.c |  2 +-
 drivers/net/ethernet/intel/i40e/i40e_main.c  | 41 ++++++++++++--------
 drivers/net/ethernet/intel/i40e/i40e_txrx.c  |  5 ++-
 drivers/net/ethernet/intel/ice/ice_base.c    | 30 +++++---------
 drivers/net/ethernet/intel/ice/ice_xsk.c     |  3 ++
 drivers/net/ethernet/intel/idpf/xdp.c        |  8 +++-
 drivers/net/ethernet/intel/idpf/xsk.c        |  1 +
 include/net/xdp_sock_drv.h                   |  5 +++
 net/core/filter.c                            |  6 ++-
 9 files changed, 58 insertions(+), 43 deletions(-)

-- 
2.52.0

[PATCH bpf v2 1/9] xdp: use modulo operation to calculate XDP frag tailroom

[Larysa Zaremba]

The current formula for calculating XDP tailroom in mbuf packets works only
if each frag has its own page (if rxq->frag_size is PAGE_SIZE), this
defeats the purpose of the parameter overall and without any indication
leads to negative calculated tailroom on at least half of frags, if shared
pages are used.

There are not many drivers that set rxq->frag_size. Among them:
* i40e and enetc always split page uniformly between frags, use shared
  pages
* ice uses page_pool frags via libeth, those are power-of-2 and uniformly
  distributed across page
* idpf has variable frag_size with XDP on, so current API is not applicable
* mlx5, mtk and mvneta use PAGE_SIZE or 0 as frag_size for page_pool

As for AF_XDP ZC, only ice, i40e and idpf declare frag_size for it. Modulo
operation yields good results for aligned chunks, they are all power-of-2,
between 2K and PAGE_SIZE. Formula without modulo fails when chunk_size is
2K. Buffers in unaligned mode are not distributed uniformly, so modulo
operation would not work.

To accommodate unaligned buffers, we could define frag_size as
data + tailroom, and hence do not subtract offset when calculating
tailroom, but this would necessitate more changes in the drivers.

Define rxq->frag_size as an even portion of a page that fully belongs to a
single frag. When calculating tailroom, locate the data start within such
portion by performing a modulo operation on page offset.

Fixes: bf25146a5595 ("bpf: add frags support to the bpf_xdp_adjust_tail() API")
Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
---
 net/core/filter.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index ba019ded773d..5f5489665c58 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4156,7 +4156,8 @@ static int bpf_xdp_frags_increase_tail(struct xdp_buff *xdp, int offset)
 	if (!rxq->frag_size || rxq->frag_size > xdp->frame_sz)
 		return -EOPNOTSUPP;
 
-	tailroom = rxq->frag_size - skb_frag_size(frag) - skb_frag_off(frag);
+	tailroom = rxq->frag_size - skb_frag_size(frag) -
+		   skb_frag_off(frag) % rxq->frag_size;
 	if (unlikely(offset > tailroom))
 		return -EINVAL;
 
-- 
2.52.0

[PATCH bpf v2 2/9] xsk: introduce helper to determine rxq->frag_size

[Larysa Zaremba]

rxq->frag_size is basically a step between consecutive strictly aligned
frames. In ZC mode, chunk size fits exactly, but if chunks are unaligned,
there is no safe way to determine accessible space to grow tailroom.

Report frag_size to be zero, if chunks are unaligned, chunk_size otherwise.

Fixes: 24ea50127ecf ("xsk: support mbuf on ZC RX")
Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
---
 include/net/xdp_sock_drv.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/net/xdp_sock_drv.h b/include/net/xdp_sock_drv.h
index 242e34f771cc..1f6dbb13dec8 100644
--- a/include/net/xdp_sock_drv.h
+++ b/include/net/xdp_sock_drv.h
@@ -51,6 +51,11 @@ static inline u32 xsk_pool_get_rx_frame_size(struct xsk_buff_pool *pool)
 	return xsk_pool_get_chunk_size(pool) - xsk_pool_get_headroom(pool);
 }
 
+static inline u32 xsk_pool_get_rx_frag_step(struct xsk_buff_pool *pool)
+{
+	return pool->unaligned ? 0 : xsk_pool_get_chunk_size(pool);
+}
+
 static inline void xsk_pool_set_rxq_info(struct xsk_buff_pool *pool,
 					 struct xdp_rxq_info *rxq)
 {
-- 
2.52.0

[PATCH bpf v2 3/9] ice: fix rxq info registering in mbuf packets

[Larysa Zaremba]

XDP RxQ info contains frag_size, which depends on the MTU. This makes the
old way of registering RxQ info before calculating new buffer sizes
invalid. Currently, it leads to frag_size being outdated, making it
sometimes impossible to grow tailroom in a mbuf packet. E.g. fragments are
actually 3K+, but frag size is still as if MTU was 1500.

Always register new XDP RxQ info after reconfiguring memory pools.

Fixes: 2fba7dc5157b ("ice: Add support for XDP multi-buffer on Rx side")
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_base.c | 26 ++++++-----------------
 drivers/net/ethernet/intel/ice/ice_xsk.c  |  3 +++
 2 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_base.c b/drivers/net/ethernet/intel/ice/ice_base.c
index eadb1e3d12b3..511d803cf0a4 100644
--- a/drivers/net/ethernet/intel/ice/ice_base.c
+++ b/drivers/net/ethernet/intel/ice/ice_base.c
@@ -663,23 +663,12 @@ static int ice_vsi_cfg_rxq(struct ice_rx_ring *ring)
 	int err;
 
 	if (ring->vsi->type == ICE_VSI_PF || ring->vsi->type == ICE_VSI_SF) {
-		if (!xdp_rxq_info_is_reg(&ring->xdp_rxq)) {
-			err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
-						 ring->q_index,
-						 ring->q_vector->napi.napi_id,
-						 ring->rx_buf_len);
-			if (err)
-				return err;
-		}
-
 		ice_rx_xsk_pool(ring);
 		err = ice_realloc_rx_xdp_bufs(ring, ring->xsk_pool);
 		if (err)
 			return err;
 
 		if (ring->xsk_pool) {
-			xdp_rxq_info_unreg(&ring->xdp_rxq);
-
 			rx_buf_len =
 				xsk_pool_get_rx_frame_size(ring->xsk_pool);
 			err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
@@ -702,14 +691,13 @@ static int ice_vsi_cfg_rxq(struct ice_rx_ring *ring)
 			if (err)
 				return err;
 
-			if (!xdp_rxq_info_is_reg(&ring->xdp_rxq)) {
-				err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
-							 ring->q_index,
-							 ring->q_vector->napi.napi_id,
-							 ring->rx_buf_len);
-				if (err)
-					goto err_destroy_fq;
-			}
+			err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
+						 ring->q_index,
+						 ring->q_vector->napi.napi_id,
+						 ring->rx_buf_len);
+			if (err)
+				goto err_destroy_fq;
+
 			xdp_rxq_info_attach_page_pool(&ring->xdp_rxq,
 						      ring->pp);
 		}
diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
index 989ff1fd9110..102631398af3 100644
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -900,6 +900,9 @@ void ice_xsk_clean_rx_ring(struct ice_rx_ring *rx_ring)
 	u16 ntc = rx_ring->next_to_clean;
 	u16 ntu = rx_ring->next_to_use;
 
+	if (xdp_rxq_info_is_reg(&rx_ring->xdp_rxq))
+		xdp_rxq_info_unreg(&rx_ring->xdp_rxq);
+
 	while (ntc != ntu) {
 		struct xdp_buff *xdp = *ice_xdp_buf(rx_ring, ntc);
 
-- 
2.52.0

[PATCH bpf v2 4/9] ice: change XDP RxQ frag_size from DMA write length to xdp.frame_sz

[Larysa Zaremba]

The only user of frag_size field in XDP RxQ info is
bpf_xdp_frags_increase_tail(). It clearly expects whole buff size instead
of DMA write size. Different assumptions in ice driver configuration lead
to negative tailroom.

This allows to trigger kernel panic, when using
XDP_ADJUST_TAIL_GROW_MULTI_BUFF xskxceiver test and changing packet size to
6912 and the requisted offset to a huge value, e.g.
XSK_UMEM__MAX_FRAME_SIZE * 100.

Due to other quirks of the ZC configuration in ice, panic is not observed
in ZC mode, but tailroom growing still fails when it should not.

Use fill queue buffer truesize instead of DMA write size in XDP RxQ info.
Fix ZC mode too by using the new helper.

Fixes: 2fba7dc5157b ("ice: Add support for XDP multi-buffer on Rx side")
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_base.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_base.c b/drivers/net/ethernet/intel/ice/ice_base.c
index 511d803cf0a4..37ac5cbc7bcd 100644
--- a/drivers/net/ethernet/intel/ice/ice_base.c
+++ b/drivers/net/ethernet/intel/ice/ice_base.c
@@ -669,12 +669,14 @@ static int ice_vsi_cfg_rxq(struct ice_rx_ring *ring)
 			return err;
 
 		if (ring->xsk_pool) {
+			u32 frag_size =
+				xsk_pool_get_rx_frag_step(ring->xsk_pool);
 			rx_buf_len =
 				xsk_pool_get_rx_frame_size(ring->xsk_pool);
 			err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
 						 ring->q_index,
 						 ring->q_vector->napi.napi_id,
-						 rx_buf_len);
+						 frag_size);
 			if (err)
 				return err;
 			err = xdp_rxq_info_reg_mem_model(&ring->xdp_rxq,
@@ -694,7 +696,7 @@ static int ice_vsi_cfg_rxq(struct ice_rx_ring *ring)
 			err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
 						 ring->q_index,
 						 ring->q_vector->napi.napi_id,
-						 ring->rx_buf_len);
+						 ring->truesize);
 			if (err)
 				goto err_destroy_fq;
 
-- 
2.52.0

[PATCH bpf v2 5/9] i40e: fix registering XDP RxQ info

[Larysa Zaremba]

Current way of handling XDP RxQ info in i40e has following problems:
* when xsk_buff_pool is detached, memory model is not unregistered before
  registering a new one, this leads to a dangling xsk_buff_pool in the
  memory model table
* frag_size is not updated when xsk_buff_pool is detached or when MTU is
  changed, this leads to growing tail always failing for multi-buffer
  packets.

Couple XDP RxQ info registering with buffer allocations and unregistering
with cleaning the ring.

Fixes: a045d2f2d03d ("i40e: set xdp_rxq_info::frag_size")
Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 34 ++++++++++++---------
 drivers/net/ethernet/intel/i40e/i40e_txrx.c |  5 +--
 2 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index d3bc3207054f..eaa5b65e6daf 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -3577,18 +3577,8 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
 	if (ring->vsi->type != I40E_VSI_MAIN)
 		goto skip;
 
-	if (!xdp_rxq_info_is_reg(&ring->xdp_rxq)) {
-		err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
-					 ring->queue_index,
-					 ring->q_vector->napi.napi_id,
-					 ring->rx_buf_len);
-		if (err)
-			return err;
-	}
-
 	ring->xsk_pool = i40e_xsk_pool(ring);
 	if (ring->xsk_pool) {
-		xdp_rxq_info_unreg(&ring->xdp_rxq);
 		ring->rx_buf_len = xsk_pool_get_rx_frame_size(ring->xsk_pool);
 		err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
 					 ring->queue_index,
@@ -3600,17 +3590,23 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
 						 MEM_TYPE_XSK_BUFF_POOL,
 						 NULL);
 		if (err)
-			return err;
+			goto unreg_xdp;
 		dev_info(&vsi->back->pdev->dev,
 			 "Registered XDP mem model MEM_TYPE_XSK_BUFF_POOL on Rx ring %d\n",
 			 ring->queue_index);
 
 	} else {
+		err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
+					 ring->queue_index,
+					 ring->q_vector->napi.napi_id,
+					 ring->rx_buf_len);
+		if (err)
+			return err;
 		err = xdp_rxq_info_reg_mem_model(&ring->xdp_rxq,
 						 MEM_TYPE_PAGE_SHARED,
 						 NULL);
 		if (err)
-			return err;
+			goto unreg_xdp;
 	}
 
 skip:
@@ -3648,7 +3644,8 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
 		dev_info(&vsi->back->pdev->dev,
 			 "Failed to clear LAN Rx queue context on Rx ring %d (pf_q %d), error: %d\n",
 			 ring->queue_index, pf_q, err);
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto unreg_xdp;
 	}
 
 	/* set the context in the HMC */
@@ -3657,7 +3654,8 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
 		dev_info(&vsi->back->pdev->dev,
 			 "Failed to set LAN Rx queue context on Rx ring %d (pf_q %d), error: %d\n",
 			 ring->queue_index, pf_q, err);
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto unreg_xdp;
 	}
 
 	/* configure Rx buffer alignment */
@@ -3665,7 +3663,8 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
 		if (I40E_2K_TOO_SMALL_WITH_PADDING) {
 			dev_info(&vsi->back->pdev->dev,
 				 "2k Rx buffer is too small to fit standard MTU and skb_shared_info\n");
-			return -EOPNOTSUPP;
+			err = -EOPNOTSUPP;
+			goto unreg_xdp;
 		}
 		clear_ring_build_skb_enabled(ring);
 	} else {
@@ -3695,6 +3694,11 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
 	}
 
 	return 0;
+unreg_xdp:
+	if (ring->vsi->type == I40E_VSI_MAIN)
+		xdp_rxq_info_unreg(&ring->xdp_rxq);
+
+	return err;
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index cc0b9efc2637..816179c7e271 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1470,6 +1470,9 @@ void i40e_clean_rx_ring(struct i40e_ring *rx_ring)
 	if (!rx_ring->rx_bi)
 		return;
 
+	if (xdp_rxq_info_is_reg(&rx_ring->xdp_rxq))
+		xdp_rxq_info_unreg(&rx_ring->xdp_rxq);
+
 	if (rx_ring->xsk_pool) {
 		i40e_xsk_clean_rx_ring(rx_ring);
 		goto skip_free;
@@ -1527,8 +1530,6 @@ void i40e_clean_rx_ring(struct i40e_ring *rx_ring)
 void i40e_free_rx_resources(struct i40e_ring *rx_ring)
 {
 	i40e_clean_rx_ring(rx_ring);
-	if (rx_ring->vsi->type == I40E_VSI_MAIN)
-		xdp_rxq_info_unreg(&rx_ring->xdp_rxq);
 	rx_ring->xdp_prog = NULL;
 	kfree(rx_ring->rx_bi);
 	rx_ring->rx_bi = NULL;
-- 
2.52.0

[PATCH bpf v2 6/9] i40e: use xdp.frame_sz as XDP RxQ info frag_size

[Larysa Zaremba]

The only user of frag_size field in XDP RxQ info is
bpf_xdp_frags_increase_tail(). It clearly expects whole buffer size instead
of DMA write size. Different assumptions in i40e driver configuration lead
to negative tailroom.

Set frag_size to the same value as frame_sz in shared pages mode, use new
helper to set frag_size when AF_XDP ZC is active.

Fixes: a045d2f2d03d ("i40e: set xdp_rxq_info::frag_size")
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index eaa5b65e6daf..f91157774f1d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -3563,6 +3563,7 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
 	u16 pf_q = vsi->base_queue + ring->queue_index;
 	struct i40e_hw *hw = &vsi->back->hw;
 	struct i40e_hmc_obj_rxq rx_ctx;
+	u32 xdp_frame_sz = 0;
 	int err = 0;
 	bool ok;
 
@@ -3579,11 +3580,12 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
 
 	ring->xsk_pool = i40e_xsk_pool(ring);
 	if (ring->xsk_pool) {
+		xdp_frame_sz = xsk_pool_get_rx_frag_step(ring->xsk_pool);
 		ring->rx_buf_len = xsk_pool_get_rx_frame_size(ring->xsk_pool);
 		err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
 					 ring->queue_index,
 					 ring->q_vector->napi.napi_id,
-					 ring->rx_buf_len);
+					 xdp_frame_sz);
 		if (err)
 			return err;
 		err = xdp_rxq_info_reg_mem_model(&ring->xdp_rxq,
@@ -3596,10 +3598,11 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
 			 ring->queue_index);
 
 	} else {
+		xdp_frame_sz = i40e_rx_pg_size(ring) / 2;
 		err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
 					 ring->queue_index,
 					 ring->q_vector->napi.napi_id,
-					 ring->rx_buf_len);
+					 xdp_frame_sz);
 		if (err)
 			return err;
 		err = xdp_rxq_info_reg_mem_model(&ring->xdp_rxq,
@@ -3610,7 +3613,7 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
 	}
 
 skip:
-	xdp_init_buff(&ring->xdp, i40e_rx_pg_size(ring) / 2, &ring->xdp_rxq);
+	xdp_init_buff(&ring->xdp, xdp_frame_sz, &ring->xdp_rxq);
 
 	rx_ctx.dbuff = DIV_ROUND_UP(ring->rx_buf_len,
 				    BIT_ULL(I40E_RXQ_CTX_DBUFF_SHIFT));
-- 
2.52.0

[PATCH bpf v2 7/9] idpf: use truesize as XDP RxQ info frag_size

[Larysa Zaremba]

The only user of frag_size field in XDP RxQ info is
bpf_xdp_frags_increase_tail(). It clearly expects whole buffer size instead
of DMA write size. Different assumptions in idpf driver configuration lead
to negative tailroom.

To make it worse, buffer sizes are not actually uniform in idpf when
splitq is enabled, as there are several buffer queues, so rxq->rx_buf_size
is meaningless in this case.

Use rxq->truesize as a frag_size for singleq and truesize of the first bufq
in AF_XDP ZC, as there is only one. Disable growinf tail for regular
splitq.

Fixes: ac8a861f632e ("idpf: prepare structures to support XDP")
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
---
 drivers/net/ethernet/intel/idpf/xdp.c | 8 +++++++-
 drivers/net/ethernet/intel/idpf/xsk.c | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/idpf/xdp.c b/drivers/net/ethernet/intel/idpf/xdp.c
index 958d16f87424..a152c9a26976 100644
--- a/drivers/net/ethernet/intel/idpf/xdp.c
+++ b/drivers/net/ethernet/intel/idpf/xdp.c
@@ -46,11 +46,17 @@ static int __idpf_xdp_rxq_info_init(struct idpf_rx_queue *rxq, void *arg)
 {
 	const struct idpf_vport *vport = rxq->q_vector->vport;
 	bool split = idpf_is_queue_model_split(vport->rxq_model);
+	u32 frag_size = 0;
 	int err;
 
+	if (idpf_queue_has(XSK, rxq) && split)
+		frag_size = rxq->bufq_sets[0].bufq.truesize;
+	else if (!split)
+		frag_size = rxq->truesize;
+
 	err = __xdp_rxq_info_reg(&rxq->xdp_rxq, vport->netdev, rxq->idx,
 				 rxq->q_vector->napi.napi_id,
-				 rxq->rx_buf_size);
+				 frag_size);
 	if (err)
 		return err;
 
diff --git a/drivers/net/ethernet/intel/idpf/xsk.c b/drivers/net/ethernet/intel/idpf/xsk.c
index fd2cc43ab43c..febe1073b9b4 100644
--- a/drivers/net/ethernet/intel/idpf/xsk.c
+++ b/drivers/net/ethernet/intel/idpf/xsk.c
@@ -401,6 +401,7 @@ int idpf_xskfq_init(struct idpf_buf_queue *bufq)
 	bufq->pending = fq.pending;
 	bufq->thresh = fq.thresh;
 	bufq->rx_buf_size = fq.buf_len;
+	bufq->truesize = xsk_pool_get_rx_frag_step(fq.pool);
 
 	if (!idpf_xskfq_refill(bufq))
 		netdev_err(bufq->pool->netdev,
-- 
2.52.0

[PATCH bpf v2 8/9] net: enetc: use truesize as XDP RxQ info frag_size

[Larysa Zaremba]

The only user of frag_size field in XDP RxQ info is
bpf_xdp_frags_increase_tail(). It clearly expects truesize instead of DMA
write size. Different assumptions in enetc driver configuration lead to
negative tailroom.

Set frag_size to the same value as frame_sz.

Fixes: 2768b2e2f7d2 ("net: enetc: register XDP RX queues with frag_size")
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index e380a4f39855..9fdd448e602f 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -3468,7 +3468,7 @@ static int enetc_int_vector_init(struct enetc_ndev_priv *priv, int i,
 	priv->rx_ring[i] = bdr;
 
 	err = __xdp_rxq_info_reg(&bdr->xdp.rxq, priv->ndev, i, 0,
-				 ENETC_RXB_DMA_SIZE_XDP);
+				 ENETC_RXB_TRUESIZE);
 	if (err)
 		goto free_vector;
 
-- 
2.52.0

[PATCH bpf v2 9/9] xdp: produce a warning when calculated tailroom is negative

[Larysa Zaremba]

Many ethernet drivers report xdp Rx queue frag size as being the same as
DMA write size. However, the only user of this field, namely
bpf_xdp_frags_increase_tail(), clearly expects a truesize.

Such difference leads to unspecific memory corruption issues under certain
circumstances, e.g. in ixgbevf maximum DMA write size is 3 KB, so when
running xskxceiver's XDP_ADJUST_TAIL_GROW_MULTI_BUFF, 6K packet fully uses
all DMA-writable space in 2 buffers. This would be fine, if only
rxq->frag_size was properly set to 4K, but value of 3K results in a
negative tailroom, because there is a non-zero page offset.

We are supposed to return -EINVAL and be done with it in such case, but due
to tailroom being stored as an unsigned int, it is reported to be somewhere
near UINT_MAX, resulting in a tail being grown, even if the requested
offset is too much (it is around 2K in the abovementioned test). This later
leads to all kinds of unspecific calltraces.

[ 7340.337579] xskxceiver[1440]: segfault at 1da718 ip 00007f4161aeac9d sp 00007f41615a6a00 error 6
[ 7340.338040] xskxceiver[1441]: segfault at 7f410000000b ip 00000000004042b5 sp 00007f415bffecf0 error 4
[ 7340.338179]  in libc.so.6[61c9d,7f4161aaf000+160000]
[ 7340.339230]  in xskxceiver[42b5,400000+69000]
[ 7340.340300]  likely on CPU 6 (core 0, socket 6)
[ 7340.340302] Code: ff ff 01 e9 f4 fe ff ff 0f 1f 44 00 00 4c 39 f0 74 73 31 c0 ba 01 00 00 00 f0 0f b1 17 0f 85 ba 00 00 00 49 8b 87 88 00 00 00 <4c> 89 70 08 eb cc 0f 1f 44 00 00 48 8d bd f0 fe ff ff 89 85 ec fe
[ 7340.340888]  likely on CPU 3 (core 0, socket 3)
[ 7340.345088] Code: 00 00 00 ba 00 00 00 00 be 00 00 00 00 89 c7 e8 31 ca ff ff 89 45 ec 8b 45 ec 85 c0 78 07 b8 00 00 00 00 eb 46 e8 0b c8 ff ff <8b> 00 83 f8 69 74 24 e8 ff c7 ff ff 8b 00 83 f8 0b 74 18 e8 f3 c7
[ 7340.404334] Oops: general protection fault, probably for non-canonical address 0x6d255010bdffc: 0000 [#1] SMP NOPTI
[ 7340.405972] CPU: 7 UID: 0 PID: 1439 Comm: xskxceiver Not tainted 6.19.0-rc1+ #21 PREEMPT(lazy)
[ 7340.408006] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.17.0-5.fc42 04/01/2014
[ 7340.409716] RIP: 0010:lookup_swap_cgroup_id+0x44/0x80
[ 7340.410455] Code: 83 f8 1c 73 39 48 ba ff ff ff ff ff ff ff 03 48 8b 04 c5 20 55 fa bd 48 21 d1 48 89 ca 83 e1 01 48 d1 ea c1 e1 04 48 8d 04 90 <8b> 00 48 83 c4 10 d3 e8 c3 cc cc cc cc 31 c0 e9 98 b7 dd 00 48 89
[ 7340.412787] RSP: 0018:ffffcc5c04f7f6d0 EFLAGS: 00010202
[ 7340.413494] RAX: 0006d255010bdffc RBX: ffff891f477895a8 RCX: 0000000000000010
[ 7340.414431] RDX: 0001c17e3fffffff RSI: 00fa070000000000 RDI: 000382fc7fffffff
[ 7340.415354] RBP: 00fa070000000000 R08: ffffcc5c04f7f8f8 R09: ffffcc5c04f7f7d0
[ 7340.416283] R10: ffff891f4c1a7000 R11: ffffcc5c04f7f9c8 R12: ffffcc5c04f7f7d0
[ 7340.417218] R13: 03ffffffffffffff R14: 00fa06fffffffe00 R15: ffff891f47789500
[ 7340.418229] FS:  0000000000000000(0000) GS:ffff891ffdfaa000(0000) knlGS:0000000000000000
[ 7340.419489] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 7340.420286] CR2: 00007f415bfffd58 CR3: 0000000103f03002 CR4: 0000000000772ef0
[ 7340.421237] PKRU: 55555554
[ 7340.421623] Call Trace:
[ 7340.421987]  <TASK>
[ 7340.422309]  ? softleaf_from_pte+0x77/0xa0
[ 7340.422855]  swap_pte_batch+0xa7/0x290
[ 7340.423363]  zap_nonpresent_ptes.constprop.0.isra.0+0xd1/0x270
[ 7340.424102]  zap_pte_range+0x281/0x580
[ 7340.424607]  zap_pmd_range.isra.0+0xc9/0x240
[ 7340.425177]  unmap_page_range+0x24d/0x420
[ 7340.425714]  unmap_vmas+0xa1/0x180
[ 7340.426185]  exit_mmap+0xe1/0x3b0
[ 7340.426644]  __mmput+0x41/0x150
[ 7340.427098]  exit_mm+0xb1/0x110
[ 7340.427539]  do_exit+0x1b2/0x460
[ 7340.427992]  do_group_exit+0x2d/0xc0
[ 7340.428477]  get_signal+0x79d/0x7e0
[ 7340.428957]  arch_do_signal_or_restart+0x34/0x100
[ 7340.429571]  exit_to_user_mode_loop+0x8e/0x4c0
[ 7340.430159]  do_syscall_64+0x188/0x6b0
[ 7340.430672]  ? __do_sys_clone3+0xd9/0x120
[ 7340.431212]  ? switch_fpu_return+0x4e/0xd0
[ 7340.431761]  ? arch_exit_to_user_mode_prepare.isra.0+0xa1/0xc0
[ 7340.432498]  ? do_syscall_64+0xbb/0x6b0
[ 7340.433015]  ? __handle_mm_fault+0x445/0x690
[ 7340.433582]  ? count_memcg_events+0xd6/0x210
[ 7340.434151]  ? handle_mm_fault+0x212/0x340
[ 7340.434697]  ? do_user_addr_fault+0x2b4/0x7b0
[ 7340.435271]  ? clear_bhb_loop+0x30/0x80
[ 7340.435788]  ? clear_bhb_loop+0x30/0x80
[ 7340.436299]  ? clear_bhb_loop+0x30/0x80
[ 7340.436812]  ? clear_bhb_loop+0x30/0x80
[ 7340.437323]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 7340.437973] RIP: 0033:0x7f4161b14169
[ 7340.438468] Code: Unable to access opcode bytes at 0x7f4161b1413f.
[ 7340.439242] RSP: 002b:00007ffc6ebfa770 EFLAGS: 00000246 ORIG_RAX: 00000000000000ca
[ 7340.440173] RAX: fffffffffffffe00 RBX: 00000000000005a1 RCX: 00007f4161b14169
[ 7340.441061] RDX: 00000000000005a1 RSI: 0000000000000109 RDI: 00007f415bfff990
[ 7340.441943] RBP: 00007ffc6ebfa7a0 R08: 0000000000000000 R09: 00000000ffffffff
[ 7340.442824] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
[ 7340.443707] R13: 0000000000000000 R14: 00007f415bfff990 R15: 00007f415bfff6c0
[ 7340.444586]  </TASK>
[ 7340.444922] Modules linked in: rfkill intel_rapl_msr intel_rapl_common intel_uncore_frequency_common skx_edac_common nfit libnvdimm kvm_intel vfat fat kvm snd_pcm irqbypass rapl iTCO_wdt snd_timer intel_pmc_bxt iTCO_vendor_support snd ixgbevf virtio_net soundcore i2c_i801 pcspkr libeth_xdp net_failover i2c_smbus lpc_ich failover libeth virtio_balloon joydev 9p fuse loop zram lz4hc_compress lz4_compress 9pnet_virtio 9pnet netfs ghash_clmulni_intel serio_raw qemu_fw_cfg
[ 7340.449650] ---[ end trace 0000000000000000 ]---

The issue can be fixed in all in-tree drivers, but we cannot just trust OOT
drivers to not do this. Therefore, make tailroom a signed int and produce a
warning when it is negative to prevent such mistakes in the future.

Fixes: bf25146a5595 ("bpf: add frags support to the bpf_xdp_adjust_tail() API")
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: Martin KaFai Lau <martin.lau@kernel.org>
Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
---
 net/core/filter.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 5f5489665c58..e93d9dc0471a 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4151,13 +4151,14 @@ static int bpf_xdp_frags_increase_tail(struct xdp_buff *xdp, int offset)
 	struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp);
 	skb_frag_t *frag = &sinfo->frags[sinfo->nr_frags - 1];
 	struct xdp_rxq_info *rxq = xdp->rxq;
-	unsigned int tailroom;
+	int tailroom;
 
 	if (!rxq->frag_size || rxq->frag_size > xdp->frame_sz)
 		return -EOPNOTSUPP;
 
 	tailroom = rxq->frag_size - skb_frag_size(frag) -
 		   skb_frag_off(frag) % rxq->frag_size;
+	WARN_ON_ONCE(tailroom < 0);
 	if (unlikely(offset > tailroom))
 		return -EINVAL;
 
-- 
2.52.0

Re: [PATCH bpf v2 3/9] ice: fix rxq info registering in mbuf packets

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2937 bytes --]

> diff --git a/drivers/net/ethernet/intel/ice/ice_base.c b/drivers/net/ethernet/intel/ice/ice_base.c
> index eadb1e3d12b3..511d803cf0a4 100644
> --- a/drivers/net/ethernet/intel/ice/ice_base.c
> +++ b/drivers/net/ethernet/intel/ice/ice_base.c
> @@ -663,23 +663,12 @@ static int ice_vsi_cfg_rxq(struct ice_rx_ring *ring)
>  	int err;
>
>  	if (ring->vsi->type == ICE_VSI_PF || ring->vsi->type == ICE_VSI_SF) {

[ ... ]

> @@ -702,14 +691,13 @@ static int ice_vsi_cfg_rxq(struct ice_rx_ring *ring)
>  			if (err)
>  				return err;
>
> -			if (!xdp_rxq_info_is_reg(&ring->xdp_rxq)) {
> -				err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
> -							 ring->q_index,
> -							 ring->q_vector->napi.napi_id,
> -							 ring->rx_buf_len);
> -				if (err)
> -					goto err_destroy_fq;
> -			}
> +			err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
> +						 ring->q_index,
> +						 ring->q_vector->napi.napi_id,
> +						 ring->rx_buf_len);
> +			if (err)
> +				goto err_destroy_fq;

The registration condition check was removed, so __xdp_rxq_info_reg() is
now called unconditionally. This works correctly when cleanup properly
unregisters the xdp_rxq, but there appears to be a gap for ICE_VSI_SF
in non-XSK mode.

Looking at ice_clean_rx_ring() in ice_txrx.c:

    if (rx_ring->vsi->type == ICE_VSI_PF &&
        xdp_rxq_info_is_reg(&rx_ring->xdp_rxq)) {
        xdp_rxq_info_detach_mem_model(&rx_ring->xdp_rxq);
        xdp_rxq_info_unreg(&rx_ring->xdp_rxq);
    }

This only unregisters for ICE_VSI_PF, not ICE_VSI_SF. With the old code,
the conditional registration skipped re-registration for ICE_VSI_SF. With
this change, for ICE_VSI_SF in non-XSK mode during reconfiguration:

1. ice_clean_rx_ring() - doesn't unregister (only checks ICE_VSI_PF)
2. ice_vsi_cfg_rxq() - unconditionally calls __xdp_rxq_info_reg()
3. __xdp_rxq_info_reg() sees REG_STATE_REGISTERED and triggers:

    WARN(1, "Missing unregister, handled but fix driver");

While this is handled gracefully by the XDP core (it unregisters internally
before re-registering), will this cause spurious WARN splats for subfunction
devices on every reconfiguration?

Should the ICE_VSI_PF check in ice_clean_rx_ring() be updated to also
include ICE_VSI_SF to match the registration path?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/21960574055

AI-authorship-score: low
AI-authorship-explanation: The commit shows domain-specific knowledge of ice driver internals and XDP subsystem, with targeted changes that suggest human authorship by an Intel driver developer.
issues-found: 1
issue-severity-score: low
issue-severity-explanation: The issue causes WARN splats for ICE_VSI_SF devices during reconfiguration, but is handled gracefully by XDP core without functional impact.

RE: [PATCH bpf v2 4/9] ice: change XDP RxQ frag_size from DMA write length to xdp.frame_sz

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Zaremba, Larysa <larysa.zaremba@intel.com>
> Sent: Thursday, February 12, 2026 7:33 PM
> To: bpf@vger.kernel.org
> Cc: Zaremba, Larysa <larysa.zaremba@intel.com>; Claudiu Manoil
> <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> Wei Fang <wei.fang@nxp.com>; Clark Wang <xiaoning.wang@nxp.com>;
> Andrew Lunn <andrew+netdev@lunn.ch>; David S. Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Nguyen,
> Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Alexei Starovoitov <ast@kernel.org>;
> Daniel Borkmann <daniel@iogearbox.net>; Jesper Dangaard Brouer
> <hawk@kernel.org>; John Fastabend <john.fastabend@gmail.com>;
> Stanislav Fomichev <sdf@fomichev.me>; Andrii Nakryiko
> <andrii@kernel.org>; Martin KaFai Lau <martin.lau@linux.dev>; Eduard
> Zingerman <eddyz87@gmail.com>; Song Liu <song@kernel.org>; Yonghong
> Song <yonghong.song@linux.dev>; KP Singh <kpsingh@kernel.org>; Hao Luo
> <haoluo@google.com>; Jiri Olsa <jolsa@kernel.org>; Simon Horman
> <horms@kernel.org>; Shuah Khan <shuah@kernel.org>; Lobakin, Aleksander
> <aleksander.lobakin@intel.com>; Fijalkowski, Maciej
> <maciej.fijalkowski@intel.com>; Bastien Curutchet (eBPF Foundation)
> <bastien.curutchet@bootlin.com>; Vyavahare, Tushar
> <tushar.vyavahare@intel.com>; Jason Xing <kernelxing@tencent.com>;
> Ricardo B. Marlière <rbm@suse.com>; Eelco Chaudron
> <echaudro@redhat.com>; Lorenzo Bianconi <lorenzo@kernel.org>; Toke
> Hoiland-Jorgensen <toke@redhat.com>; imx@lists.linux.dev;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; intel-wired-
> lan@lists.osuosl.org; linux-kselftest@vger.kernel.org; Loktionov,
> Aleksandr <aleksandr.loktionov@intel.com>; Dragos Tatulea
> <dtatulea@nvidia.com>
> Subject: [PATCH bpf v2 4/9] ice: change XDP RxQ frag_size from DMA
> write length to xdp.frame_sz
> 
> The only user of frag_size field in XDP RxQ info is
> bpf_xdp_frags_increase_tail(). It clearly expects whole buff size
> instead of DMA write size. Different assumptions in ice driver
> configuration lead to negative tailroom.
> 
> This allows to trigger kernel panic, when using
> XDP_ADJUST_TAIL_GROW_MULTI_BUFF xskxceiver test and changing packet
> size to
> 6912 and the requisted offset to a huge value, e.g.

"requisted offset" -> "requested offset"

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> XSK_UMEM__MAX_FRAME_SIZE * 100.
> 
> Due to other quirks of the ZC configuration in ice, panic is not
> observed in ZC mode, but tailroom growing still fails when it should
> not.
> 
> Use fill queue buffer truesize instead of DMA write size in XDP RxQ
> info.
> Fix ZC mode too by using the new helper.
> 
> Fixes: 2fba7dc5157b ("ice: Add support for XDP multi-buffer on Rx
> side")
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
> ---

...
 
> --
> 2.52.0

RE: [PATCH bpf v2 6/9] i40e: use xdp.frame_sz as XDP RxQ info frag_size

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Zaremba, Larysa <larysa.zaremba@intel.com>
> Sent: Thursday, February 12, 2026 7:33 PM
> To: bpf@vger.kernel.org
> Cc: Zaremba, Larysa <larysa.zaremba@intel.com>; Claudiu Manoil
> <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> Wei Fang <wei.fang@nxp.com>; Clark Wang <xiaoning.wang@nxp.com>;
> Andrew Lunn <andrew+netdev@lunn.ch>; David S. Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Nguyen,
> Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Alexei Starovoitov <ast@kernel.org>;
> Daniel Borkmann <daniel@iogearbox.net>; Jesper Dangaard Brouer
> <hawk@kernel.org>; John Fastabend <john.fastabend@gmail.com>;
> Stanislav Fomichev <sdf@fomichev.me>; Andrii Nakryiko
> <andrii@kernel.org>; Martin KaFai Lau <martin.lau@linux.dev>; Eduard
> Zingerman <eddyz87@gmail.com>; Song Liu <song@kernel.org>; Yonghong
> Song <yonghong.song@linux.dev>; KP Singh <kpsingh@kernel.org>; Hao
> Luo <haoluo@google.com>; Jiri Olsa <jolsa@kernel.org>; Simon Horman
> <horms@kernel.org>; Shuah Khan <shuah@kernel.org>; Lobakin,
> Aleksander <aleksander.lobakin@intel.com>; Fijalkowski, Maciej
> <maciej.fijalkowski@intel.com>; Bastien Curutchet (eBPF Foundation)
> <bastien.curutchet@bootlin.com>; Vyavahare, Tushar
> <tushar.vyavahare@intel.com>; Jason Xing <kernelxing@tencent.com>;
> Ricardo B. Marlière <rbm@suse.com>; Eelco Chaudron
> <echaudro@redhat.com>; Lorenzo Bianconi <lorenzo@kernel.org>; Toke
> Hoiland-Jorgensen <toke@redhat.com>; imx@lists.linux.dev;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; intel-wired-
> lan@lists.osuosl.org; linux-kselftest@vger.kernel.org; Loktionov,
> Aleksandr <aleksandr.loktionov@intel.com>; Dragos Tatulea
> <dtatulea@nvidia.com>
> Subject: [PATCH bpf v2 6/9] i40e: use xdp.frame_sz as XDP RxQ info
> frag_size
> 
> The only user of frag_size field in XDP RxQ info is
> bpf_xdp_frags_increase_tail(). It clearly expects whole buffer size
> instead of DMA write size. Different assumptions in i40e driver
> configuration lead to negative tailroom.
> 
> Set frag_size to the same value as frame_sz in shared pages mode,
> use new helper to set frag_size when AF_XDP ZC is active.
> 
> Fixes: a045d2f2d03d ("i40e: set xdp_rxq_info::frag_size")
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index eaa5b65e6daf..f91157774f1d 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -3563,6 +3563,7 @@ static int i40e_configure_rx_ring(struct
> i40e_ring *ring)
>  	u16 pf_q = vsi->base_queue + ring->queue_index;
>  	struct i40e_hw *hw = &vsi->back->hw;
>  	struct i40e_hmc_obj_rxq rx_ctx;
> +	u32 xdp_frame_sz = 0;
>  	int err = 0;
>  	bool ok;
> 
https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/net/ethernet/intel/i40e/i40e_main.c#L3577
All VSIs except main will be skipped, so xdp_init_buff() will be called with the default xdp_frame_sz = 0;
Is it ok?

> @@ -3579,11 +3580,12 @@ static int i40e_configure_rx_ring(struct
> i40e_ring *ring)
> 
>  	ring->xsk_pool = i40e_xsk_pool(ring);
>  	if (ring->xsk_pool) {
> +		xdp_frame_sz = xsk_pool_get_rx_frag_step(ring-
> >xsk_pool);
>  		ring->rx_buf_len = xsk_pool_get_rx_frame_size(ring-
> >xsk_pool);
>  		err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
>  					 ring->queue_index,
>  					 ring->q_vector->napi.napi_id,
> -					 ring->rx_buf_len);
> +					 xdp_frame_sz);
>  		if (err)
>  			return err;
>  		err = xdp_rxq_info_reg_mem_model(&ring->xdp_rxq,
> @@ -3596,10 +3598,11 @@ static int i40e_configure_rx_ring(struct
> i40e_ring *ring)
>  			 ring->queue_index);
> 
>  	} else {
> +		xdp_frame_sz = i40e_rx_pg_size(ring) / 2;
>  		err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
>  					 ring->queue_index,
>  					 ring->q_vector->napi.napi_id,
> -					 ring->rx_buf_len);
> +					 xdp_frame_sz);
>  		if (err)
>  			return err;
>  		err = xdp_rxq_info_reg_mem_model(&ring->xdp_rxq,
> @@ -3610,7 +3613,7 @@ static int i40e_configure_rx_ring(struct
> i40e_ring *ring)
>  	}
> 
>  skip:
> -	xdp_init_buff(&ring->xdp, i40e_rx_pg_size(ring) / 2, &ring-
> >xdp_rxq);
> +	xdp_init_buff(&ring->xdp, xdp_frame_sz, &ring->xdp_rxq);
> 
>  	rx_ctx.dbuff = DIV_ROUND_UP(ring->rx_buf_len,
>  				    BIT_ULL(I40E_RXQ_CTX_DBUFF_SHIFT));
> --
> 2.52.0

Re: [PATCH bpf v2 4/9] ice: change XDP RxQ frag_size from DMA write length to xdp.frame_sz

[Larysa Zaremba]

On Fri, Feb 13, 2026 at 04:57:25AM +0100, Loktionov, Aleksandr wrote:
> 
> 
> > -----Original Message-----
> > From: Zaremba, Larysa <larysa.zaremba@intel.com>
> > Sent: Thursday, February 12, 2026 7:33 PM
> > To: bpf@vger.kernel.org
> > Cc: Zaremba, Larysa <larysa.zaremba@intel.com>; Claudiu Manoil
> > <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> > Wei Fang <wei.fang@nxp.com>; Clark Wang <xiaoning.wang@nxp.com>;
> > Andrew Lunn <andrew+netdev@lunn.ch>; David S. Miller
> > <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> > Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Nguyen,
> > Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> > <przemyslaw.kitszel@intel.com>; Alexei Starovoitov <ast@kernel.org>;
> > Daniel Borkmann <daniel@iogearbox.net>; Jesper Dangaard Brouer
> > <hawk@kernel.org>; John Fastabend <john.fastabend@gmail.com>;
> > Stanislav Fomichev <sdf@fomichev.me>; Andrii Nakryiko
> > <andrii@kernel.org>; Martin KaFai Lau <martin.lau@linux.dev>; Eduard
> > Zingerman <eddyz87@gmail.com>; Song Liu <song@kernel.org>; Yonghong
> > Song <yonghong.song@linux.dev>; KP Singh <kpsingh@kernel.org>; Hao Luo
> > <haoluo@google.com>; Jiri Olsa <jolsa@kernel.org>; Simon Horman
> > <horms@kernel.org>; Shuah Khan <shuah@kernel.org>; Lobakin, Aleksander
> > <aleksander.lobakin@intel.com>; Fijalkowski, Maciej
> > <maciej.fijalkowski@intel.com>; Bastien Curutchet (eBPF Foundation)
> > <bastien.curutchet@bootlin.com>; Vyavahare, Tushar
> > <tushar.vyavahare@intel.com>; Jason Xing <kernelxing@tencent.com>;
> > Ricardo B. Marlière <rbm@suse.com>; Eelco Chaudron
> > <echaudro@redhat.com>; Lorenzo Bianconi <lorenzo@kernel.org>; Toke
> > Hoiland-Jorgensen <toke@redhat.com>; imx@lists.linux.dev;
> > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; intel-wired-
> > lan@lists.osuosl.org; linux-kselftest@vger.kernel.org; Loktionov,
> > Aleksandr <aleksandr.loktionov@intel.com>; Dragos Tatulea
> > <dtatulea@nvidia.com>
> > Subject: [PATCH bpf v2 4/9] ice: change XDP RxQ frag_size from DMA
> > write length to xdp.frame_sz
> > 
> > The only user of frag_size field in XDP RxQ info is
> > bpf_xdp_frags_increase_tail(). It clearly expects whole buff size
> > instead of DMA write size. Different assumptions in ice driver
> > configuration lead to negative tailroom.
> > 
> > This allows to trigger kernel panic, when using
> > XDP_ADJUST_TAIL_GROW_MULTI_BUFF xskxceiver test and changing packet
> > size to
> > 6912 and the requisted offset to a huge value, e.g.
> 
> "requisted offset" -> "requested offset"
> 

I am curious why codespell thinks this is fine.

Will fix, bot has correctly identified that u32 rx_buf_len; is not longer needed 
after this patch.

> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> > XSK_UMEM__MAX_FRAME_SIZE * 100.
> > 
> > Due to other quirks of the ZC configuration in ice, panic is not
> > observed in ZC mode, but tailroom growing still fails when it should
> > not.
> > 
> > Use fill queue buffer truesize instead of DMA write size in XDP RxQ
> > info.
> > Fix ZC mode too by using the new helper.
> > 
> > Fixes: 2fba7dc5157b ("ice: Add support for XDP multi-buffer on Rx
> > side")
> > Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
> > ---
> 
> ...
>  
> > --
> > 2.52.0
> 

Re: [PATCH bpf v2 6/9] i40e: use xdp.frame_sz as XDP RxQ info frag_size

[Larysa Zaremba]

On Fri, Feb 13, 2026 at 05:04:03AM +0100, Loktionov, Aleksandr wrote:
> 
> 
> > -----Original Message-----
> > From: Zaremba, Larysa <larysa.zaremba@intel.com>
> > Sent: Thursday, February 12, 2026 7:33 PM
> > To: bpf@vger.kernel.org
> > Cc: Zaremba, Larysa <larysa.zaremba@intel.com>; Claudiu Manoil
> > <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> > Wei Fang <wei.fang@nxp.com>; Clark Wang <xiaoning.wang@nxp.com>;
> > Andrew Lunn <andrew+netdev@lunn.ch>; David S. Miller
> > <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> > Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Nguyen,
> > Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> > <przemyslaw.kitszel@intel.com>; Alexei Starovoitov <ast@kernel.org>;
> > Daniel Borkmann <daniel@iogearbox.net>; Jesper Dangaard Brouer
> > <hawk@kernel.org>; John Fastabend <john.fastabend@gmail.com>;
> > Stanislav Fomichev <sdf@fomichev.me>; Andrii Nakryiko
> > <andrii@kernel.org>; Martin KaFai Lau <martin.lau@linux.dev>; Eduard
> > Zingerman <eddyz87@gmail.com>; Song Liu <song@kernel.org>; Yonghong
> > Song <yonghong.song@linux.dev>; KP Singh <kpsingh@kernel.org>; Hao
> > Luo <haoluo@google.com>; Jiri Olsa <jolsa@kernel.org>; Simon Horman
> > <horms@kernel.org>; Shuah Khan <shuah@kernel.org>; Lobakin,
> > Aleksander <aleksander.lobakin@intel.com>; Fijalkowski, Maciej
> > <maciej.fijalkowski@intel.com>; Bastien Curutchet (eBPF Foundation)
> > <bastien.curutchet@bootlin.com>; Vyavahare, Tushar
> > <tushar.vyavahare@intel.com>; Jason Xing <kernelxing@tencent.com>;
> > Ricardo B. Marlière <rbm@suse.com>; Eelco Chaudron
> > <echaudro@redhat.com>; Lorenzo Bianconi <lorenzo@kernel.org>; Toke
> > Hoiland-Jorgensen <toke@redhat.com>; imx@lists.linux.dev;
> > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; intel-wired-
> > lan@lists.osuosl.org; linux-kselftest@vger.kernel.org; Loktionov,
> > Aleksandr <aleksandr.loktionov@intel.com>; Dragos Tatulea
> > <dtatulea@nvidia.com>
> > Subject: [PATCH bpf v2 6/9] i40e: use xdp.frame_sz as XDP RxQ info
> > frag_size
> > 
> > The only user of frag_size field in XDP RxQ info is
> > bpf_xdp_frags_increase_tail(). It clearly expects whole buffer size
> > instead of DMA write size. Different assumptions in i40e driver
> > configuration lead to negative tailroom.
> > 
> > Set frag_size to the same value as frame_sz in shared pages mode,
> > use new helper to set frag_size when AF_XDP ZC is active.
> > 
> > Fixes: a045d2f2d03d ("i40e: set xdp_rxq_info::frag_size")
> > Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
> > ---
> >  drivers/net/ethernet/intel/i40e/i40e_main.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> > b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > index eaa5b65e6daf..f91157774f1d 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > @@ -3563,6 +3563,7 @@ static int i40e_configure_rx_ring(struct
> > i40e_ring *ring)
> >  	u16 pf_q = vsi->base_queue + ring->queue_index;
> >  	struct i40e_hw *hw = &vsi->back->hw;
> >  	struct i40e_hmc_obj_rxq rx_ctx;
> > +	u32 xdp_frame_sz = 0;
> >  	int err = 0;
> >  	bool ok;
> > 
> https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/net/ethernet/intel/i40e/i40e_main.c#L3577
> All VSIs except main will be skipped, so xdp_init_buff() will be called with the default xdp_frame_sz = 0;
> Is it ok?

Sorry, you forgot to remove your Reviewed-by(s), patch changed too much.
You are right, xdp_frame_sz should be (i40e_rx_pg_size(ring) / 2) for non-main 
VSIs. I assumed that they do not need xdp_buff, but it is unescapable in non-XDP 
Rx processing after XDP mbuf introduction.

> 
> > @@ -3579,11 +3580,12 @@ static int i40e_configure_rx_ring(struct
> > i40e_ring *ring)
> > 
> >  	ring->xsk_pool = i40e_xsk_pool(ring);
> >  	if (ring->xsk_pool) {
> > +		xdp_frame_sz = xsk_pool_get_rx_frag_step(ring-
> > >xsk_pool);
> >  		ring->rx_buf_len = xsk_pool_get_rx_frame_size(ring-
> > >xsk_pool);
> >  		err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
> >  					 ring->queue_index,
> >  					 ring->q_vector->napi.napi_id,
> > -					 ring->rx_buf_len);
> > +					 xdp_frame_sz);
> >  		if (err)
> >  			return err;
> >  		err = xdp_rxq_info_reg_mem_model(&ring->xdp_rxq,
> > @@ -3596,10 +3598,11 @@ static int i40e_configure_rx_ring(struct
> > i40e_ring *ring)
> >  			 ring->queue_index);
> > 
> >  	} else {
> > +		xdp_frame_sz = i40e_rx_pg_size(ring) / 2;
> >  		err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
> >  					 ring->queue_index,
> >  					 ring->q_vector->napi.napi_id,
> > -					 ring->rx_buf_len);
> > +					 xdp_frame_sz);
> >  		if (err)
> >  			return err;
> >  		err = xdp_rxq_info_reg_mem_model(&ring->xdp_rxq,
> > @@ -3610,7 +3613,7 @@ static int i40e_configure_rx_ring(struct
> > i40e_ring *ring)
> >  	}
> > 
> >  skip:
> > -	xdp_init_buff(&ring->xdp, i40e_rx_pg_size(ring) / 2, &ring-
> > >xdp_rxq);
> > +	xdp_init_buff(&ring->xdp, xdp_frame_sz, &ring->xdp_rxq);
> > 
> >  	rx_ctx.dbuff = DIV_ROUND_UP(ring->rx_buf_len,
> >  				    BIT_ULL(I40E_RXQ_CTX_DBUFF_SHIFT));
> > --
> > 2.52.0
> 

Re: [PATCH bpf v2 1/9] xdp: use modulo operation to calculate XDP frag tailroom

[Jakub Kicinski]

On Thu, 12 Feb 2026 19:33:16 +0100 Larysa Zaremba wrote:
> -	tailroom = rxq->frag_size - skb_frag_size(frag) - skb_frag_off(frag);
> +	tailroom = rxq->frag_size - skb_frag_size(frag) -
> +		   skb_frag_off(frag) % rxq->frag_size;
>  	if (unlikely(offset > tailroom))

Acked-by: Jakub Kicinski <kuba@kernel.org>

Re: [PATCH bpf v2 7/9] idpf: use truesize as XDP RxQ info frag_size

[Alexander Lobakin]

From: Zaremba, Larysa <larysa.zaremba@intel.com>
Date: Thu, 12 Feb 2026 19:33:22 +0100

> The only user of frag_size field in XDP RxQ info is
> bpf_xdp_frags_increase_tail(). It clearly expects whole buffer size instead
> of DMA write size. Different assumptions in idpf driver configuration lead
> to negative tailroom.
> 
> To make it worse, buffer sizes are not actually uniform in idpf when
> splitq is enabled, as there are several buffer queues, so rxq->rx_buf_size
> is meaningless in this case.
> 
> Use rxq->truesize as a frag_size for singleq and truesize of the first bufq
> in AF_XDP ZC, as there is only one. Disable growinf tail for regular
> splitq.
> 
> Fixes: ac8a861f632e ("idpf: prepare structures to support XDP")
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
> ---
>  drivers/net/ethernet/intel/idpf/xdp.c | 8 +++++++-
>  drivers/net/ethernet/intel/idpf/xsk.c | 1 +
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/xdp.c b/drivers/net/ethernet/intel/idpf/xdp.c
> index 958d16f87424..a152c9a26976 100644
> --- a/drivers/net/ethernet/intel/idpf/xdp.c
> +++ b/drivers/net/ethernet/intel/idpf/xdp.c
> @@ -46,11 +46,17 @@ static int __idpf_xdp_rxq_info_init(struct idpf_rx_queue *rxq, void *arg)
>  {
>  	const struct idpf_vport *vport = rxq->q_vector->vport;
>  	bool split = idpf_is_queue_model_split(vport->rxq_model);
> +	u32 frag_size = 0;
>  	int err;
>  
> +	if (idpf_queue_has(XSK, rxq) && split)
> +		frag_size = rxq->bufq_sets[0].bufq.truesize;
> +	else if (!split)
> +		frag_size = rxq->truesize;

XDP and XSk are supported only in mode splitq mode, so you can remove
the second condition and change the first one to just `has(XSK)`.

> +
>  	err = __xdp_rxq_info_reg(&rxq->xdp_rxq, vport->netdev, rxq->idx,
>  				 rxq->q_vector->napi.napi_id,
> -				 rxq->rx_buf_size);
> +				 frag_size);
>  	if (err)
>  		return err;
>  
> diff --git a/drivers/net/ethernet/intel/idpf/xsk.c b/drivers/net/ethernet/intel/idpf/xsk.c
> index fd2cc43ab43c..febe1073b9b4 100644
> --- a/drivers/net/ethernet/intel/idpf/xsk.c
> +++ b/drivers/net/ethernet/intel/idpf/xsk.c
> @@ -401,6 +401,7 @@ int idpf_xskfq_init(struct idpf_buf_queue *bufq)
>  	bufq->pending = fq.pending;
>  	bufq->thresh = fq.thresh;
>  	bufq->rx_buf_size = fq.buf_len;
> +	bufq->truesize = xsk_pool_get_rx_frag_step(fq.pool);
>  
>  	if (!idpf_xskfq_refill(bufq))
>  		netdev_err(bufq->pool->netdev,

Thanks,
Olek

Re: [PATCH bpf v2 7/9] idpf: use truesize as XDP RxQ info frag_size

[Alexander Lobakin]

From: Alexander Lobakin <aleksander.lobakin@intel.com>
Date: Mon, 16 Feb 2026 11:46:05 +0100

> From: Zaremba, Larysa <larysa.zaremba@intel.com>
> Date: Thu, 12 Feb 2026 19:33:22 +0100
> 
>> The only user of frag_size field in XDP RxQ info is
>> bpf_xdp_frags_increase_tail(). It clearly expects whole buffer size instead
>> of DMA write size. Different assumptions in idpf driver configuration lead
>> to negative tailroom.
>>
>> To make it worse, buffer sizes are not actually uniform in idpf when
>> splitq is enabled, as there are several buffer queues, so rxq->rx_buf_size
>> is meaningless in this case.
>>
>> Use rxq->truesize as a frag_size for singleq and truesize of the first bufq
>> in AF_XDP ZC, as there is only one. Disable growinf tail for regular
>> splitq.
>>
>> Fixes: ac8a861f632e ("idpf: prepare structures to support XDP")
>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>> Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
>> ---
>>  drivers/net/ethernet/intel/idpf/xdp.c | 8 +++++++-
>>  drivers/net/ethernet/intel/idpf/xsk.c | 1 +
>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/idpf/xdp.c b/drivers/net/ethernet/intel/idpf/xdp.c
>> index 958d16f87424..a152c9a26976 100644
>> --- a/drivers/net/ethernet/intel/idpf/xdp.c
>> +++ b/drivers/net/ethernet/intel/idpf/xdp.c
>> @@ -46,11 +46,17 @@ static int __idpf_xdp_rxq_info_init(struct idpf_rx_queue *rxq, void *arg)
>>  {
>>  	const struct idpf_vport *vport = rxq->q_vector->vport;
>>  	bool split = idpf_is_queue_model_split(vport->rxq_model);
>> +	u32 frag_size = 0;
>>  	int err;
>>  
>> +	if (idpf_queue_has(XSK, rxq) && split)
>> +		frag_size = rxq->bufq_sets[0].bufq.truesize;
>> +	else if (!split)
>> +		frag_size = rxq->truesize;
> 
> XDP and XSk are supported only in mode splitq mode, so you can remove
> the second condition and change the first one to just `has(XSK)`.
> 
>> +
>>  	err = __xdp_rxq_info_reg(&rxq->xdp_rxq, vport->netdev, rxq->idx,
>>  				 rxq->q_vector->napi.napi_id,
>> -				 rxq->rx_buf_size);
>> +				 frag_size);
>>  	if (err)
>>  		return err;
>>  
>> diff --git a/drivers/net/ethernet/intel/idpf/xsk.c b/drivers/net/ethernet/intel/idpf/xsk.c
>> index fd2cc43ab43c..febe1073b9b4 100644
>> --- a/drivers/net/ethernet/intel/idpf/xsk.c
>> +++ b/drivers/net/ethernet/intel/idpf/xsk.c
>> @@ -401,6 +401,7 @@ int idpf_xskfq_init(struct idpf_buf_queue *bufq)
>>  	bufq->pending = fq.pending;
>>  	bufq->thresh = fq.thresh;
>>  	bufq->rx_buf_size = fq.buf_len;
>> +	bufq->truesize = xsk_pool_get_rx_frag_step(fq.pool);

Better to do that in libeth_xdp rather than here?

>>  
>>  	if (!idpf_xskfq_refill(bufq))
>>  		netdev_err(bufq->pool->netdev,

Thanks,
Olek

Re: [PATCH bpf v2 7/9] idpf: use truesize as XDP RxQ info frag_size

[Larysa Zaremba]

On Mon, Feb 16, 2026 at 11:48:27AM +0100, Alexander Lobakin wrote:
> From: Alexander Lobakin <aleksander.lobakin@intel.com>
> Date: Mon, 16 Feb 2026 11:46:05 +0100
> 
> > From: Zaremba, Larysa <larysa.zaremba@intel.com>
> > Date: Thu, 12 Feb 2026 19:33:22 +0100
> > 
> >> The only user of frag_size field in XDP RxQ info is
> >> bpf_xdp_frags_increase_tail(). It clearly expects whole buffer size instead
> >> of DMA write size. Different assumptions in idpf driver configuration lead
> >> to negative tailroom.
> >>
> >> To make it worse, buffer sizes are not actually uniform in idpf when
> >> splitq is enabled, as there are several buffer queues, so rxq->rx_buf_size
> >> is meaningless in this case.
> >>
> >> Use rxq->truesize as a frag_size for singleq and truesize of the first bufq
> >> in AF_XDP ZC, as there is only one. Disable growinf tail for regular
> >> splitq.
> >>
> >> Fixes: ac8a861f632e ("idpf: prepare structures to support XDP")
> >> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> >> Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
> >> ---
> >>  drivers/net/ethernet/intel/idpf/xdp.c | 8 +++++++-
> >>  drivers/net/ethernet/intel/idpf/xsk.c | 1 +
> >>  2 files changed, 8 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/ethernet/intel/idpf/xdp.c b/drivers/net/ethernet/intel/idpf/xdp.c
> >> index 958d16f87424..a152c9a26976 100644
> >> --- a/drivers/net/ethernet/intel/idpf/xdp.c
> >> +++ b/drivers/net/ethernet/intel/idpf/xdp.c
> >> @@ -46,11 +46,17 @@ static int __idpf_xdp_rxq_info_init(struct idpf_rx_queue *rxq, void *arg)
> >>  {
> >>  	const struct idpf_vport *vport = rxq->q_vector->vport;
> >>  	bool split = idpf_is_queue_model_split(vport->rxq_model);
> >> +	u32 frag_size = 0;
> >>  	int err;
> >>  
> >> +	if (idpf_queue_has(XSK, rxq) && split)
> >> +		frag_size = rxq->bufq_sets[0].bufq.truesize;
> >> +	else if (!split)
> >> +		frag_size = rxq->truesize;
> > 
> > XDP and XSk are supported only in mode splitq mode, so you can remove
> > the second condition and change the first one to just `has(XSK)`.
> >

But the function itself handles singleq case. I do not see why frag_size should 
be treated differently.

Not that I am against of removing this logic, it would look more neat without 
these conditions.

> >> +
> >>  	err = __xdp_rxq_info_reg(&rxq->xdp_rxq, vport->netdev, rxq->idx,
> >>  				 rxq->q_vector->napi.napi_id,
> >> -				 rxq->rx_buf_size);
> >> +				 frag_size);
> >>  	if (err)
> >>  		return err;
> >>  
> >> diff --git a/drivers/net/ethernet/intel/idpf/xsk.c b/drivers/net/ethernet/intel/idpf/xsk.c
> >> index fd2cc43ab43c..febe1073b9b4 100644
> >> --- a/drivers/net/ethernet/intel/idpf/xsk.c
> >> +++ b/drivers/net/ethernet/intel/idpf/xsk.c
> >> @@ -401,6 +401,7 @@ int idpf_xskfq_init(struct idpf_buf_queue *bufq)
> >>  	bufq->pending = fq.pending;
> >>  	bufq->thresh = fq.thresh;
> >>  	bufq->rx_buf_size = fq.buf_len;
> >> +	bufq->truesize = xsk_pool_get_rx_frag_step(fq.pool);
> 
> Better to do that in libeth_xdp rather than here?
>

Smth like that?

diff --git a/drivers/net/ethernet/intel/idpf/xsk.c b/drivers/net/ethernet/intel/idpf/xsk.c
index febe1073b9b4..de87455b92d7 100644
--- a/drivers/net/ethernet/intel/idpf/xsk.c
+++ b/drivers/net/ethernet/intel/idpf/xsk.c
@@ -401,7 +401,7 @@ int idpf_xskfq_init(struct idpf_buf_queue *bufq)
        bufq->pending = fq.pending;
        bufq->thresh = fq.thresh;
        bufq->rx_buf_size = fq.buf_len;
-       bufq->truesize = xsk_pool_get_rx_frag_step(fq.pool);
+       bufq->truesize = fq.chunk_align;

        if (!idpf_xskfq_refill(bufq))
                netdev_err(bufq->pool->netdev,
diff --git a/drivers/net/ethernet/intel/libeth/xsk.c b/drivers/net/ethernet/intel/libeth/xsk.c
index 846e902e31b6..5b298558ecfd 100644
--- a/drivers/net/ethernet/intel/libeth/xsk.c
+++ b/drivers/net/ethernet/intel/libeth/xsk.c
@@ -167,6 +167,7 @@ int libeth_xskfq_create(struct libeth_xskfq *fq)
        fq->pending = fq->count;
        fq->thresh = libeth_xdp_queue_threshold(fq->count);
        fq->buf_len = xsk_pool_get_rx_frame_size(fq->pool);
+       fq->chunk_align = xsk_pool_get_rx_frag_step(fq->pool);

        return 0;
 }
diff --git a/include/net/libeth/xsk.h b/include/net/libeth/xsk.h
index 481a7b28e6f2..a3ea90d30d17 100644
--- a/include/net/libeth/xsk.h
+++ b/include/net/libeth/xsk.h
@@ -598,6 +598,7 @@ __libeth_xsk_run_pass(struct libeth_xdp_buff *xdp,
  * @thresh: threshold below which the queue is refilled
  * @buf_len: HW-writeable length per each buffer
  * @nid: ID of the closest NUMA node with memory
+ * @chunk_align: step between consecutive buffers, 0 if none exists
  */
 struct libeth_xskfq {
        struct_group_tagged(libeth_xskfq_fp, fp,
@@ -615,6 +616,8 @@ struct libeth_xskfq {

        u32                     buf_len;
        int                     nid;
+
+       u32                     chunk_align;
 };

 int libeth_xskfq_create(struct libeth_xskfq *fq);


> >>  
> >>  	if (!idpf_xskfq_refill(bufq))
> >>  		netdev_err(bufq->pool->netdev,
> 
> Thanks,
> Olek

Re: [PATCH bpf v2 7/9] idpf: use truesize as XDP RxQ info frag_size

[Alexander Lobakin]

From: Zaremba, Larysa <larysa.zaremba@intel.com>
Date: Mon, 16 Feb 2026 15:01:45 +0100

> On Mon, Feb 16, 2026 at 11:48:27AM +0100, Alexander Lobakin wrote:
>> From: Alexander Lobakin <aleksander.lobakin@intel.com>
>> Date: Mon, 16 Feb 2026 11:46:05 +0100
>>
>>> From: Zaremba, Larysa <larysa.zaremba@intel.com>
>>> Date: Thu, 12 Feb 2026 19:33:22 +0100
>>>
>>>> The only user of frag_size field in XDP RxQ info is
>>>> bpf_xdp_frags_increase_tail(). It clearly expects whole buffer size instead
>>>> of DMA write size. Different assumptions in idpf driver configuration lead
>>>> to negative tailroom.
>>>>
>>>> To make it worse, buffer sizes are not actually uniform in idpf when
>>>> splitq is enabled, as there are several buffer queues, so rxq->rx_buf_size
>>>> is meaningless in this case.
>>>>
>>>> Use rxq->truesize as a frag_size for singleq and truesize of the first bufq
>>>> in AF_XDP ZC, as there is only one. Disable growinf tail for regular
>>>> splitq.
>>>>
>>>> Fixes: ac8a861f632e ("idpf: prepare structures to support XDP")
>>>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>>>> Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
>>>> ---
>>>>  drivers/net/ethernet/intel/idpf/xdp.c | 8 +++++++-
>>>>  drivers/net/ethernet/intel/idpf/xsk.c | 1 +
>>>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/idpf/xdp.c b/drivers/net/ethernet/intel/idpf/xdp.c
>>>> index 958d16f87424..a152c9a26976 100644
>>>> --- a/drivers/net/ethernet/intel/idpf/xdp.c
>>>> +++ b/drivers/net/ethernet/intel/idpf/xdp.c
>>>> @@ -46,11 +46,17 @@ static int __idpf_xdp_rxq_info_init(struct idpf_rx_queue *rxq, void *arg)
>>>>  {
>>>>  	const struct idpf_vport *vport = rxq->q_vector->vport;
>>>>  	bool split = idpf_is_queue_model_split(vport->rxq_model);
>>>> +	u32 frag_size = 0;
>>>>  	int err;
>>>>  
>>>> +	if (idpf_queue_has(XSK, rxq) && split)
>>>> +		frag_size = rxq->bufq_sets[0].bufq.truesize;
>>>> +	else if (!split)
>>>> +		frag_size = rxq->truesize;
>>>
>>> XDP and XSk are supported only in mode splitq mode, so you can remove
>>> the second condition and change the first one to just `has(XSK)`.
>>>
> 
> But the function itself handles singleq case. I do not see why frag_size should 
> be treated differently.

singleq will still be handled and will always receive 0 here as in case
when XSk is not enabled.

> 
> Not that I am against of removing this logic, it would look more neat without 
> these conditions.
> 
>>>> +
>>>>  	err = __xdp_rxq_info_reg(&rxq->xdp_rxq, vport->netdev, rxq->idx,
>>>>  				 rxq->q_vector->napi.napi_id,
>>>> -				 rxq->rx_buf_size);
>>>> +				 frag_size);
>>>>  	if (err)
>>>>  		return err;
>>>>  
>>>> diff --git a/drivers/net/ethernet/intel/idpf/xsk.c b/drivers/net/ethernet/intel/idpf/xsk.c
>>>> index fd2cc43ab43c..febe1073b9b4 100644
>>>> --- a/drivers/net/ethernet/intel/idpf/xsk.c
>>>> +++ b/drivers/net/ethernet/intel/idpf/xsk.c
>>>> @@ -401,6 +401,7 @@ int idpf_xskfq_init(struct idpf_buf_queue *bufq)
>>>>  	bufq->pending = fq.pending;
>>>>  	bufq->thresh = fq.thresh;
>>>>  	bufq->rx_buf_size = fq.buf_len;
>>>> +	bufq->truesize = xsk_pool_get_rx_frag_step(fq.pool);
>>
>> Better to do that in libeth_xdp rather than here?
>>
> 
> Smth like that?
> 
> diff --git a/drivers/net/ethernet/intel/idpf/xsk.c b/drivers/net/ethernet/intel/idpf/xsk.c
> index febe1073b9b4..de87455b92d7 100644
> --- a/drivers/net/ethernet/intel/idpf/xsk.c
> +++ b/drivers/net/ethernet/intel/idpf/xsk.c
> @@ -401,7 +401,7 @@ int idpf_xskfq_init(struct idpf_buf_queue *bufq)
>         bufq->pending = fq.pending;
>         bufq->thresh = fq.thresh;
>         bufq->rx_buf_size = fq.buf_len;
> -       bufq->truesize = xsk_pool_get_rx_frag_step(fq.pool);
> +       bufq->truesize = fq.chunk_align;
> 
>         if (!idpf_xskfq_refill(bufq))
>                 netdev_err(bufq->pool->netdev,
> diff --git a/drivers/net/ethernet/intel/libeth/xsk.c b/drivers/net/ethernet/intel/libeth/xsk.c
> index 846e902e31b6..5b298558ecfd 100644
> --- a/drivers/net/ethernet/intel/libeth/xsk.c
> +++ b/drivers/net/ethernet/intel/libeth/xsk.c
> @@ -167,6 +167,7 @@ int libeth_xskfq_create(struct libeth_xskfq *fq)
>         fq->pending = fq->count;
>         fq->thresh = libeth_xdp_queue_threshold(fq->count);
>         fq->buf_len = xsk_pool_get_rx_frame_size(fq->pool);
> +       fq->chunk_align = xsk_pool_get_rx_frag_step(fq->pool);
> 
>         return 0;
>  }
> diff --git a/include/net/libeth/xsk.h b/include/net/libeth/xsk.h
> index 481a7b28e6f2..a3ea90d30d17 100644
> --- a/include/net/libeth/xsk.h
> +++ b/include/net/libeth/xsk.h
> @@ -598,6 +598,7 @@ __libeth_xsk_run_pass(struct libeth_xdp_buff *xdp,
>   * @thresh: threshold below which the queue is refilled
>   * @buf_len: HW-writeable length per each buffer
>   * @nid: ID of the closest NUMA node with memory
> + * @chunk_align: step between consecutive buffers, 0 if none exists
>   */
>  struct libeth_xskfq {
>         struct_group_tagged(libeth_xskfq_fp, fp,
> @@ -615,6 +616,8 @@ struct libeth_xskfq {
> 
>         u32                     buf_len;
>         int                     nid;
> +
> +       u32                     chunk_align;
>  };
> 
>  int libeth_xskfq_create(struct libeth_xskfq *fq);

Ah, I forgot that for XSkFQ we have a separate structure...

I'd do it like that:

	u32 buf_len;
	u32 truesize; // new field

	int nid;
};

Maybe it's not really truesize, but it will be more straightforward and
clear to the users what it means and which field they should assign the
value to.
truesize/chunk_align is related closely to buf_len, so I'd group them
together in the struct, while nid is a different story, so it might stay
separated by a newline.

> 
> 
>>>>  
>>>>  	if (!idpf_xskfq_refill(bufq))
>>>>  		netdev_err(bufq->pool->netdev,
Thanks,
Olek