diff mbox series

[net] net: stmmac: Disable Tx queues when reconfiguring the interface

Message ID 20211123185448.335924-1-yannick.vignon@oss.nxp.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net] net: stmmac: Disable Tx queues when reconfiguring the interface | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 7 maintainers not CCed: bpf@vger.kernel.org daniel@iogearbox.net linux-stm32@st-md-mailman.stormreply.com linux-arm-kernel@lists.infradead.org hawk@kernel.org john.fastabend@gmail.com ast@kernel.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes fail Problems with Fixes tag: 1
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: Unknown commit id '78cb988d36b6', maybe rebased or not pulled?
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Yannick Vignon Nov. 23, 2021, 6:54 p.m. UTC 
From: Yannick Vignon <yannick.vignon@nxp.com>

The Tx queues were not disabled in cases where the driver needed to stop
the interface to apply a new configuration. This could result in a kernel
panic when doing any of the 3 following actions:
* reconfiguring the number of queues (ethtool -L)
* reconfiguring the size of the ring buffers (ethtool -G)
* installing/removing an XDP program (ip l set dev ethX xdp)

Prevent the panic by making sure netif_tx_disable is called when stopping
an interface.

Without this patch, the following kernel panic can be observed when loading
an XDP program:

Unable to handle kernel paging request at virtual address ffff80001238d040
[....]
 Call trace:
  dwmac4_set_addr+0x8/0x10
  dev_hard_start_xmit+0xe4/0x1ac
  sch_direct_xmit+0xe8/0x39c
  __dev_queue_xmit+0x3ec/0xaf0
  dev_queue_xmit+0x14/0x20
[...]
[ end trace 0000000000000002 ]---

Fixes: 78cb988d36b6 ("net: stmmac: Add initial XDP support")
Signed-off-by: Yannick Vignon <yannick.vignon@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Jakub Kicinski Nov. 24, 2021, 4:07 a.m. UTC | #1 
On Tue, 23 Nov 2021 19:54:48 +0100 Yannick Vignon wrote:
> From: Yannick Vignon <yannick.vignon@nxp.com>
> 
> The Tx queues were not disabled in cases where the driver needed to stop
> the interface to apply a new configuration. This could result in a kernel
> panic when doing any of the 3 following actions:
> * reconfiguring the number of queues (ethtool -L)
> * reconfiguring the size of the ring buffers (ethtool -G)
> * installing/removing an XDP program (ip l set dev ethX xdp)
> 
> Prevent the panic by making sure netif_tx_disable is called when stopping
> an interface.
> 
> Without this patch, the following kernel panic can be observed when loading
> an XDP program:
> 
> Unable to handle kernel paging request at virtual address ffff80001238d040
> [....]
>  Call trace:
>   dwmac4_set_addr+0x8/0x10
>   dev_hard_start_xmit+0xe4/0x1ac
>   sch_direct_xmit+0xe8/0x39c
>   __dev_queue_xmit+0x3ec/0xaf0
>   dev_queue_xmit+0x14/0x20
> [...]
> [ end trace 0000000000000002 ]---
> 
> Fixes: 78cb988d36b6 ("net: stmmac: Add initial XDP support")
> Signed-off-by: Yannick Vignon <yannick.vignon@nxp.com>

Fixes tag: Fixes: 78cb988d36b6 ("net: stmmac: Add initial XDP support")
Has these problem(s):
	- Target SHA1 does not exist
Vladimir Oltean Nov. 24, 2021, 10:06 a.m. UTC | #2 
On Tue, Nov 23, 2021 at 08:07:51PM -0800, Jakub Kicinski wrote:
> On Tue, 23 Nov 2021 19:54:48 +0100 Yannick Vignon wrote:
> > From: Yannick Vignon <yannick.vignon@nxp.com>
> > 
> > The Tx queues were not disabled in cases where the driver needed to stop
> > the interface to apply a new configuration. This could result in a kernel
> > panic when doing any of the 3 following actions:
> > * reconfiguring the number of queues (ethtool -L)
> > * reconfiguring the size of the ring buffers (ethtool -G)
> > * installing/removing an XDP program (ip l set dev ethX xdp)
> > 
> > Prevent the panic by making sure netif_tx_disable is called when stopping
> > an interface.
> > 
> > Without this patch, the following kernel panic can be observed when loading
> > an XDP program:
> > 
> > Unable to handle kernel paging request at virtual address ffff80001238d040
> > [....]
> >  Call trace:
> >   dwmac4_set_addr+0x8/0x10
> >   dev_hard_start_xmit+0xe4/0x1ac
> >   sch_direct_xmit+0xe8/0x39c
> >   __dev_queue_xmit+0x3ec/0xaf0
> >   dev_queue_xmit+0x14/0x20
> > [...]
> > [ end trace 0000000000000002 ]---
> > 
> > Fixes: 78cb988d36b6 ("net: stmmac: Add initial XDP support")
> > Signed-off-by: Yannick Vignon <yannick.vignon@nxp.com>
> 
> Fixes tag: Fixes: 78cb988d36b6 ("net: stmmac: Add initial XDP support")
> Has these problem(s):
> 	- Target SHA1 does not exist

