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
*/
