* [patch net-next v3 1/7] net: treat possible_net_t net pointer as an RCU one and add read_pnet_rcu()
2023-10-13 12:10 [patch net-next v3 0/7] devlink: fix a deadlock when taking devlink instance lock while holding RTNL lock Jiri Pirko
@ 2023-10-13 12:10 ` Jiri Pirko
2023-10-17 8:49 ` Simon Horman
2023-10-13 12:10 ` [patch net-next v3 2/7] devlink: call peernet2id_alloc() with net pointer under RCU read lock Jiri Pirko
` (6 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Jiri Pirko @ 2023-10-13 12:10 UTC (permalink / raw)
To: netdev; +Cc: kuba, pabeni, davem, edumazet
From: Jiri Pirko <jiri@nvidia.com>
Make the net pointer stored in possible_net_t structure annotated as
an RCU pointer. Change the access helpers to treat it as such.
Introduce read_pnet_rcu() helper to allow caller to dereference
the net pointer under RCU read lock.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v1->v2:
- new patch
---
include/net/net_namespace.h | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index eb6cd43b1746..13b3a4e29fdb 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -368,21 +368,30 @@ static inline void put_net_track(struct net *net, netns_tracker *tracker)
typedef struct {
#ifdef CONFIG_NET_NS
- struct net *net;
+ struct net __rcu *net;
#endif
} possible_net_t;
static inline void write_pnet(possible_net_t *pnet, struct net *net)
{
#ifdef CONFIG_NET_NS
- pnet->net = net;
+ rcu_assign_pointer(pnet->net, net);
#endif
}
static inline struct net *read_pnet(const possible_net_t *pnet)
{
#ifdef CONFIG_NET_NS
- return pnet->net;
+ return rcu_dereference_protected(pnet->net, true);
+#else
+ return &init_net;
+#endif
+}
+
+static inline struct net *read_pnet_rcu(possible_net_t *pnet)
+{
+#ifdef CONFIG_NET_NS
+ return rcu_dereference(pnet->net);
#else
return &init_net;
#endif
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [patch net-next v3 2/7] devlink: call peernet2id_alloc() with net pointer under RCU read lock
2023-10-13 12:10 [patch net-next v3 0/7] devlink: fix a deadlock when taking devlink instance lock while holding RTNL lock Jiri Pirko
2023-10-13 12:10 ` [patch net-next v3 1/7] net: treat possible_net_t net pointer as an RCU one and add read_pnet_rcu() Jiri Pirko
@ 2023-10-13 12:10 ` Jiri Pirko
2023-10-17 8:50 ` Simon Horman
2023-10-13 12:10 ` [patch net-next v3 3/7] devlink: take device reference for devlink object Jiri Pirko
` (5 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Jiri Pirko @ 2023-10-13 12:10 UTC (permalink / raw)
To: netdev; +Cc: kuba, pabeni, davem, edumazet
From: Jiri Pirko <jiri@nvidia.com>
peernet2id_alloc() allows to be called lockless with peer net pointer
obtained in RCU critical section and makes sure to return ns ID if net
namespaces is not being removed concurrently. Benefit from
read_pnet_rcu() helper addition, use it to obtain net pointer under RCU
read lock and pass it to peernet2id_alloc() to get ns ID.
Fixes: c137743bce02 ("devlink: introduce object and nested devlink relationship infra")
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v2->v3:
- added fixes tag
v1->v2:
- moved the netns related bits from "devlink: don't take instance lock
for nested handle put"
- fixed the code using RCU to avoid use after free of peer net struct
---
net/devlink/netlink.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c
index 499304d9de49..809bfc3ba8c4 100644
--- a/net/devlink/netlink.c
+++ b/net/devlink/netlink.c
@@ -86,18 +86,24 @@ int devlink_nl_put_nested_handle(struct sk_buff *msg, struct net *net,
struct devlink *devlink, int attrtype)
{
struct nlattr *nested_attr;
+ struct net *devl_net;
nested_attr = nla_nest_start(msg, attrtype);
if (!nested_attr)
return -EMSGSIZE;
if (devlink_nl_put_handle(msg, devlink))
goto nla_put_failure;
- if (!net_eq(net, devlink_net(devlink))) {
- int id = peernet2id_alloc(net, devlink_net(devlink),
- GFP_KERNEL);
+ rcu_read_lock();
+ devl_net = read_pnet_rcu(&devlink->_net);
+ if (!net_eq(net, devl_net)) {
+ int id = peernet2id_alloc(net, devl_net, GFP_ATOMIC);
+
+ rcu_read_unlock();
if (nla_put_s32(msg, DEVLINK_ATTR_NETNS_ID, id))
return -EMSGSIZE;
+ } else {
+ rcu_read_unlock();
}
nla_nest_end(msg, nested_attr);
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [patch net-next v3 2/7] devlink: call peernet2id_alloc() with net pointer under RCU read lock
2023-10-13 12:10 ` [patch net-next v3 2/7] devlink: call peernet2id_alloc() with net pointer under RCU read lock Jiri Pirko
@ 2023-10-17 8:50 ` Simon Horman
0 siblings, 0 replies; 16+ messages in thread
From: Simon Horman @ 2023-10-17 8:50 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, kuba, pabeni, davem, edumazet
On Fri, Oct 13, 2023 at 02:10:24PM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> peernet2id_alloc() allows to be called lockless with peer net pointer
> obtained in RCU critical section and makes sure to return ns ID if net
> namespaces is not being removed concurrently. Benefit from
> read_pnet_rcu() helper addition, use it to obtain net pointer under RCU
> read lock and pass it to peernet2id_alloc() to get ns ID.
>
> Fixes: c137743bce02 ("devlink: introduce object and nested devlink relationship infra")
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Simon Horman <horms@kernel.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [patch net-next v3 3/7] devlink: take device reference for devlink object
2023-10-13 12:10 [patch net-next v3 0/7] devlink: fix a deadlock when taking devlink instance lock while holding RTNL lock Jiri Pirko
2023-10-13 12:10 ` [patch net-next v3 1/7] net: treat possible_net_t net pointer as an RCU one and add read_pnet_rcu() Jiri Pirko
2023-10-13 12:10 ` [patch net-next v3 2/7] devlink: call peernet2id_alloc() with net pointer under RCU read lock Jiri Pirko
@ 2023-10-13 12:10 ` Jiri Pirko
2023-10-17 8:50 ` Simon Horman
2023-10-13 12:10 ` [patch net-next v3 4/7] devlink: don't take instance lock for nested handle put Jiri Pirko
` (4 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Jiri Pirko @ 2023-10-13 12:10 UTC (permalink / raw)
To: netdev; +Cc: kuba, pabeni, davem, edumazet
From: Jiri Pirko <jiri@nvidia.com>
In preparation to allow to access device pointer without devlink
instance lock held, make sure the device pointer is usable until
devlink_release() is called.
Fixes: c137743bce02 ("devlink: introduce object and nested devlink relationship infra")
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v2->v3:
- moved the device reference related bits from "devlink: don't take
instance lock for nested handle put"
---
net/devlink/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/devlink/core.c b/net/devlink/core.c
index bcbbb952569f..c47c9e6c744f 100644
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -310,6 +310,7 @@ static void devlink_release(struct work_struct *work)
mutex_destroy(&devlink->lock);
lockdep_unregister_key(&devlink->lock_key);
+ put_device(devlink->dev);
kfree(devlink);
}
@@ -425,7 +426,7 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
if (ret < 0)
goto err_xa_alloc;
- devlink->dev = dev;
+ devlink->dev = get_device(dev);
devlink->ops = ops;
xa_init_flags(&devlink->ports, XA_FLAGS_ALLOC);
xa_init_flags(&devlink->params, XA_FLAGS_ALLOC);
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [patch net-next v3 4/7] devlink: don't take instance lock for nested handle put
2023-10-13 12:10 [patch net-next v3 0/7] devlink: fix a deadlock when taking devlink instance lock while holding RTNL lock Jiri Pirko
` (2 preceding siblings ...)
2023-10-13 12:10 ` [patch net-next v3 3/7] devlink: take device reference for devlink object Jiri Pirko
@ 2023-10-13 12:10 ` Jiri Pirko
2023-10-17 8:51 ` Simon Horman
2023-10-13 12:10 ` [patch net-next v3 5/7] Documentation: devlink: add nested instance section Jiri Pirko
` (3 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Jiri Pirko @ 2023-10-13 12:10 UTC (permalink / raw)
To: netdev; +Cc: kuba, pabeni, davem, edumazet
From: Jiri Pirko <jiri@nvidia.com>
Lockdep reports following issue:
WARNING: possible circular locking dependency detected
------------------------------------------------------
devlink/8191 is trying to acquire lock:
ffff88813f32c250 (&devlink->lock_key#14){+.+.}-{3:3}, at: devlink_rel_devlink_handle_put+0x11e/0x2d0
but task is already holding lock:
ffffffff8511eca8 (rtnl_mutex){+.+.}-{3:3}, at: unregister_netdev+0xe/0x20
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #3 (rtnl_mutex){+.+.}-{3:3}:
lock_acquire+0x1c3/0x500
__mutex_lock+0x14c/0x1b20
register_netdevice_notifier_net+0x13/0x30
mlx5_lag_add_mdev+0x51c/0xa00 [mlx5_core]
mlx5_load+0x222/0xc70 [mlx5_core]
mlx5_init_one_devl_locked+0x4a0/0x1310 [mlx5_core]
mlx5_init_one+0x3b/0x60 [mlx5_core]
probe_one+0x786/0xd00 [mlx5_core]
local_pci_probe+0xd7/0x180
pci_device_probe+0x231/0x720
really_probe+0x1e4/0xb60
__driver_probe_device+0x261/0x470
driver_probe_device+0x49/0x130
__driver_attach+0x215/0x4c0
bus_for_each_dev+0xf0/0x170
bus_add_driver+0x21d/0x590
driver_register+0x133/0x460
vdpa_match_remove+0x89/0xc0 [vdpa]
do_one_initcall+0xc4/0x360
do_init_module+0x22d/0x760
load_module+0x51d7/0x6750
init_module_from_file+0xd2/0x130
idempotent_init_module+0x326/0x5a0
__x64_sys_finit_module+0xc1/0x130
do_syscall_64+0x3d/0x90
entry_SYSCALL_64_after_hwframe+0x46/0xb0
-> #2 (mlx5_intf_mutex){+.+.}-{3:3}:
lock_acquire+0x1c3/0x500
__mutex_lock+0x14c/0x1b20
mlx5_register_device+0x3e/0xd0 [mlx5_core]
mlx5_init_one_devl_locked+0x8fa/0x1310 [mlx5_core]
mlx5_devlink_reload_up+0x147/0x170 [mlx5_core]
devlink_reload+0x203/0x380
devlink_nl_cmd_reload+0xb84/0x10e0
genl_family_rcv_msg_doit+0x1cc/0x2a0
genl_rcv_msg+0x3c9/0x670
netlink_rcv_skb+0x12c/0x360
genl_rcv+0x24/0x40
netlink_unicast+0x435/0x6f0
netlink_sendmsg+0x7a0/0xc70
sock_sendmsg+0xc5/0x190
__sys_sendto+0x1c8/0x290
__x64_sys_sendto+0xdc/0x1b0
do_syscall_64+0x3d/0x90
entry_SYSCALL_64_after_hwframe+0x46/0xb0
-> #1 (&dev->lock_key#8){+.+.}-{3:3}:
lock_acquire+0x1c3/0x500
__mutex_lock+0x14c/0x1b20
mlx5_init_one_devl_locked+0x45/0x1310 [mlx5_core]
mlx5_devlink_reload_up+0x147/0x170 [mlx5_core]
devlink_reload+0x203/0x380
devlink_nl_cmd_reload+0xb84/0x10e0
genl_family_rcv_msg_doit+0x1cc/0x2a0
genl_rcv_msg+0x3c9/0x670
netlink_rcv_skb+0x12c/0x360
genl_rcv+0x24/0x40
netlink_unicast+0x435/0x6f0
netlink_sendmsg+0x7a0/0xc70
sock_sendmsg+0xc5/0x190
__sys_sendto+0x1c8/0x290
__x64_sys_sendto+0xdc/0x1b0
do_syscall_64+0x3d/0x90
entry_SYSCALL_64_after_hwframe+0x46/0xb0
-> #0 (&devlink->lock_key#14){+.+.}-{3:3}:
check_prev_add+0x1af/0x2300
__lock_acquire+0x31d7/0x4eb0
lock_acquire+0x1c3/0x500
__mutex_lock+0x14c/0x1b20
devlink_rel_devlink_handle_put+0x11e/0x2d0
devlink_nl_port_fill+0xddf/0x1b00
devlink_port_notify+0xb5/0x220
__devlink_port_type_set+0x151/0x510
devlink_port_netdevice_event+0x17c/0x220
notifier_call_chain+0x97/0x240
unregister_netdevice_many_notify+0x876/0x1790
unregister_netdevice_queue+0x274/0x350
unregister_netdev+0x18/0x20
mlx5e_vport_rep_unload+0xc5/0x1c0 [mlx5_core]
__esw_offloads_unload_rep+0xd8/0x130 [mlx5_core]
mlx5_esw_offloads_rep_unload+0x52/0x70 [mlx5_core]
mlx5_esw_offloads_unload_rep+0x85/0xc0 [mlx5_core]
mlx5_eswitch_unload_sf_vport+0x41/0x90 [mlx5_core]
mlx5_devlink_sf_port_del+0x120/0x280 [mlx5_core]
genl_family_rcv_msg_doit+0x1cc/0x2a0
genl_rcv_msg+0x3c9/0x670
netlink_rcv_skb+0x12c/0x360
genl_rcv+0x24/0x40
netlink_unicast+0x435/0x6f0
netlink_sendmsg+0x7a0/0xc70
sock_sendmsg+0xc5/0x190
__sys_sendto+0x1c8/0x290
__x64_sys_sendto+0xdc/0x1b0
do_syscall_64+0x3d/0x90
entry_SYSCALL_64_after_hwframe+0x46/0xb0
other info that might help us debug this:
Chain exists of:
&devlink->lock_key#14 --> mlx5_intf_mutex --> rtnl_mutex
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(rtnl_mutex);
lock(mlx5_intf_mutex);
lock(rtnl_mutex);
lock(&devlink->lock_key#14);
Problem is taking the devlink instance lock of nested instance when RTNL
is already held.
To fix this, don't take the devlink instance lock when putting nested
handle. Instead, rely on the preparations done by previous two patches
to be able to access device pointer and obtain netns id without devlink
instance lock held.
Fixes: c137743bce02 ("devlink: introduce object and nested devlink relationship infra")
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v2->v3:
- push device reference part into separate patch
- adjusted the patch description
v1->v2:
- push netns part into separate patch
---
net/devlink/core.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/net/devlink/core.c b/net/devlink/core.c
index c47c9e6c744f..655903ddbdfd 100644
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -183,9 +183,8 @@ static struct devlink_rel *devlink_rel_find(unsigned long rel_index)
DEVLINK_REL_IN_USE);
}
-static struct devlink *devlink_rel_devlink_get_lock(u32 rel_index)
+static struct devlink *devlink_rel_devlink_get(u32 rel_index)
{
- struct devlink *devlink;
struct devlink_rel *rel;
u32 devlink_index;
@@ -198,16 +197,7 @@ static struct devlink *devlink_rel_devlink_get_lock(u32 rel_index)
xa_unlock(&devlink_rels);
if (!rel)
return NULL;
- devlink = devlinks_xa_get(devlink_index);
- if (!devlink)
- return NULL;
- devl_lock(devlink);
- if (!devl_is_registered(devlink)) {
- devl_unlock(devlink);
- devlink_put(devlink);
- return NULL;
- }
- return devlink;
+ return devlinks_xa_get(devlink_index);
}
int devlink_rel_devlink_handle_put(struct sk_buff *msg, struct devlink *devlink,
@@ -218,11 +208,10 @@ int devlink_rel_devlink_handle_put(struct sk_buff *msg, struct devlink *devlink,
struct devlink *rel_devlink;
int err;
- rel_devlink = devlink_rel_devlink_get_lock(rel_index);
+ rel_devlink = devlink_rel_devlink_get(rel_index);
if (!rel_devlink)
return 0;
err = devlink_nl_put_nested_handle(msg, net, rel_devlink, attrtype);
- devl_unlock(rel_devlink);
devlink_put(rel_devlink);
if (!err && msg_updated)
*msg_updated = true;
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [patch net-next v3 4/7] devlink: don't take instance lock for nested handle put
2023-10-13 12:10 ` [patch net-next v3 4/7] devlink: don't take instance lock for nested handle put Jiri Pirko
@ 2023-10-17 8:51 ` Simon Horman
0 siblings, 0 replies; 16+ messages in thread
From: Simon Horman @ 2023-10-17 8:51 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, kuba, pabeni, davem, edumazet
On Fri, Oct 13, 2023 at 02:10:26PM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> Lockdep reports following issue:
>
> WARNING: possible circular locking dependency detected
> ------------------------------------------------------
> devlink/8191 is trying to acquire lock:
> ffff88813f32c250 (&devlink->lock_key#14){+.+.}-{3:3}, at: devlink_rel_devlink_handle_put+0x11e/0x2d0
>
> but task is already holding lock:
> ffffffff8511eca8 (rtnl_mutex){+.+.}-{3:3}, at: unregister_netdev+0xe/0x20
>
> which lock already depends on the new lock.
>
> the existing dependency chain (in reverse order) is:
>
> -> #3 (rtnl_mutex){+.+.}-{3:3}:
...
> Possible unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> lock(rtnl_mutex);
> lock(mlx5_intf_mutex);
> lock(rtnl_mutex);
> lock(&devlink->lock_key#14);
>
> Problem is taking the devlink instance lock of nested instance when RTNL
> is already held.
>
> To fix this, don't take the devlink instance lock when putting nested
> handle. Instead, rely on the preparations done by previous two patches
> to be able to access device pointer and obtain netns id without devlink
> instance lock held.
>
> Fixes: c137743bce02 ("devlink: introduce object and nested devlink relationship infra")
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Simon Horman <horms@kernel.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [patch net-next v3 5/7] Documentation: devlink: add nested instance section
2023-10-13 12:10 [patch net-next v3 0/7] devlink: fix a deadlock when taking devlink instance lock while holding RTNL lock Jiri Pirko
` (3 preceding siblings ...)
2023-10-13 12:10 ` [patch net-next v3 4/7] devlink: don't take instance lock for nested handle put Jiri Pirko
@ 2023-10-13 12:10 ` Jiri Pirko
2023-10-17 8:52 ` Simon Horman
2023-10-13 12:10 ` [patch net-next v3 6/7] Documentation: devlink: add a note about RTNL lock into locking section Jiri Pirko
` (2 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Jiri Pirko @ 2023-10-13 12:10 UTC (permalink / raw)
To: netdev; +Cc: kuba, pabeni, davem, edumazet
From: Jiri Pirko <jiri@nvidia.com>
Add a part talking about nested devlink instances describing
the helpers and locking ordering.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v2->v3:
- new patch
---
Documentation/networking/devlink/index.rst | 24 ++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
index b49749e2b9a6..52e52a1b603d 100644
--- a/Documentation/networking/devlink/index.rst
+++ b/Documentation/networking/devlink/index.rst
@@ -18,6 +18,30 @@ netlink commands.
Drivers are encouraged to use the devlink instance lock for their own needs.
+Nested instances
+----------------
+
+Some objects, like linecards or port functions, could have another
+devlink instances created underneath. In that case, drivers should make
+sure to respect following rules:
+
+ - Lock ordering should be maintained. If driver needs to take instance
+ lock of both nested and parent instances at the same time, devlink
+ instance lock of the parent instance should be taken first, only then
+ instance lock of the nested instance could be taken.
+ - Driver should use object-specific helpers to setup the
+ nested relationship:
+
+ - ``devl_nested_devlink_set()`` - called to setup devlink -> nested
+ devlink relationship (could be user for multiple nested instances.
+ - ``devl_port_fn_devlink_set()`` - called to setup port function ->
+ nested devlink relationship.
+ - ``devlink_linecard_nested_dl_set()`` - called to setup linecard ->
+ nested devlink relationship.
+
+The nested devlink info is exposed to the userspace over object-specific
+attributes of devlink netlink.
+
Interface documentation
-----------------------
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [patch net-next v3 6/7] Documentation: devlink: add a note about RTNL lock into locking section
2023-10-13 12:10 [patch net-next v3 0/7] devlink: fix a deadlock when taking devlink instance lock while holding RTNL lock Jiri Pirko
` (4 preceding siblings ...)
2023-10-13 12:10 ` [patch net-next v3 5/7] Documentation: devlink: add nested instance section Jiri Pirko
@ 2023-10-13 12:10 ` Jiri Pirko
2023-10-17 8:53 ` Simon Horman
2023-10-13 12:10 ` [patch net-next v3 7/7] devlink: document devlink_rel_nested_in_notify() function Jiri Pirko
2023-10-18 8:30 ` [patch net-next v3 0/7] devlink: fix a deadlock when taking devlink instance lock while holding RTNL lock patchwork-bot+netdevbpf
7 siblings, 1 reply; 16+ messages in thread
From: Jiri Pirko @ 2023-10-13 12:10 UTC (permalink / raw)
To: netdev; +Cc: kuba, pabeni, davem, edumazet
From: Jiri Pirko <jiri@nvidia.com>
Add a note describing the locking order of taking RTNL lock with devlink
instance lock.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v2->v3:
- new patch
---
Documentation/networking/devlink/index.rst | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
index 52e52a1b603d..2884ad243b54 100644
--- a/Documentation/networking/devlink/index.rst
+++ b/Documentation/networking/devlink/index.rst
@@ -18,6 +18,10 @@ netlink commands.
Drivers are encouraged to use the devlink instance lock for their own needs.
+Drivers need to be cautious when taking devlink instance lock and
+taking RTNL lock at the same time. Devlink instance lock needs to be taken
+first, only after that RTNL lock could be taken.
+
Nested instances
----------------
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [patch net-next v3 7/7] devlink: document devlink_rel_nested_in_notify() function
2023-10-13 12:10 [patch net-next v3 0/7] devlink: fix a deadlock when taking devlink instance lock while holding RTNL lock Jiri Pirko
` (5 preceding siblings ...)
2023-10-13 12:10 ` [patch net-next v3 6/7] Documentation: devlink: add a note about RTNL lock into locking section Jiri Pirko
@ 2023-10-13 12:10 ` Jiri Pirko
2023-10-17 8:53 ` Simon Horman
2023-10-18 8:30 ` [patch net-next v3 0/7] devlink: fix a deadlock when taking devlink instance lock while holding RTNL lock patchwork-bot+netdevbpf
7 siblings, 1 reply; 16+ messages in thread
From: Jiri Pirko @ 2023-10-13 12:10 UTC (permalink / raw)
To: netdev; +Cc: kuba, pabeni, davem, edumazet
From: Jiri Pirko <jiri@nvidia.com>
Add a documentation for devlink_rel_nested_in_notify() describing the
devlink instance locking consequences.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v2->v3:
- new patch
---
net/devlink/core.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/net/devlink/core.c b/net/devlink/core.c
index 655903ddbdfd..6984877e9f10 100644
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -168,6 +168,20 @@ int devlink_rel_nested_in_add(u32 *rel_index, u32 devlink_index,
return 0;
}
+/**
+ * devlink_rel_nested_in_notify - Notify the object this devlink
+ * instance is nested in.
+ * @devlink: devlink
+ *
+ * This is called upon network namespace change of devlink instance.
+ * In case this devlink instance is nested in another devlink object,
+ * a notification of a change of this object should be sent
+ * over netlink. The parent devlink instance lock needs to be
+ * taken during the notification preparation.
+ * However, since the devlink lock of nested instance is held here,
+ * we would end with wrong devlink instance lock ordering and
+ * deadlock. Therefore the work is utilized to avoid that.
+ */
void devlink_rel_nested_in_notify(struct devlink *devlink)
{
struct devlink_rel *rel = devlink->rel;
--
2.41.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [patch net-next v3 0/7] devlink: fix a deadlock when taking devlink instance lock while holding RTNL lock
2023-10-13 12:10 [patch net-next v3 0/7] devlink: fix a deadlock when taking devlink instance lock while holding RTNL lock Jiri Pirko
` (6 preceding siblings ...)
2023-10-13 12:10 ` [patch net-next v3 7/7] devlink: document devlink_rel_nested_in_notify() function Jiri Pirko
@ 2023-10-18 8:30 ` patchwork-bot+netdevbpf
7 siblings, 0 replies; 16+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-10-18 8:30 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, kuba, pabeni, davem, edumazet
Hello:
This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:
On Fri, 13 Oct 2023 14:10:22 +0200 you wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> devlink_port_fill() may be called sometimes with RTNL lock held.
> When putting the nested port function devlink instance attrs,
> current code takes nested devlink instance lock. In that case lock
> ordering is wrong.
>
> [...]
Here is the summary with links:
- [net-next,v3,1/7] net: treat possible_net_t net pointer as an RCU one and add read_pnet_rcu()
https://git.kernel.org/netdev/net-next/c/2034d90ae41a
- [net-next,v3,2/7] devlink: call peernet2id_alloc() with net pointer under RCU read lock
https://git.kernel.org/netdev/net-next/c/c503bc7df602
- [net-next,v3,3/7] devlink: take device reference for devlink object
https://git.kernel.org/netdev/net-next/c/a380687200e0
- [net-next,v3,4/7] devlink: don't take instance lock for nested handle put
https://git.kernel.org/netdev/net-next/c/b5f4e371336a
- [net-next,v3,5/7] Documentation: devlink: add nested instance section
https://git.kernel.org/netdev/net-next/c/b6f23b319aad
- [net-next,v3,6/7] Documentation: devlink: add a note about RTNL lock into locking section
https://git.kernel.org/netdev/net-next/c/bb11cf9b2c4a
- [net-next,v3,7/7] devlink: document devlink_rel_nested_in_notify() function
https://git.kernel.org/netdev/net-next/c/5d77371e8c85
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 16+ messages in thread