Re: [Intel-wired-lan] [PATCH net-next v2] i40e: xsk: use xdp_build_skb_from_zc() for XDP_PASS

2026-07-30 Thread Larysa Zaremba 
On Wed, Jul 29, 2026 at 01:49:16PM +0800, Chenguang Zhao wrote:
> From: Chenguang Zhao 
> 
> Replace the driver-local i40e_construct_skb_zc() with the common
> helper xdp_build_skb_from_zc(). On failure, free the xdp buff in
> the caller.
> 
> xdp_build_skb_from_zc() already calls skb_record_rx_queue() and
> eth_type_trans(), so pull the remaining descriptor field setup into
> __i40e_process_skb_fields() and use that on the XDP_PASS path.
> Briefly restore the Ethernet header around eth_skb_pad() so padding
> sees the full L2 frame length.
> 
> Signed-off-by: Chenguang Zhao 
> ---
> v2:
> - Extract descriptor field setup into __i40e_process_skb_fields() and
>   use it on the XDP_PASS path, so skb_record_rx_queue()/eth_type_trans()
>   are not repeated after xdp_build_skb_from_zc().
> - Keep a brief __skb_push()/__skb_pull() around eth_skb_pad() so padding
>   still sees the full L2 frame length.
> 
> v1:
>  - 
> https://lore.kernel.org/all/[email protected]/
> ---
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 32 ++--
>  .../ethernet/intel/i40e/i40e_txrx_common.h|  2 +
>  drivers/net/ethernet/intel/i40e/i40e_xsk.c| 76 +++
>  3 files changed, 36 insertions(+), 74 deletions(-)
> 

[...]

>  static void i40e_handle_xdp_result_zc(struct i40e_ring *rx_ring,
> struct xdp_buff *xdp_buff,
> union i40e_rx_desc *rx_desc,
> @@ -372,21 +309,28 @@ 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() and
> +  * skb_record_rx_queue(). Restore the Ethernet header only for
> +  * eth_skb_pad(), then pull it back.
> +  */
> + __skb_push(skb, ETH_HLEN);
>   if (eth_skb_pad(skb)) {
>   *rx_packets = 0;
>   *rx_bytes = 0;
>   return;
>   }
> + __skb_pull(skb, ETH_HLEN);

I am pretty sure that eth_skb_pad() is not needed on Rx, network stack handles 
small skbs just fine, and no other vendor does this. We were trying to figure 
out the historical reasons for eth_skb_pad() being there in the intel drivers, 
but with no luck.

>  
> - i40e_process_skb_fields(rx_ring, rx_desc, skb);
> + __i40e_process_skb_fields(rx_ring, rx_desc, skb);
>   napi_gro_receive(&rx_ring->q_vector->napi, skb);
>   return;
>   }
> -- 
> 2.25.1
> 
>

[Intel-wired-lan] [PATCH net-next v2] i40e: xsk: use xdp_build_skb_from_zc() for XDP_PASS

2026-07-28 Thread Chenguang Zhao 
From: Chenguang Zhao 

Replace the driver-local i40e_construct_skb_zc() with the common
helper xdp_build_skb_from_zc(). On failure, free the xdp buff in
the caller.

xdp_build_skb_from_zc() already calls skb_record_rx_queue() and
eth_type_trans(), so pull the remaining descriptor field setup into
__i40e_process_skb_fields() and use that on the XDP_PASS path.
Briefly restore the Ethernet header around eth_skb_pad() so padding
sees the full L2 frame length.

Signed-off-by: Chenguang Zhao 
---
v2:
- Extract descriptor field setup into __i40e_process_skb_fields() and
  use it on the XDP_PASS path, so skb_record_rx_queue()/eth_type_trans()
  are not repeated after xdp_build_skb_from_zc().
- Keep a brief __skb_push()/__skb_pull() around eth_skb_pad() so padding
  still sees the full L2 frame length.

