[PATCH] ucc_geth: Reset BQL queue when stopping device

[Li Yang]

On Mon, Jan 28, 2019 at 8:37 AM Mathias Thore
<Mathias.Thore at infinera.com> wrote:
>
> Hi,
>
>
> This is what we observed: there was a storm on the medium so that our controller could not do its TX, resulting in timeout. When timeout occurs, the driver clears all descriptors from the TX queue. The function called in this patch is used to reflect this clearing also in the BQL layer. Without it, the controller would get stuck, unable to perform TX, even several minutes after the storm had ended. Bringing the device down and then up again would solve the problem, but this patch also solves it automatically.

The explanation makes sense.  So this should only be required in the
timeout scenario instead of other clean up scenarios like device
shutdown?  If so, it probably it will be better to be done in
ucc_geth_timeout_work()?

>
>
> Some other drivers do the same, for example e1000e driver calls netdev_reset_queue in its e1000_clean_tx_ring function. It is possible that other drivers should do the same; I have no way of verifying this.
>
>
> Regards,
>
> Mathias
>
> --
>
>
> From: Christophe Leroy <christophe.leroy at c-s.fr>
> Sent: Monday, January 28, 2019 10:48 AM
> To: Mathias Thore; leoyang.li at nxp.com; netdev at vger.kernel.org; linuxppc-dev at lists.ozlabs.org; David Gounaris; Joakim Tjernlund
> Subject: Re: [PATCH] ucc_geth: Reset BQL queue when stopping device
>
>
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> Hi,
>
> Le 28/01/2019 à 10:07, Mathias Thore a écrit :
> > After a timeout event caused by for example a broadcast storm, when
> > the MAC and PHY are reset, the BQL TX queue needs to be reset as
> > well. Otherwise, the device will exhibit severe performance issues
> > even after the storm has ended.
>
> What are the symptomns ?
>
> Is this reset needed on any network driver in that case, or is it
> something particular for the ucc_geth ?
> For instance, the freescale fs_enet doesn't have that reset. Should it
> have it too ?
>
> Christophe
>
> >
> > Co-authored-by: David Gounaris <david.gounaris at infinera.com>
> > Signed-off-by: Mathias Thore <mathias.thore at infinera.com>
> > ---
> >   drivers/net/ethernet/freescale/ucc_geth.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
> > index c3d539e209ed..eb3e65e8868f 100644
> > --- a/drivers/net/ethernet/freescale/ucc_geth.c
> > +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> > @@ -1879,6 +1879,8 @@ static void ucc_geth_free_tx(struct ucc_geth_private *ugeth)
> >       u16 i, j;
> >       u8 __iomem *bd;
> >
> > +     netdev_reset_queue(ugeth->ndev);
> > +
> >       ug_info = ugeth->ug_info;
> >       uf_info = &ug_info->uf_info;
> >
> >
>