You caught him backporting! Although I agree, things sent to the "net"
tree should also be tested against the "net" tree.
Yannick Vignon Nov. 24, 2021, 12:10 p.m. UTC | #3 
On 11/24/2021 11:06 AM, Vladimir Oltean wrote:
> On Tue, Nov 23, 2021 at 08:07:51PM -0800, Jakub Kicinski wrote:
>> On Tue, 23 Nov 2021 19:54:48 +0100 Yannick Vignon wrote:
>>> From: Yannick Vignon <yannick.vignon@nxp.com>
>>>
>>> The Tx queues were not disabled in cases where the driver needed to stop
>>> the interface to apply a new configuration. This could result in a kernel
>>> panic when doing any of the 3 following actions:
>>> * reconfiguring the number of queues (ethtool -L)
>>> * reconfiguring the size of the ring buffers (ethtool -G)
>>> * installing/removing an XDP program (ip l set dev ethX xdp)
>>>
>>> Prevent the panic by making sure netif_tx_disable is called when stopping
>>> an interface.
>>>
>>> Without this patch, the following kernel panic can be observed when loading
>>> an XDP program:
>>>
>>> Unable to handle kernel paging request at virtual address ffff80001238d040
>>> [....]
>>>   Call trace:
>>>    dwmac4_set_addr+0x8/0x10
>>>    dev_hard_start_xmit+0xe4/0x1ac
>>>    sch_direct_xmit+0xe8/0x39c
>>>    __dev_queue_xmit+0x3ec/0xaf0
>>>    dev_queue_xmit+0x14/0x20
>>> [...]
>>> [ end trace 0000000000000002 ]---
>>>
>>> Fixes: 78cb988d36b6 ("net: stmmac: Add initial XDP support")
>>> Signed-off-by: Yannick Vignon <yannick.vignon@nxp.com>
>>
>> Fixes tag: Fixes: 78cb988d36b6 ("net: stmmac: Add initial XDP support")
>> Has these problem(s):
>> 	- Target SHA1 does not exist
> 
> You caught him backporting! Although I agree, things sent to the "net"
> tree should also be tested against the "net" tree.
> 

That would be more like forward-porting in this case, since I first 
fixed the issue on an older 5.10 kernel :)
And yes, I did reproduce the bug and confirm the fix on the tip of the 
netdev branch yesterday before sending the patch. But I guess I looked 
at the wrong branch when adding the Fixes tag...

Will post a V2 to fix the Fixes tag.
Vladimir Oltean Nov. 24, 2021, 12:39 p.m. UTC | #4 
On Wed, Nov 24, 2021 at 01:10:55PM +0100, Yannick Vignon wrote:
> On 11/24/2021 11:06 AM, Vladimir Oltean wrote:
> > On Tue, Nov 23, 2021 at 08:07:51PM -0800, Jakub Kicinski wrote:
> > > On Tue, 23 Nov 2021 19:54:48 +0100 Yannick Vignon wrote:
> > > > From: Yannick Vignon <yannick.vignon@nxp.com>
> > > > 
> > > > The Tx queues were not disabled in cases where the driver needed to stop
> > > > the interface to apply a new configuration. This could result in a kernel
> > > > panic when doing any of the 3 following actions:
> > > > * reconfiguring the number of queues (ethtool -L)
> > > > * reconfiguring the size of the ring buffers (ethtool -G)
> > > > * installing/removing an XDP program (ip l set dev ethX xdp)
> > > > 
> > > > Prevent the panic by making sure netif_tx_disable is called when stopping
> > > > an interface.
> > > > 
> > > > Without this patch, the following kernel panic can be observed when loading
> > > > an XDP program:
> > > > 
> > > > Unable to handle kernel paging request at virtual address ffff80001238d040
> > > > [....]
> > > >   Call trace:
> > > >    dwmac4_set_addr+0x8/0x10
> > > >    dev_hard_start_xmit+0xe4/0x1ac
> > > >    sch_direct_xmit+0xe8/0x39c
> > > >    __dev_queue_xmit+0x3ec/0xaf0
> > > >    dev_queue_xmit+0x14/0x20
> > > > [...]
> > > > [ end trace 0000000000000002 ]---
> > > > 
> > > > Fixes: 78cb988d36b6 ("net: stmmac: Add initial XDP support")
> > > > Signed-off-by: Yannick Vignon <yannick.vignon@nxp.com>
> > > 
> > > Fixes tag: Fixes: 78cb988d36b6 ("net: stmmac: Add initial XDP support")
> > > Has these problem(s):
> > > 	- Target SHA1 does not exist
> > 
> > You caught him backporting! Although I agree, things sent to the "net"
> > tree should also be tested against the "net" tree.
> > 
> 
> That would be more like forward-porting in this case, since I first fixed
> the issue on an older 5.10 kernel :)

XDP for stmmac isn't in v5.10, so backporting is what it is.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f12097c8a485..748195697e5a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3802,6 +3802,8 @@  int stmmac_release(struct net_device *dev)
 	struct stmmac_priv *priv = netdev_priv(dev);
 	u32 chan;
 
+	netif_tx_disable(dev);
+
 	if (device_may_wakeup(priv->device))
 		phylink_speed_down(priv->phylink, false);
 	/* Stop and disconnect the PHY */