Re: [PATCH net-next 3/3] net/mlx5e: SHAMPO, Switch to header memcpy

[Paolo Abeni <pabeni@redhat.com>]

On 1/12/26 2:22 PM, Tariq Toukan wrote:
> @@ -1292,15 +1065,41 @@ static void mlx5e_shampo_update_ipv6_udp_hdr(struct mlx5e_rq *rq, struct ipv6hdr
>  	skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_L4;
>  }
>  
> +static inline u32 mlx5e_shampo_get_header_offset(int header_index)

Is the above really needed to get this function inlined? Perhaps you
could use a macro insted of a function? 'static inline' in c files
should be avoided.

> +{
> +	return (header_index & (MLX5E_SHAMPO_WQ_HEADER_PER_PAGE - 1)) *
> +	       BIT(MLX5E_SHAMPO_LOG_HEADER_ENTRY_SIZE);
> +}
> +
> +static void *mlx5e_shampo_get_hdr(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe,
> +				  int len)
> +{
> +	struct mlx5e_shampo_hd *shampo = rq->mpwqe.shampo;
> +	u32 head_offset, header_index, di_index;
> +	struct mlx5e_dma_info *di;
> +
> +	header_index = mlx5e_shampo_get_cqe_header_index(rq, cqe);
> +	head_offset = mlx5e_shampo_get_header_offset(header_index);
> +	di_index = header_index >> MLX5E_SHAMPO_LOG_WQ_HEADER_PER_PAGE;
> +	di = &shampo->hd_buf_pages[di_index];
> +
> +	dma_sync_single_range_for_cpu(rq->pdev, di->addr, head_offset,
> +				      len, rq->buff.map_dir);
> +
> +	return page_address(di->page) + head_offset;
> +}
> +
>  static void mlx5e_shampo_update_fin_psh_flags(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe,
>  					      struct tcphdr *skb_tcp_hd)
>  {
> -	u16 header_index = mlx5e_shampo_get_cqe_header_index(rq, cqe);
> +	int nhoff = ETH_HLEN + rq->hw_gro_data->fk.control.thoff;
> +	int len = nhoff + sizeof(struct tcphdr);
>  	struct tcphdr *last_tcp_hd;
>  	void *last_hd_addr;
>  
> -	last_hd_addr = mlx5e_shampo_get_packet_hd(rq, header_index);
> -	last_tcp_hd =  last_hd_addr + ETH_HLEN + rq->hw_gro_data->fk.control.thoff;
> +	last_hd_addr = mlx5e_shampo_get_hdr(rq, cqe, len);
> +	last_tcp_hd = (struct tcphdr *)(last_hd_addr + nhoff);
> +
>  	tcp_flag_word(skb_tcp_hd) |= tcp_flag_word(last_tcp_hd) & (TCP_FLAG_FIN | TCP_FLAG_PSH);
>  }
>  
> @@ -2299,52 +2098,29 @@ static struct sk_buff *
>  mlx5e_skb_from_cqe_shampo(struct mlx5e_rq *rq, struct mlx5e_mpw_info *wi,
>  			  struct mlx5_cqe64 *cqe, u16 header_index)
>  {
> -	struct mlx5e_frag_page *frag_page = mlx5e_shampo_hd_to_frag_page(rq, header_index);
> -	u16 head_offset = mlx5e_shampo_hd_offset(rq, header_index);
>  	struct mlx5e_shampo_hd *shampo = rq->mpwqe.shampo;
>  	u16 head_size = cqe->shampo.header_size;
> -	u16 rx_headroom = rq->buff.headroom;
> -	struct sk_buff *skb = NULL;
> -	dma_addr_t page_dma_addr;
> -	dma_addr_t dma_addr;
> -	void *hdr, *data;
> -	u32 frag_size;
> -
> -	page_dma_addr = page_pool_get_dma_addr_netmem(frag_page->netmem);
> -	dma_addr = page_dma_addr + head_offset;
> +	struct mlx5e_dma_info *di;
> +	u32 head_offset, di_index;
> +	struct sk_buff *skb;
> +	int len;
>  
> -	hdr		= netmem_address(frag_page->netmem) + head_offset;
> -	data		= hdr + rx_headroom;
> -	frag_size	= MLX5_SKB_FRAG_SZ(rx_headroom + head_size);
> +	len = ALIGN(head_size, sizeof(long));
> +	skb = napi_alloc_skb(rq->cq.napi, len);
> +	if (unlikely(!skb)) {
> +		rq->stats->buff_alloc_err++;
> +		return NULL;
> +	}
>  
> -	if (likely(frag_size <= BIT(shampo->log_hd_entry_size))) {
> -		/* build SKB around header */
> -		dma_sync_single_range_for_cpu(rq->pdev, dma_addr, 0, frag_size, rq->buff.map_dir);
> -		net_prefetchw(hdr);
> -		net_prefetch(data);
> -		skb = mlx5e_build_linear_skb(rq, hdr, frag_size, rx_headroom, head_size, 0);
> -		if (unlikely(!skb))
> -			return NULL;
> +	net_prefetchw(skb->data);
>  
> -		frag_page->frags++;
> -	} else {
> -		/* allocate SKB and copy header for large header */
> -		rq->stats->gro_large_hds++;
> -		skb = napi_alloc_skb(rq->cq.napi,
> -				     ALIGN(head_size, sizeof(long)));
> -		if (unlikely(!skb)) {
> -			rq->stats->buff_alloc_err++;
> -			return NULL;
> -		}
> +	head_offset = mlx5e_shampo_get_header_offset(header_index);
> +	di_index = header_index >> MLX5E_SHAMPO_LOG_WQ_HEADER_PER_PAGE;
> +	di = &shampo->hd_buf_pages[di_index];

The above 3 statement are repeated verbatim in mlx5e_shampo_get_hdr();
perhaps you could factor out a common helper.

/P