Re: [net RFC] net/mlx4_en: Use frag stride in crossing page boundary condition

[Eric Dumazet <eric.dumazet@gmail.com>]

On 06/13/2018 05:53 PM, Saeed Mahameed wrote:
> When a new rx packet arrives, the rx path will decide whether to reuse
> the same page or not according to the current rx frag page offset and
> frag size, i.e:
> release = frags->page_offset + frag_info->frag_size > PAGE_SIZE;
> 
> Martin debugged this and he showed that this can cause mlx4 XDP to reuse
> buffers when it shouldn't.
> 
> Using frag_info->frag_size is wrong since frag size is always the port
> mtu and the frag stride might be larger, especially in XDP case where
> frag_stride == PAGE_SIZE.

Hmmm... why frag_size is not PAGE_SIZE for XDP then ?

This patch is a bit strange, since we do :

u32 sz_align = ALIGN(frag_size, SMP_CACHE_BYTES);

frags->page_offset += sz_align;
release = frags->page_offset + frag_info->frag_size > PAGE_SIZE;

So the @release condition is really to have enough space from frags->page_offset
to hold up to frag_info->frag_size bytes.

(NIC wont push more than frag_info->frag_size bytes, regardless of how big is out frag_stride )

Your patch would prevent a page being reused if say the amount of available bytes is 1536,
but frag_stride = 2048


I would suggest a patch like the following instead.

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 5edd0cf2999cbde37b3404aafd700584696af940..83d6b17b102bc9a22bfd8e68863d079f38670501 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -1082,7 +1082,7 @@ void mlx4_en_calc_rx_buf(struct net_device *dev)
         * This only works when num_frags == 1.
         */
        if (priv->tx_ring_num[TX_XDP]) {
-               priv->frag_info[0].frag_size = eff_mtu;
+               priv->frag_info[0].frag_size = PAGE_SIZE;
                /* This will gain efficient xdp frame recycling at the
                 * expense of more costly truesize accounting
                 */