> -----Original Message-----
> From: Jakub Kicinski <k...@kernel.org>
> Sent: Wednesday, March 17, 2021 3:30 AM
> To: Bhaskar Upadhaya <bupadh...@marvell.com>
> Cc: netdev@vger.kernel.org; Ariel Elior <ael...@marvell.com>; Igor Russkikh
> <irussk...@marvell.com>; da...@davemloft.net
> Subject: [EXT] Re: [PATCH net 1/2] qede: fix to disable start_xmit
> functionality
> during self adapter test
>
> External Email
>
> ----------------------------------------------------------------------
> On Tue, 16 Mar 2021 11:34:09 -0700 Bhaskar Upadhaya wrote:
> > start_xmit function should not be called during the execution of self
> > adapter test, netif_tx_disable() gives this guarantee, since it takes
> > the transmit queue lock while marking the queue stopped. This will
> > wait for the transmit function to complete before returning.
> >
> > Fixes: 16f46bf054f8 ("qede: add implementation for internal loopback
> > test.")
> > Signed-off-by: Bhaskar Upadhaya <bupadh...@marvell.com>
> > Signed-off-by: Igor Russkikh <irussk...@marvell.com>
> > Signed-off-by: Ariel Elior <ael...@marvell.com>
> > ---
> > drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
> > b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
> > index 1560ad3d9290..f9702cc7bc55 100644
> > --- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
> > +++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
> > @@ -1611,7 +1611,7 @@ static int qede_selftest_run_loopback(struct
> qede_dev *edev, u32 loopback_mode)
> > return -EINVAL;
> > }
> >
> > - qede_netif_stop(edev);
> > + netif_tx_disable(edev->ndev);
>
> But an interrupt can come in after and enable Tx again.
> I think you should keep the qede_netif_stop() here instead of moving it
> down, no?
Hi Jakub,
Normal Traffic flow is enabled by qede_netif_start(edev) and which is placed at
the end of this qede_selftest_run_loopback()
qede_netif_stop(edev) is called prior to the call to qede_netif_start(edev), so
unless qede_netif_start(edev) is called Normal traffic flow will not
be operational.
>
> > /* Bring up the link in Loopback mode */
> > memset(&link_params, 0, sizeof(link_params)); @@ -1623,6 +1623,8
> @@
> > static int qede_selftest_run_loopback(struct qede_dev *edev, u32
> loopback_mode)
> > /* Wait for loopback configuration to apply */
> > msleep_interruptible(500);
> >
> > + qede_netif_stop(edev);
> > +
> > /* Setting max packet size to 1.5K to avoid data being split over
> > * multiple BDs in cases where MTU > PAGE_SIZE.
> > */
