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

On Wed, Jul 29, 2026 at 01:49:16PM +0800, Chenguang Zhao wrote:
> From: Chenguang Zhao <[email protected]>
> 
> 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 <[email protected]>
> ---
> 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
> 
>