v1:
 - https://lore.kernel.org/all/[email protected]/
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 32 ++--
 .../ethernet/intel/i40e/i40e_txrx_common.h|  2 +
 drivers/net/ethernet/intel/i40e/i40e_xsk.c| 76 +++
 3 files changed, 36 insertions(+), 74 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c 
b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index ef5e657816f0..87553b14d34a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1833,17 +1833,17 @@ static inline void i40e_rx_hash(struct i40e_ring *ring,
 }
 
 /**
- * i40e_process_skb_fields - Populate skb header fields from Rx descriptor
+ * __i40e_process_skb_fields - Populate skb fields from Rx descriptor
  * @rx_ring: rx descriptor ring packet is being transacted on
  * @rx_desc: pointer to the EOP Rx descriptor
  * @skb: pointer to current skb being populated
  *
- * This function checks the ring, descriptor, and packet information in
- * order to populate the hash, checksum, VLAN, protocol, and
- * other fields within the skb.
+ * Populate hash, checksum, PTP timestamp and VLAN from @rx_desc. Does not
+ * call skb_record_rx_queue() or eth_type_trans(); callers that already got
+ * those from xdp_build_skb_from_zc() should use this helper.
  **/
-void i40e_process_skb_fields(struct i40e_ring *rx_ring,
-union i40e_rx_desc *rx_desc, struct sk_buff *skb)
+void __i40e_process_skb_fields(struct i40e_ring *rx_ring,
+  union i40e_rx_desc *rx_desc, struct sk_buff *skb)
 {
u64 qword = le64_to_cpu(rx_desc->wb.qword1.status_error_len);
u32 rx_status = FIELD_GET(I40E_RXD_QW1_STATUS_MASK, qword);
@@ -1858,14 +1858,30 @@ void i40e_process_skb_fields(struct i40e_ring *rx_ring,
 
i40e_rx_checksum(rx_ring->vsi, skb, rx_desc);
 
-   skb_record_rx_queue(skb, rx_ring->queue_index);
-
if (qword & BIT(I40E_RX_DESC_STATUS_L2TAG1P_SHIFT)) {
__le16 vlan_tag = rx_desc->wb.qword0.lo_dword.l2tag1;
 
__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
   le16_to_cpu(vlan_tag));
}
+}
+
+/**
+ * i40e_process_skb_fields - Populate skb header fields from Rx descriptor
+ * @rx_ring: rx descriptor ring packet is being transacted on
+ * @rx_desc: pointer to the EOP Rx descriptor
+ * @skb: pointer to current skb being populated
+ *
+ * This function checks the ring, descriptor, and packet information in
+ * order to populate the hash, checksum, VLAN, protocol, and
+ * other fields within the skb.
+ **/
+void i40e_process_skb_fields(struct i40e_ring *rx_ring,
+union i40e_rx_desc *rx_desc, struct sk_buff *skb)
+{
+   __i40e_process_skb_fields(rx_ring, rx_desc, skb);
+
+   skb_record_rx_queue(skb, rx_ring->queue_index);
 
/* modifies the skb - consumes the enet header */
skb->protocol = eth_type_trans(skb, rx_ring->netdev);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx_common.h 
b/drivers/net/ethernet/intel/i40e/i40e_txrx_common.h
index e26807fd2123..3c52e0e60487 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx_common.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx_common.h
@@ -9,6 +9,8 @@
 int i40e_xmit_xdp_tx_ring(struct xdp_buff *xdp, struct i40e_ring *xdp_ring);
 void i40e_clean_programming_status(struct i40e_ring *rx_ring, u64 qword0_raw,
   u64 qword1);
+void __i40e_process_skb_fields(struct i40e_ring *rx_ring,
+  union i40e_rx_desc *rx_desc, struct sk_buff 
*skb);
 void i40e_process_skb_fields(struct i40e_ring *rx_ring,
 union i40e_rx_desc *rx_desc, struct sk_buff *skb);
 void i40e_xdp_ring_update_tail(struct i40e_ring *xdp_ring);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c 
b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 9f47388eaba5..8741dff82bd1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -3,6 +3,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include