* [Intel-wired-lan] [PATCH net v2] i40e: xsk: fix multi-buffer XDP_PASS skb construction
@ 2026-07-17 1:24 Chenguang Zhao
2026-07-20 15:03 ` Alexander Lobakin
2026-07-21 23:15 ` Jason Xing
0 siblings, 2 replies; 6+ messages in thread
From: Chenguang Zhao @ 2026-07-17 1:24 UTC (permalink / raw)
To: anthony.l.nguyen, przemyslaw.kitszel, andrew+netdev, davem,
edumazet, kuba, pabeni
Cc: intel-wired-lan, netdev, chenguang.zhao, Chenguang Zhao
From: Chenguang Zhao <zhaochenguang@kylinos.cn>
When AF_XDP ZC receives a multi-buffer frame and XDP returns XDP_PASS,
i40e_construct_skb_zc() copied frags incorrectly: memcpy used
skb_frag_page() (page metadata) and __skb_fill_page_desc_noacc() was
given a virtual address instead of a struct page *.
Drop the custom helper and use xdp_build_skb_from_zc() instead. On
failure, free the xdp buff in the caller. Push the Ethernet header
back before eth_skb_pad()/i40e_process_skb_fields() because
xdp_build_skb_from_zc() already called eth_type_trans().
Fixes: 1c9ba9c14658 ("i40e: xsk: add RX multi-buffer support")
Signed-off-by: Chenguang Zhao <zhaochenguang@kylinos.cn>
---
Revised as suggested by Maciej:
- Replace i40e_construct_skb_zc() with xdp_build_skb_from_zc()
v1:
https://lore.kernel.org/all/20260714025112.284724-1-chenguang.zhao@linux.dev/
drivers/net/ethernet/intel/i40e/i40e_xsk.c | 73 +++-------------------
1 file changed, 8 insertions(+), 65 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 9f47388eaba5..1319a5c22625 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -3,6 +3,7 @@
#include <linux/bpf_trace.h>
#include <linux/unroll.h>
+#include <net/xdp.h>
#include <net/xdp_sock_drv.h>
#include "i40e_txrx_common.h"
#include "i40e_xsk.h"
@@ -277,70 +278,6 @@ bool i40e_alloc_rx_buffers_zc(struct i40e_ring *rx_ring, u16 count)
return count == nb_buffs;
}
-/**
- * i40e_construct_skb_zc - Create skbuff from zero-copy Rx buffer
- * @rx_ring: Rx ring
- * @xdp: xdp_buff
- *
- * This functions allocates a new skb from a zero-copy Rx buffer.
- *
- * Returns the skb, or NULL on failure.
- **/
-static struct sk_buff *i40e_construct_skb_zc(struct i40e_ring *rx_ring,
- struct xdp_buff *xdp)
-{
- unsigned int totalsize = xdp->data_end - xdp->data_meta;
- unsigned int metasize = xdp->data - xdp->data_meta;
- struct skb_shared_info *sinfo = NULL;
- struct sk_buff *skb;
- u32 nr_frags = 0;
-
- if (unlikely(xdp_buff_has_frags(xdp))) {
- sinfo = xdp_get_shared_info_from_buff(xdp);
- nr_frags = sinfo->nr_frags;
- }
- net_prefetch(xdp->data_meta);
-
- /* allocate a skb to store the frags */
- skb = napi_alloc_skb(&rx_ring->q_vector->napi, totalsize);
- if (unlikely(!skb))
- goto out;
-
- memcpy(__skb_put(skb, totalsize), xdp->data_meta,
- ALIGN(totalsize, sizeof(long)));
-
- if (metasize) {
- skb_metadata_set(skb, metasize);
- __skb_pull(skb, metasize);
- }
-
- if (likely(!xdp_buff_has_frags(xdp)))
- goto out;
-
- for (int i = 0; i < nr_frags; i++) {
- struct skb_shared_info *skinfo = skb_shinfo(skb);
- skb_frag_t *frag = &sinfo->frags[i];
- struct page *page;
- void *addr;
-
- page = dev_alloc_page();
- if (!page) {
- dev_kfree_skb(skb);
- return NULL;
- }
- addr = page_to_virt(page);
-
- memcpy(addr, skb_frag_page(frag), skb_frag_size(frag));
-
- __skb_fill_page_desc_noacc(skinfo, skinfo->nr_frags++,
- addr, 0, skb_frag_size(frag));
- }
-
-out:
- xsk_buff_free(xdp);
- return skb;
-}
-
static void i40e_handle_xdp_result_zc(struct i40e_ring *rx_ring,
struct xdp_buff *xdp_buff,
union i40e_rx_desc *rx_desc,
@@ -372,14 +309,20 @@ static void i40e_handle_xdp_result_zc(struct i40e_ring *rx_ring,
* BIT(I40E_RXD_QW1_ERROR_SHIFT). This is due to that
* SBP is *not* set in PRT_SBPVSI (default not set).
*/
- skb = i40e_construct_skb_zc(rx_ring, xdp_buff);
+ skb = xdp_build_skb_from_zc(xdp_buff);
if (!skb) {
+ xsk_buff_free(xdp_buff);
rx_ring->rx_stats.alloc_buff_failed++;
*rx_packets = 0;
*rx_bytes = 0;
return;
}
+ /* xdp_build_skb_from_zc() already ran eth_type_trans();
+ * restore the header for eth_skb_pad()/process_skb_fields().
+ */
+ __skb_push(skb, skb->data - skb_mac_header(skb));
+
if (eth_skb_pad(skb)) {
*rx_packets = 0;
*rx_bytes = 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Intel-wired-lan] [PATCH net v2] i40e: xsk: fix multi-buffer XDP_PASS skb construction 2026-07-17 1:24 [Intel-wired-lan] [PATCH net v2] i40e: xsk: fix multi-buffer XDP_PASS skb construction Chenguang Zhao @ 2026-07-20 15:03 ` Alexander Lobakin 2026-07-21 23:15 ` Jason Xing 1 sibling, 0 replies; 6+ messages in thread From: Alexander Lobakin @ 2026-07-20 15:03 UTC (permalink / raw) To: Chenguang Zhao Cc: anthony.l.nguyen, przemyslaw.kitszel, andrew+netdev, davem, edumazet, kuba, pabeni, intel-wired-lan, netdev, Chenguang Zhao From: Chenguang Zhao <chenguang.zhao@linux.dev> Date: Fri, 17 Jul 2026 09:24:16 +0800 > From: Chenguang Zhao <zhaochenguang@kylinos.cn> > > When AF_XDP ZC receives a multi-buffer frame and XDP returns XDP_PASS, > i40e_construct_skb_zc() copied frags incorrectly: memcpy used > skb_frag_page() (page metadata) and __skb_fill_page_desc_noacc() was > given a virtual address instead of a struct page *. > > Drop the custom helper and use xdp_build_skb_from_zc() instead. On > failure, free the xdp buff in the caller. Push the Ethernet header > back before eth_skb_pad()/i40e_process_skb_fields() because > xdp_build_skb_from_zc() already called eth_type_trans(). > > Fixes: 1c9ba9c14658 ("i40e: xsk: add RX multi-buffer support") > Signed-off-by: Chenguang Zhao <zhaochenguang@kylinos.cn> Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com> One nit below tho. [...] > @@ -372,14 +309,20 @@ static void i40e_handle_xdp_result_zc(struct i40e_ring *rx_ring, > * BIT(I40E_RXD_QW1_ERROR_SHIFT). This is due to that > * SBP is *not* set in PRT_SBPVSI (default not set). > */ > - skb = i40e_construct_skb_zc(rx_ring, xdp_buff); > + skb = xdp_build_skb_from_zc(xdp_buff); > if (!skb) { > + xsk_buff_free(xdp_buff); > rx_ring->rx_stats.alloc_buff_failed++; > *rx_packets = 0; > *rx_bytes = 0; > return; > } > > + /* xdp_build_skb_from_zc() already ran eth_type_trans(); > + * restore the header for eth_skb_pad()/process_skb_fields(). > + */ The netdev rules prefer generic comment style over what we used in the past for some time already. I.e. /* * xdp_build_skb_from_zc() ... * restore ... */ > + __skb_push(skb, skb->data - skb_mac_header(skb)); > + > if (eth_skb_pad(skb)) { > *rx_packets = 0; > *rx_bytes = 0; Thanks, Olek ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v2] i40e: xsk: fix multi-buffer XDP_PASS skb construction 2026-07-17 1:24 [Intel-wired-lan] [PATCH net v2] i40e: xsk: fix multi-buffer XDP_PASS skb construction Chenguang Zhao 2026-07-20 15:03 ` Alexander Lobakin @ 2026-07-21 23:15 ` Jason Xing 2026-07-23 7:02 ` Chenguang Zhao 1 sibling, 1 reply; 6+ messages in thread From: Jason Xing @ 2026-07-21 23:15 UTC (permalink / raw) To: Chenguang Zhao Cc: anthony.l.nguyen, przemyslaw.kitszel, andrew+netdev, davem, edumazet, kuba, pabeni, intel-wired-lan, netdev, Chenguang Zhao On Fri, Jul 17, 2026 at 9:24 AM Chenguang Zhao <chenguang.zhao@linux.dev> wrote: > > From: Chenguang Zhao <zhaochenguang@kylinos.cn> > > When AF_XDP ZC receives a multi-buffer frame and XDP returns XDP_PASS, > i40e_construct_skb_zc() copied frags incorrectly: memcpy used > skb_frag_page() (page metadata) and __skb_fill_page_desc_noacc() was > given a virtual address instead of a struct page *. > > Drop the custom helper and use xdp_build_skb_from_zc() instead. On > failure, free the xdp buff in the caller. Push the Ethernet header > back before eth_skb_pad()/i40e_process_skb_fields() because > xdp_build_skb_from_zc() already called eth_type_trans(). > > Fixes: 1c9ba9c14658 ("i40e: xsk: add RX multi-buffer support") > Signed-off-by: Chenguang Zhao <zhaochenguang@kylinos.cn> > --- > Revised as suggested by Maciej: > - Replace i40e_construct_skb_zc() with xdp_build_skb_from_zc() I might have a different opinion on this patch: yes, it actually belongs to -next material. The process should be like: 1) fix the issues by v1, 2) refactor it by v2. The reason behind that is the helper was introduced in 2024 while the home-grown part was in 2023, which means it doesn't help for stable steam to cherry-pick the patch in older kernels like 6.6[1]. [1]: https://www.kernel.org/ Thanks, Jason > > v1: > https://lore.kernel.org/all/20260714025112.284724-1-chenguang.zhao@linux.dev/ > > drivers/net/ethernet/intel/i40e/i40e_xsk.c | 73 +++------------------- > 1 file changed, 8 insertions(+), 65 deletions(-) > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c > index 9f47388eaba5..1319a5c22625 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c > @@ -3,6 +3,7 @@ > > #include <linux/bpf_trace.h> > #include <linux/unroll.h> > +#include <net/xdp.h> > #include <net/xdp_sock_drv.h> > #include "i40e_txrx_common.h" > #include "i40e_xsk.h" > @@ -277,70 +278,6 @@ bool i40e_alloc_rx_buffers_zc(struct i40e_ring *rx_ring, u16 count) > return count == nb_buffs; > } > > -/** > - * i40e_construct_skb_zc - Create skbuff from zero-copy Rx buffer > - * @rx_ring: Rx ring > - * @xdp: xdp_buff > - * > - * This functions allocates a new skb from a zero-copy Rx buffer. > - * > - * Returns the skb, or NULL on failure. > - **/ > -static struct sk_buff *i40e_construct_skb_zc(struct i40e_ring *rx_ring, > - struct xdp_buff *xdp) > -{ > - unsigned int totalsize = xdp->data_end - xdp->data_meta; > - unsigned int metasize = xdp->data - xdp->data_meta; > - struct skb_shared_info *sinfo = NULL; > - struct sk_buff *skb; > - u32 nr_frags = 0; > - > - if (unlikely(xdp_buff_has_frags(xdp))) { > - sinfo = xdp_get_shared_info_from_buff(xdp); > - nr_frags = sinfo->nr_frags; > - } > - net_prefetch(xdp->data_meta); > - > - /* allocate a skb to store the frags */ > - skb = napi_alloc_skb(&rx_ring->q_vector->napi, totalsize); > - if (unlikely(!skb)) > - goto out; > - > - memcpy(__skb_put(skb, totalsize), xdp->data_meta, > - ALIGN(totalsize, sizeof(long))); > - > - if (metasize) { > - skb_metadata_set(skb, metasize); > - __skb_pull(skb, metasize); > - } > - > - if (likely(!xdp_buff_has_frags(xdp))) > - goto out; > - > - for (int i = 0; i < nr_frags; i++) { > - struct skb_shared_info *skinfo = skb_shinfo(skb); > - skb_frag_t *frag = &sinfo->frags[i]; > - struct page *page; > - void *addr; > - > - page = dev_alloc_page(); > - if (!page) { > - dev_kfree_skb(skb); > - return NULL; > - } > - addr = page_to_virt(page); > - > - memcpy(addr, skb_frag_page(frag), skb_frag_size(frag)); > - > - __skb_fill_page_desc_noacc(skinfo, skinfo->nr_frags++, > - addr, 0, skb_frag_size(frag)); > - } > - > -out: > - xsk_buff_free(xdp); > - return skb; > -} > - > static void i40e_handle_xdp_result_zc(struct i40e_ring *rx_ring, > struct xdp_buff *xdp_buff, > union i40e_rx_desc *rx_desc, > @@ -372,14 +309,20 @@ static void i40e_handle_xdp_result_zc(struct i40e_ring *rx_ring, > * BIT(I40E_RXD_QW1_ERROR_SHIFT). This is due to that > * SBP is *not* set in PRT_SBPVSI (default not set). > */ > - skb = i40e_construct_skb_zc(rx_ring, xdp_buff); > + skb = xdp_build_skb_from_zc(xdp_buff); > if (!skb) { > + xsk_buff_free(xdp_buff); > rx_ring->rx_stats.alloc_buff_failed++; > *rx_packets = 0; > *rx_bytes = 0; > return; > } > > + /* xdp_build_skb_from_zc() already ran eth_type_trans(); > + * restore the header for eth_skb_pad()/process_skb_fields(). > + */ > + __skb_push(skb, skb->data - skb_mac_header(skb)); > + > if (eth_skb_pad(skb)) { > *rx_packets = 0; > *rx_bytes = 0; > -- > 2.25.1 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v2] i40e: xsk: fix multi-buffer XDP_PASS skb construction 2026-07-21 23:15 ` Jason Xing @ 2026-07-23 7:02 ` Chenguang Zhao 2026-07-23 7:59 ` Jason Xing 0 siblings, 1 reply; 6+ messages in thread From: Chenguang Zhao @ 2026-07-23 7:02 UTC (permalink / raw) To: Jason Xing Cc: anthony.l.nguyen, przemyslaw.kitszel, andrew+netdev, davem, edumazet, kuba, pabeni, intel-wired-lan, netdev, Chenguang Zhao Thanks for the clarification. I agree: the minimal bugfix belongs in -net for stable backportability, and the conversion to xdp_build_skb_from_zc() belongs in -next. Please apply the existing v1 bugfix to -net: https://lore.kernel.org/all/20260714025112.284724-1-chenguang.zhao@linux.dev/ It already has : Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> I will drop v2 as a Fixes candidate. In parallel, I will send a separate net-next series (as v1) that replaces i40e_construct_skb_zc() with xdp_build_skb_from_zc(), based on net-next. That patch will not carry a Fixes tag. Thanks, Chenguang 在 2026/7/22 07:15, Jason Xing 写道: > On Fri, Jul 17, 2026 at 9:24 AM Chenguang Zhao <chenguang.zhao@linux.dev> wrote: >> From: Chenguang Zhao <zhaochenguang@kylinos.cn> >> >> When AF_XDP ZC receives a multi-buffer frame and XDP returns XDP_PASS, >> i40e_construct_skb_zc() copied frags incorrectly: memcpy used >> skb_frag_page() (page metadata) and __skb_fill_page_desc_noacc() was >> given a virtual address instead of a struct page *. >> >> Drop the custom helper and use xdp_build_skb_from_zc() instead. On >> failure, free the xdp buff in the caller. Push the Ethernet header >> back before eth_skb_pad()/i40e_process_skb_fields() because >> xdp_build_skb_from_zc() already called eth_type_trans(). >> >> Fixes: 1c9ba9c14658 ("i40e: xsk: add RX multi-buffer support") >> Signed-off-by: Chenguang Zhao <zhaochenguang@kylinos.cn> >> --- >> Revised as suggested by Maciej: >> - Replace i40e_construct_skb_zc() with xdp_build_skb_from_zc() > I might have a different opinion on this patch: yes, it actually > belongs to -next material. The process should be like: 1) fix the > issues by v1, 2) refactor it by v2. The reason behind that is the > helper was introduced in 2024 while the home-grown part was in 2023, > which means it doesn't help for stable steam to cherry-pick the patch > in older kernels like 6.6[1]. > > [1]: https://www.kernel.org/ > > Thanks, > Jason > >> v1: >> https://lore.kernel.org/all/20260714025112.284724-1-chenguang.zhao@linux.dev/ >> >> drivers/net/ethernet/intel/i40e/i40e_xsk.c | 73 +++------------------- >> 1 file changed, 8 insertions(+), 65 deletions(-) >> >> diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c >> index 9f47388eaba5..1319a5c22625 100644 >> --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c >> +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c >> @@ -3,6 +3,7 @@ >> >> #include <linux/bpf_trace.h> >> #include <linux/unroll.h> >> +#include <net/xdp.h> >> #include <net/xdp_sock_drv.h> >> #include "i40e_txrx_common.h" >> #include "i40e_xsk.h" >> @@ -277,70 +278,6 @@ bool i40e_alloc_rx_buffers_zc(struct i40e_ring *rx_ring, u16 count) >> return count == nb_buffs; >> } >> >> -/** >> - * i40e_construct_skb_zc - Create skbuff from zero-copy Rx buffer >> - * @rx_ring: Rx ring >> - * @xdp: xdp_buff >> - * >> - * This functions allocates a new skb from a zero-copy Rx buffer. >> - * >> - * Returns the skb, or NULL on failure. >> - **/ >> -static struct sk_buff *i40e_construct_skb_zc(struct i40e_ring *rx_ring, >> - struct xdp_buff *xdp) >> -{ >> - unsigned int totalsize = xdp->data_end - xdp->data_meta; >> - unsigned int metasize = xdp->data - xdp->data_meta; >> - struct skb_shared_info *sinfo = NULL; >> - struct sk_buff *skb; >> - u32 nr_frags = 0; >> - >> - if (unlikely(xdp_buff_has_frags(xdp))) { >> - sinfo = xdp_get_shared_info_from_buff(xdp); >> - nr_frags = sinfo->nr_frags; >> - } >> - net_prefetch(xdp->data_meta); >> - >> - /* allocate a skb to store the frags */ >> - skb = napi_alloc_skb(&rx_ring->q_vector->napi, totalsize); >> - if (unlikely(!skb)) >> - goto out; >> - >> - memcpy(__skb_put(skb, totalsize), xdp->data_meta, >> - ALIGN(totalsize, sizeof(long))); >> - >> - if (metasize) { >> - skb_metadata_set(skb, metasize); >> - __skb_pull(skb, metasize); >> - } >> - >> - if (likely(!xdp_buff_has_frags(xdp))) >> - goto out; >> - >> - for (int i = 0; i < nr_frags; i++) { >> - struct skb_shared_info *skinfo = skb_shinfo(skb); >> - skb_frag_t *frag = &sinfo->frags[i]; >> - struct page *page; >> - void *addr; >> - >> - page = dev_alloc_page(); >> - if (!page) { >> - dev_kfree_skb(skb); >> - return NULL; >> - } >> - addr = page_to_virt(page); >> - >> - memcpy(addr, skb_frag_page(frag), skb_frag_size(frag)); >> - >> - __skb_fill_page_desc_noacc(skinfo, skinfo->nr_frags++, >> - addr, 0, skb_frag_size(frag)); >> - } >> - >> -out: >> - xsk_buff_free(xdp); >> - return skb; >> -} >> - >> static void i40e_handle_xdp_result_zc(struct i40e_ring *rx_ring, >> struct xdp_buff *xdp_buff, >> union i40e_rx_desc *rx_desc, >> @@ -372,14 +309,20 @@ static void i40e_handle_xdp_result_zc(struct i40e_ring *rx_ring, >> * BIT(I40E_RXD_QW1_ERROR_SHIFT). This is due to that >> * SBP is *not* set in PRT_SBPVSI (default not set). >> */ >> - skb = i40e_construct_skb_zc(rx_ring, xdp_buff); >> + skb = xdp_build_skb_from_zc(xdp_buff); >> if (!skb) { >> + xsk_buff_free(xdp_buff); >> rx_ring->rx_stats.alloc_buff_failed++; >> *rx_packets = 0; >> *rx_bytes = 0; >> return; >> } >> >> + /* xdp_build_skb_from_zc() already ran eth_type_trans(); >> + * restore the header for eth_skb_pad()/process_skb_fields(). >> + */ >> + __skb_push(skb, skb->data - skb_mac_header(skb)); >> + >> if (eth_skb_pad(skb)) { >> *rx_packets = 0; >> *rx_bytes = 0; >> -- >> 2.25.1 >> >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v2] i40e: xsk: fix multi-buffer XDP_PASS skb construction 2026-07-23 7:02 ` Chenguang Zhao @ 2026-07-23 7:59 ` Jason Xing 2026-07-23 10:04 ` Maciej Fijalkowski 0 siblings, 1 reply; 6+ messages in thread From: Jason Xing @ 2026-07-23 7:59 UTC (permalink / raw) To: Chenguang Zhao Cc: anthony.l.nguyen, przemyslaw.kitszel, andrew+netdev, davem, edumazet, kuba, pabeni, intel-wired-lan, netdev, Chenguang Zhao On Thu, Jul 23, 2026 at 3:02 PM Chenguang Zhao <chenguang.zhao@linux.dev> wrote: > > Thanks for the clarification. Please do not top-post. > > I agree: the minimal bugfix belongs in -net for stable backportability, and the conversion to xdp_build_skb_from_zc() belongs in -next. Right. That's what I meant. But I will let Maciej/Tony/Olek make the decision. > > Please apply the existing v1 bugfix to -net: > > https://lore.kernel.org/all/20260714025112.284724-1-chenguang.zhao@linux.dev/ If we eventually need this patch, then you will send a v3 patch instead of asking maintainers to pick it up :) And feel free to add: Reviewed-by: Jason Xing <kerneljasonxing@gmail.com> > > It already has : > > Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> > > I will drop v2 as a Fixes candidate. > > In parallel, I will send a separate net-next series (as v1) that replaces > > i40e_construct_skb_zc() with xdp_build_skb_from_zc(), based on > > net-next. That patch will not carry a Fixes tag. As to this version, my tag still holds: Reviewed-by: Jason Xing <kerneljasonxing@gmail.com> Thanks, Jason > > > Thanks, > > Chenguang > > > 在 2026/7/22 07:15, Jason Xing 写道: > > On Fri, Jul 17, 2026 at 9:24 AM Chenguang Zhao <chenguang.zhao@linux.dev> wrote: > >> From: Chenguang Zhao <zhaochenguang@kylinos.cn> > >> > >> When AF_XDP ZC receives a multi-buffer frame and XDP returns XDP_PASS, > >> i40e_construct_skb_zc() copied frags incorrectly: memcpy used > >> skb_frag_page() (page metadata) and __skb_fill_page_desc_noacc() was > >> given a virtual address instead of a struct page *. > >> > >> Drop the custom helper and use xdp_build_skb_from_zc() instead. On > >> failure, free the xdp buff in the caller. Push the Ethernet header > >> back before eth_skb_pad()/i40e_process_skb_fields() because > >> xdp_build_skb_from_zc() already called eth_type_trans(). > >> > >> Fixes: 1c9ba9c14658 ("i40e: xsk: add RX multi-buffer support") > >> Signed-off-by: Chenguang Zhao <zhaochenguang@kylinos.cn> > >> --- > >> Revised as suggested by Maciej: > >> - Replace i40e_construct_skb_zc() with xdp_build_skb_from_zc() > > I might have a different opinion on this patch: yes, it actually > > belongs to -next material. The process should be like: 1) fix the > > issues by v1, 2) refactor it by v2. The reason behind that is the > > helper was introduced in 2024 while the home-grown part was in 2023, > > which means it doesn't help for stable steam to cherry-pick the patch > > in older kernels like 6.6[1]. > > > > [1]: https://www.kernel.org/ > > > > Thanks, > > Jason > > > >> v1: > >> https://lore.kernel.org/all/20260714025112.284724-1-chenguang.zhao@linux.dev/ > >> > >> drivers/net/ethernet/intel/i40e/i40e_xsk.c | 73 +++------------------- > >> 1 file changed, 8 insertions(+), 65 deletions(-) > >> > >> diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c > >> index 9f47388eaba5..1319a5c22625 100644 > >> --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c > >> +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c > >> @@ -3,6 +3,7 @@ > >> > >> #include <linux/bpf_trace.h> > >> #include <linux/unroll.h> > >> +#include <net/xdp.h> > >> #include <net/xdp_sock_drv.h> > >> #include "i40e_txrx_common.h" > >> #include "i40e_xsk.h" > >> @@ -277,70 +278,6 @@ bool i40e_alloc_rx_buffers_zc(struct i40e_ring *rx_ring, u16 count) > >> return count == nb_buffs; > >> } > >> > >> -/** > >> - * i40e_construct_skb_zc - Create skbuff from zero-copy Rx buffer > >> - * @rx_ring: Rx ring > >> - * @xdp: xdp_buff > >> - * > >> - * This functions allocates a new skb from a zero-copy Rx buffer. > >> - * > >> - * Returns the skb, or NULL on failure. > >> - **/ > >> -static struct sk_buff *i40e_construct_skb_zc(struct i40e_ring *rx_ring, > >> - struct xdp_buff *xdp) > >> -{ > >> - unsigned int totalsize = xdp->data_end - xdp->data_meta; > >> - unsigned int metasize = xdp->data - xdp->data_meta; > >> - struct skb_shared_info *sinfo = NULL; > >> - struct sk_buff *skb; > >> - u32 nr_frags = 0; > >> - > >> - if (unlikely(xdp_buff_has_frags(xdp))) { > >> - sinfo = xdp_get_shared_info_from_buff(xdp); > >> - nr_frags = sinfo->nr_frags; > >> - } > >> - net_prefetch(xdp->data_meta); > >> - > >> - /* allocate a skb to store the frags */ > >> - skb = napi_alloc_skb(&rx_ring->q_vector->napi, totalsize); > >> - if (unlikely(!skb)) > >> - goto out; > >> - > >> - memcpy(__skb_put(skb, totalsize), xdp->data_meta, > >> - ALIGN(totalsize, sizeof(long))); > >> - > >> - if (metasize) { > >> - skb_metadata_set(skb, metasize); > >> - __skb_pull(skb, metasize); > >> - } > >> - > >> - if (likely(!xdp_buff_has_frags(xdp))) > >> - goto out; > >> - > >> - for (int i = 0; i < nr_frags; i++) { > >> - struct skb_shared_info *skinfo = skb_shinfo(skb); > >> - skb_frag_t *frag = &sinfo->frags[i]; > >> - struct page *page; > >> - void *addr; > >> - > >> - page = dev_alloc_page(); > >> - if (!page) { > >> - dev_kfree_skb(skb); > >> - return NULL; > >> - } > >> - addr = page_to_virt(page); > >> - > >> - memcpy(addr, skb_frag_page(frag), skb_frag_size(frag)); > >> - > >> - __skb_fill_page_desc_noacc(skinfo, skinfo->nr_frags++, > >> - addr, 0, skb_frag_size(frag)); > >> - } > >> - > >> -out: > >> - xsk_buff_free(xdp); > >> - return skb; > >> -} > >> - > >> static void i40e_handle_xdp_result_zc(struct i40e_ring *rx_ring, > >> struct xdp_buff *xdp_buff, > >> union i40e_rx_desc *rx_desc, > >> @@ -372,14 +309,20 @@ static void i40e_handle_xdp_result_zc(struct i40e_ring *rx_ring, > >> * BIT(I40E_RXD_QW1_ERROR_SHIFT). This is due to that > >> * SBP is *not* set in PRT_SBPVSI (default not set). > >> */ > >> - skb = i40e_construct_skb_zc(rx_ring, xdp_buff); > >> + skb = xdp_build_skb_from_zc(xdp_buff); > >> if (!skb) { > >> + xsk_buff_free(xdp_buff); > >> rx_ring->rx_stats.alloc_buff_failed++; > >> *rx_packets = 0; > >> *rx_bytes = 0; > >> return; > >> } > >> > >> + /* xdp_build_skb_from_zc() already ran eth_type_trans(); > >> + * restore the header for eth_skb_pad()/process_skb_fields(). > >> + */ > >> + __skb_push(skb, skb->data - skb_mac_header(skb)); > >> + > >> if (eth_skb_pad(skb)) { > >> *rx_packets = 0; > >> *rx_bytes = 0; > >> -- > >> 2.25.1 > >> > >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v2] i40e: xsk: fix multi-buffer XDP_PASS skb construction 2026-07-23 7:59 ` Jason Xing @ 2026-07-23 10:04 ` Maciej Fijalkowski 0 siblings, 0 replies; 6+ messages in thread From: Maciej Fijalkowski @ 2026-07-23 10:04 UTC (permalink / raw) To: Jason Xing Cc: Chenguang Zhao, anthony.l.nguyen, przemyslaw.kitszel, andrew+netdev, davem, edumazet, kuba, pabeni, intel-wired-lan, netdev, Chenguang Zhao On Thu, Jul 23, 2026 at 03:59:44PM +0800, Jason Xing wrote: > On Thu, Jul 23, 2026 at 3:02 PM Chenguang Zhao <chenguang.zhao@linux.dev> wrote: > > > > Thanks for the clarification. > > Please do not top-post. > > > > > I agree: the minimal bugfix belongs in -net for stable backportability, and the conversion to xdp_build_skb_from_zc() belongs in -next. > > Right. That's what I meant. But I will let Maciej/Tony/Olek make the decision. Thanks Jason - agree with the logistics you are proposing. > > > > > Please apply the existing v1 bugfix to -net: > > > > https://lore.kernel.org/all/20260714025112.284724-1-chenguang.zhao@linux.dev/ > > If we eventually need this patch, then you will send a v3 patch > instead of asking maintainers to pick it up :) > > And feel free to add: > Reviewed-by: Jason Xing <kerneljasonxing@gmail.com> > > > > > It already has : > > > > Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> > > > > I will drop v2 as a Fixes candidate. > > > > In parallel, I will send a separate net-next series (as v1) that replaces > > > > i40e_construct_skb_zc() with xdp_build_skb_from_zc(), based on > > > > net-next. That patch will not carry a Fixes tag. > > As to this version, my tag still holds: > Reviewed-by: Jason Xing <kerneljasonxing@gmail.com> > > Thanks, > Jason > > > > > > > Thanks, > > > > Chenguang > > > > > > 在 2026/7/22 07:15, Jason Xing 写道: > > > On Fri, Jul 17, 2026 at 9:24 AM Chenguang Zhao <chenguang.zhao@linux.dev> wrote: > > >> From: Chenguang Zhao <zhaochenguang@kylinos.cn> > > >> > > >> When AF_XDP ZC receives a multi-buffer frame and XDP returns XDP_PASS, > > >> i40e_construct_skb_zc() copied frags incorrectly: memcpy used > > >> skb_frag_page() (page metadata) and __skb_fill_page_desc_noacc() was > > >> given a virtual address instead of a struct page *. > > >> > > >> Drop the custom helper and use xdp_build_skb_from_zc() instead. On > > >> failure, free the xdp buff in the caller. Push the Ethernet header > > >> back before eth_skb_pad()/i40e_process_skb_fields() because > > >> xdp_build_skb_from_zc() already called eth_type_trans(). > > >> > > >> Fixes: 1c9ba9c14658 ("i40e: xsk: add RX multi-buffer support") > > >> Signed-off-by: Chenguang Zhao <zhaochenguang@kylinos.cn> > > >> --- > > >> Revised as suggested by Maciej: > > >> - Replace i40e_construct_skb_zc() with xdp_build_skb_from_zc() > > > I might have a different opinion on this patch: yes, it actually > > > belongs to -next material. The process should be like: 1) fix the > > > issues by v1, 2) refactor it by v2. The reason behind that is the > > > helper was introduced in 2024 while the home-grown part was in 2023, > > > which means it doesn't help for stable steam to cherry-pick the patch > > > in older kernels like 6.6[1]. > > > > > > [1]: https://www.kernel.org/ > > > > > > Thanks, > > > Jason > > > > > >> v1: > > >> https://lore.kernel.org/all/20260714025112.284724-1-chenguang.zhao@linux.dev/ > > >> > > >> drivers/net/ethernet/intel/i40e/i40e_xsk.c | 73 +++------------------- > > >> 1 file changed, 8 insertions(+), 65 deletions(-) > > >> > > >> diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c > > >> index 9f47388eaba5..1319a5c22625 100644 > > >> --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c > > >> +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c > > >> @@ -3,6 +3,7 @@ > > >> > > >> #include <linux/bpf_trace.h> > > >> #include <linux/unroll.h> > > >> +#include <net/xdp.h> > > >> #include <net/xdp_sock_drv.h> > > >> #include "i40e_txrx_common.h" > > >> #include "i40e_xsk.h" > > >> @@ -277,70 +278,6 @@ bool i40e_alloc_rx_buffers_zc(struct i40e_ring *rx_ring, u16 count) > > >> return count == nb_buffs; > > >> } > > >> > > >> -/** > > >> - * i40e_construct_skb_zc - Create skbuff from zero-copy Rx buffer > > >> - * @rx_ring: Rx ring > > >> - * @xdp: xdp_buff > > >> - * > > >> - * This functions allocates a new skb from a zero-copy Rx buffer. > > >> - * > > >> - * Returns the skb, or NULL on failure. > > >> - **/ > > >> -static struct sk_buff *i40e_construct_skb_zc(struct i40e_ring *rx_ring, > > >> - struct xdp_buff *xdp) > > >> -{ > > >> - unsigned int totalsize = xdp->data_end - xdp->data_meta; > > >> - unsigned int metasize = xdp->data - xdp->data_meta; > > >> - struct skb_shared_info *sinfo = NULL; > > >> - struct sk_buff *skb; > > >> - u32 nr_frags = 0; > > >> - > > >> - if (unlikely(xdp_buff_has_frags(xdp))) { > > >> - sinfo = xdp_get_shared_info_from_buff(xdp); > > >> - nr_frags = sinfo->nr_frags; > > >> - } > > >> - net_prefetch(xdp->data_meta); > > >> - > > >> - /* allocate a skb to store the frags */ > > >> - skb = napi_alloc_skb(&rx_ring->q_vector->napi, totalsize); > > >> - if (unlikely(!skb)) > > >> - goto out; > > >> - > > >> - memcpy(__skb_put(skb, totalsize), xdp->data_meta, > > >> - ALIGN(totalsize, sizeof(long))); > > >> - > > >> - if (metasize) { > > >> - skb_metadata_set(skb, metasize); > > >> - __skb_pull(skb, metasize); > > >> - } > > >> - > > >> - if (likely(!xdp_buff_has_frags(xdp))) > > >> - goto out; > > >> - > > >> - for (int i = 0; i < nr_frags; i++) { > > >> - struct skb_shared_info *skinfo = skb_shinfo(skb); > > >> - skb_frag_t *frag = &sinfo->frags[i]; > > >> - struct page *page; > > >> - void *addr; > > >> - > > >> - page = dev_alloc_page(); > > >> - if (!page) { > > >> - dev_kfree_skb(skb); > > >> - return NULL; > > >> - } > > >> - addr = page_to_virt(page); > > >> - > > >> - memcpy(addr, skb_frag_page(frag), skb_frag_size(frag)); > > >> - > > >> - __skb_fill_page_desc_noacc(skinfo, skinfo->nr_frags++, > > >> - addr, 0, skb_frag_size(frag)); > > >> - } > > >> - > > >> -out: > > >> - xsk_buff_free(xdp); > > >> - return skb; > > >> -} > > >> - > > >> static void i40e_handle_xdp_result_zc(struct i40e_ring *rx_ring, > > >> struct xdp_buff *xdp_buff, > > >> union i40e_rx_desc *rx_desc, > > >> @@ -372,14 +309,20 @@ static void i40e_handle_xdp_result_zc(struct i40e_ring *rx_ring, > > >> * BIT(I40E_RXD_QW1_ERROR_SHIFT). This is due to that > > >> * SBP is *not* set in PRT_SBPVSI (default not set). > > >> */ > > >> - skb = i40e_construct_skb_zc(rx_ring, xdp_buff); > > >> + skb = xdp_build_skb_from_zc(xdp_buff); > > >> if (!skb) { > > >> + xsk_buff_free(xdp_buff); > > >> rx_ring->rx_stats.alloc_buff_failed++; > > >> *rx_packets = 0; > > >> *rx_bytes = 0; > > >> return; > > >> } > > >> > > >> + /* xdp_build_skb_from_zc() already ran eth_type_trans(); > > >> + * restore the header for eth_skb_pad()/process_skb_fields(). > > >> + */ > > >> + __skb_push(skb, skb->data - skb_mac_header(skb)); > > >> + > > >> if (eth_skb_pad(skb)) { > > >> *rx_packets = 0; > > >> *rx_bytes = 0; > > >> -- > > >> 2.25.1 > > >> > > >> > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-23 10:04 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-17 1:24 [Intel-wired-lan] [PATCH net v2] i40e: xsk: fix multi-buffer XDP_PASS skb construction Chenguang Zhao 2026-07-20 15:03 ` Alexander Lobakin 2026-07-21 23:15 ` Jason Xing 2026-07-23 7:02 ` Chenguang Zhao 2026-07-23 7:59 ` Jason Xing 2026-07-23 10:04 ` Maciej Fijalkowski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox