[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z-GDBlDsnPyc21RM@mini-arch>
Date: Mon, 24 Mar 2025 09:06:30 -0700
From: Stanislav Fomichev <stfomichev@...il.com>
To: Cosmin Ratiu <cratiu@...dia.com>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"sdf@...ichev.me" <sdf@...ichev.me>,
"saeed@...nel.org" <saeed@...nel.org>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"davem@...emloft.net" <davem@...emloft.net>,
"kuba@...nel.org" <kuba@...nel.org>,
"pabeni@...hat.com" <pabeni@...hat.com>
Subject: Re: [PATCH net-next v10 08/14] net: hold netdev instance lock during
sysfs operations
On 03/24, Cosmin Ratiu wrote:
> On Wed, 2025-03-05 at 08:37 -0800, Stanislav Fomichev wrote:
> > diff --git a/net/core/dev_api.c b/net/core/dev_api.c
> > index 059413d9ef9d..7bd667b34b80 100644
> > --- a/net/core/dev_api.c
> > +++ b/net/core/dev_api.c
> > +
> > +/**
> > + * dev_disable_lro() - disable Large Receive Offload on a device
> > + * @dev: device
> > + *
> > + * Disable Large Receive Offload (LRO) on a net device. Must be
> > + * called under RTNL. This is needed if received packets may be
> > + * forwarded to another interface.
> > + */
> > +void dev_disable_lro(struct net_device *dev)
> > +{
> > + netdev_lock_ops(dev);
> > + netif_disable_lro(dev);
> > + netdev_unlock_ops(dev);
> > +}
>
> It seems this part plus the following part from patch 6 of this series
> result in a recursive deadlock when inet forwarding is not enabled:
>
> > @@ -3013,6 +3021,8 @@ static int do_setlink(const struct sk_buff
> > *skb, struct net_device *dev,
> > char ifname[IFNAMSIZ];
> > int err;
> >
> > + netdev_lock_ops(dev);
> > +
> > err = validate_linkmsg(dev, tb, extack);
> > if (err < 0)
> > goto errout;
> >
>
> Call Trace:
> dump_stack_lvl+0x62/0x90
> print_deadlock_bug+0x274/0x3b0
> __lock_acquire+0x1229/0x2470
> lock_acquire+0xb7/0x2b0
> __mutex_lock+0xa6/0xd20
> dev_disable_lro+0x20/0x80
> inetdev_init+0x12f/0x1f0
> inetdev_event+0x48b/0x870
> notifier_call_chain+0x38/0xf0
> netif_change_net_namespace+0x72e/0x9f0
> do_setlink.isra.0+0xd5/0x1220
> rtnl_newlink+0x7ea/0xb50
> rtnetlink_rcv_msg+0x459/0x5e0
> netlink_rcv_skb+0x54/0x100
> netlink_unicast+0x193/0x270
> netlink_sendmsg+0x204/0x450
>
> inetdev_init conditionally disables LRO if forwarding is on:
> if (IPV4_DEVCONF(in_dev->cnf, FORWARDING))
> dev_disable_lro(dev);
>
> What to do in this case (besides the silly workaround to disable
> forwarding)?
I think something like the patch below should fix it? inetdev_init is
called for blackhole (sw device, we don't care about ops lock) and from
REGISTER/UNREGISTER notifiers. We hold the lock during REGISTER,
and will soon hold the lock during UNREGISTER:
https://lore.kernel.org/netdev/20250312223507.805719-9-kuba@kernel.org/
(might also need to EXPORT_SYM netif_disable_lro)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 754f60fb6e25..77e5705ac799 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -281,7 +281,7 @@ static struct in_device *inetdev_init(struct net_device *dev)
if (!in_dev->arp_parms)
goto out_kfree;
if (IPV4_DEVCONF(in_dev->cnf, FORWARDING))
- dev_disable_lro(dev);
+ netif_disable_lro(dev);
/* Reference in_dev->dev */
netdev_hold(dev, &in_dev->dev_tracker, GFP_KERNEL);
/* Account for reference dev->ip_ptr (below) */
Powered by blists - more mailing lists