| Message ID | 20241105020514.41963-8-kuniyu@amazon.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | rtnetlink: Convert rtnl_newlink() to per-netns RTNL. | expand |
On Tue, Nov 5, 2024 at 3:07 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote: > > Now, we are ready to convert rtnl_newlink() to per-netns RTNL; > rtnl_link_ops is protected by SRCU and netns is prefetched in > rtnl_newlink(). > > Let's register rtnl_newlink() with RTNL_FLAG_DOIT_PERNET and > push RTNL down as rtnl_nets_lock(). > > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com> > --- Reviewed-by: Eric Dumazet <edumazet@google.com>
On 11/5/24 03:05, Kuniyuki Iwashima wrote: > @@ -6995,7 +7017,8 @@ static struct pernet_operations rtnetlink_net_ops = { > }; > > static const struct rtnl_msg_handler rtnetlink_rtnl_msg_handlers[] __initconst = { > - {.msgtype = RTM_NEWLINK, .doit = rtnl_newlink}, > + {.msgtype = RTM_NEWLINK, .doit = rtnl_newlink, > + .flags = RTNL_FLAG_DOIT_PERNET}, The above causes a lockdep splat in many selftests: https://netdev-3.bots.linux.dev/vmksft-net-dbg/results/846801/12-bareudp-sh/stderr the problem is in rtnl_newlink(): #ifdef CONFIG_MODULES if (!ops) { __rtnl_unlock(); // we no more under the rtnlock request_module("rtnl-link-%s", kind); rtnl_lock(); ops = rtnl_link_ops_get(kind, &ops_srcu_index); } #endif Cheers, Paolo
From: Paolo Abeni <pabeni@redhat.com> Date: Tue, 5 Nov 2024 17:22:54 +0100 > On 11/5/24 03:05, Kuniyuki Iwashima wrote: > > @@ -6995,7 +7017,8 @@ static struct pernet_operations rtnetlink_net_ops = { > > }; > > > > static const struct rtnl_msg_handler rtnetlink_rtnl_msg_handlers[] __initconst = { > > - {.msgtype = RTM_NEWLINK, .doit = rtnl_newlink}, > > + {.msgtype = RTM_NEWLINK, .doit = rtnl_newlink, > > + .flags = RTNL_FLAG_DOIT_PERNET}, > > The above causes a lockdep splat in many selftests: > > https://netdev-3.bots.linux.dev/vmksft-net-dbg/results/846801/12-bareudp-sh/stderr > > the problem is in rtnl_newlink(): > > #ifdef CONFIG_MODULES > if (!ops) { > __rtnl_unlock(); > // we no more under the rtnlock > request_module("rtnl-link-%s", kind); > rtnl_lock(); > ops = rtnl_link_ops_get(kind, &ops_srcu_index); > } > #endif Oh, somehow I dropped this part of change while rebasing the RFC version... https://github.com/q2ven/linux/commit/fea4b46993e8802e6dd7341d6e7dd49396e378d1 Will remove the unlock dance in v2. Thanks!
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 48bd9e062550..0df0cba0a700 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -312,6 +312,26 @@ void rtnl_nets_add(struct rtnl_nets *rtnl_nets, struct net *net) } EXPORT_SYMBOL(rtnl_nets_add); +static void rtnl_nets_lock(struct rtnl_nets *rtnl_nets) +{ + int i; + + rtnl_lock(); + + for (i = 0; i < rtnl_nets->len; i++) + __rtnl_net_lock(rtnl_nets->net[i]); +} + +static void rtnl_nets_unlock(struct rtnl_nets *rtnl_nets) +{ + int i; + + for (i = 0; i < rtnl_nets->len; i++) + __rtnl_net_unlock(rtnl_nets->net[i]); + + rtnl_unlock(); +} + static struct rtnl_link __rcu *__rcu *rtnl_msg_handlers[RTNL_FAMILY_MAX + 1]; static inline int rtm_msgindex(int msgtype) @@ -3991,7 +4011,9 @@ static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, } } + rtnl_nets_lock(&rtnl_nets); ret = __rtnl_newlink(skb, nlh, ops, tgt_net, link_net, tbs, data, extack); + rtnl_nets_unlock(&rtnl_nets); put_net: rtnl_nets_destroy(&rtnl_nets); @@ -6995,7 +7017,8 @@ static struct pernet_operations rtnetlink_net_ops = { }; static const struct rtnl_msg_handler rtnetlink_rtnl_msg_handlers[] __initconst = { - {.msgtype = RTM_NEWLINK, .doit = rtnl_newlink}, + {.msgtype = RTM_NEWLINK, .doit = rtnl_newlink, + .flags = RTNL_FLAG_DOIT_PERNET}, {.msgtype = RTM_DELLINK, .doit = rtnl_dellink}, {.msgtype = RTM_GETLINK, .doit = rtnl_getlink, .dumpit = rtnl_dump_ifinfo, .flags = RTNL_FLAG_DUMP_SPLIT_NLM_DONE},
Now, we are ready to convert rtnl_newlink() to per-netns RTNL; rtnl_link_ops is protected by SRCU and netns is prefetched in rtnl_newlink(). Let's register rtnl_newlink() with RTNL_FLAG_DOIT_PERNET and push RTNL down as rtnl_nets_lock(). Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com> --- net/core/rtnetlink.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)