Re: [PATCH] net/mlx5: fix Tx doorbell

[Adrien Mazarguil <adrien.mazarguil@6wind.com>]

On Thu, Jan 05, 2017 at 05:01:27PM +0000, Ferruh Yigit wrote:
> On 1/5/2017 4:52 PM, Adrien Mazarguil wrote:
> > On Thu, Jan 05, 2017 at 05:32:26PM +0100, Adrien Mazarguil wrote:
> >> Hi Ferruh,
> >>
> >> On Thu, Jan 05, 2017 at 03:19:35PM +0000, Ferruh Yigit wrote:
> >>> On 12/9/2016 1:27 PM, Nelio Laranjeiro wrote:
> >>>> Too much data is uselessly written to the Tx doorbell.
> >>>>
> >>>> Fixes: 1d88ba171942 ("net/mlx5: refactor Tx data path")
> >>>>
> >>>> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> >>>> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> >>>>
> >>>
> >>> Applied to dpdk-next-net/master, thanks.
> >>>
> >>> Is not CC'ing stable intentional, since this patch depends on a patch
> >>> introduced in this release? If not intentional, please CC stable.
> >>
> >> I intended to update the commit message for this patch as in the meantime we
> >> discovered it addresses a significant regression introduced in v16.11.
> >>
> >> CC'ing stable now.
> >>
> >> If possible, can you amend the commit log with:
> >>
> >> ---
> >>
> >> net/mlx5: fix Tx doorbell
> >>
> >> Too much data is uselessly written to the Tx doorbell, which since v16.11
> >> may also cause Tx queues to behave erratically and crash applications.
> >>
> >> This regression was seen on VF devices when the BlueFlame buffer size is
> >> zero (txq->cqe_n == 0) due to the following change:
> >>
> >>  -       cqe = &(*txq->cqes)[ci & (txq->cqe_n - 1)].cqe64;
> >>  +       cqe = &(*txq->cqes)[ci & ((1 << txq->cqe_n) - 1)];
> >>
> >> Fixes: 1d88ba171942 ("net/mlx5: refactor Tx data path")
> >> Fixes: e2f116ee3cac ("net/mlx5: reduce memory overhead for CQE handling")
> >>
> >> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> >> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> >> Cc: stable@dpdk.org
> >>
> >> ---
> > 
> > I mixed the commit that introduced the regression with a similar looking yet
> > harmless one, here is the proper message to use, sorry for the noise:
> > 
> > ---
> > 
> > net/mlx5: fix Tx doorbell
> > 
> > Too much data is uselessly written to the Tx doorbell, which since v16.11
> > may also cause Tx queues to behave erratically and crash applications.
> > 
> > This regression was seen on VF devices when the BlueFlame buffer size is
> > zero (txq->bf_buf_size) due to the following change:
> > 
> >  -       txq->bf_offset ^= txq->bf_buf_size;
> >  +       txq->bf_offset ^= (1 << txq->bf_buf_size);
> > 
> > Fixes: 1d88ba171942 ("net/mlx5: refactor Tx data path")
> > Fixes: d5793daefec8 ("net/mlx5: reduce memory overhead for BF handling")
> > 
> > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > Cc: stable@dpdk.org
> > 
> > ---
> > 
> 
> Can you please confirm commit in latest next-net?

Not sure if the "Cc: stable@dpdk.org" line should have been kept, otherwise
it's perfect, thanks Ferruh!

-- 
Adrien Mazarguil
6WIND