Re: [PATCH net-next 5/5] net/mlx5e: XDP, Use page fragments for linear data in multibuf-mode

[Dragos Tatulea]

On 24.03.26 03:42, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> 
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> [...]
> 
>> @@ -702,6 +731,16 @@ static void mlx5e_dealloc_rx_mpwqe(struct mlx5e_rq *rq, u16 ix)
>>  	bitmap_fill(wi->skip_release_bitmap, rq->mpwqe.pages_per_wqe);
>>  }
>>
>> +void mlx5e_mpwqe_dealloc_linear_page(struct mlx5e_rq *rq)
>> +{
>> +	struct mlx5e_mpw_linear_info *li = rq->mpwqe.linear_info;
>> +
>> +	if (!li || !li->frag_page.netmem)
>> +		return;
>> +
>> +	mlx5e_page_release_fragmented(rq->page_pool, &li->frag_page);
>> +}
> 
> Does this function need to reset li->frag_page.netmem to 0 and
> li->frag_page.frags to li->max_frags after releasing the page?
> 
> When an RQ is flushed and then reactivated (for example, in error recovery
> via mlx5e_rx_reporter_err_rq_cqe_recover()), the following sequence occurs:
> 
> mlx5e_rx_reporter_err_rq_cqe_recover()
>   mlx5e_flush_rq()
>     mlx5e_free_rx_descs()
>       mlx5e_mpwqe_dealloc_linear_page()
>         mlx5e_page_release_fragmented()  [releases the page]
>         [but doesn't reset netmem or frags]
>   mlx5e_activate_rq()
>     [RQ becomes active again]
> 
> When the next XDP multi-buffer packet arrives, mlx5e_mpwqe_get_linear_page_frag()
> is called:
> 
> mlx5e_skb_from_cqe_mpwrq_nonlinear()
>   mlx5e_mpwqe_get_linear_page_frag()
>     mlx5e_mpwqe_linear_page_refill()
>       if (likely(li->frag_page.frags < li->max_frags))
>         return 0;  [skips allocation if frags < max_frags]
>     netmem_address(li->frag_page.netmem) + frag_offset  [UAF]
> 
> If li->frag_page.frags was not reset to li->max_frags in
> mlx5e_mpwqe_dealloc_linear_page(), the refill function will see frags <
> max_frags and skip the allocation, then compute the address using the stale
> netmem pointer that was already returned to the page pool.
> 
> Shouldn't this match the initialization in mlx5e_rq_alloc_mpwqe_linear_info()
> which sets li->frag_page.frags = li->max_frags?
Good catch. Will address in v2.

Thanks,
Dragos