* [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl
@ 2017-08-09 18:41 Florian Westphal
2017-08-09 18:41 ` [PATCH v2 net-next 1/7] rtnetlink: call rtnl_calcit directly Florian Westphal
` (7 more replies)
0 siblings, 8 replies; 13+ messages in thread
From: Florian Westphal @ 2017-08-09 18:41 UTC (permalink / raw)
To: netdev
Changes since v1:
In patch 6, don't make ipv6 route handlers lockless, they all have
assumptions on rtnl being held. Other patches are unchanged.
The RTNL mutex is used to serialize both rtnetlink calls and
dump requests.
Its also used to protect other things such as the list of current
net namespaces.
Unfortunately RTNL mutex is a performance issue, e.g. a cpu adding an
ip address prevents other cpus from seemingly unrelated tasks such as
dumping tc classifiers or doing rtnetlink route lookups.
This patch set adds basic infrastructure to start pushing the rtnl lock
down to those places that need it, or even elide it entirely in some cases.
Subsystems can now indicate that their doit() callback can run without
RTNL mutex, such callbacks can then run in parallel.
This will obviously need a lot of followup work; all current
users need to be audited/changed to benefit from this.
Initial no-rtnl spot is netns new/getid.
We have various 'get' handlers that are also a tempting target,
however, several of these depend on rtnl mutex to prevent information
from changing while objects are being read by rtnl handlers; however,
it doesn't appear impossible to change this.
Dumps are another problem entirely, see
commit 2907c35ff64708065 ("net: hold rtnl again in dump callbacks"),
this patchset doesn't touch dump requests.
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v2 net-next 1/7] rtnetlink: call rtnl_calcit directly 2017-08-09 18:41 [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl Florian Westphal @ 2017-08-09 18:41 ` Florian Westphal 2017-08-09 18:41 ` [PATCH v2 net-next 2/7] rtnetlink: make rtnl_register accept a flags parameter Florian Westphal ` (6 subsequent siblings) 7 siblings, 0 replies; 13+ messages in thread From: Florian Westphal @ 2017-08-09 18:41 UTC (permalink / raw) To: netdev; +Cc: Florian Westphal There is only a single place in the kernel that regisers the "calcit" callback (to determine min allocation for dumps). This is in rtnetlink.c for PF_UNSPEC RTM_GETLINK. The function that checks for calcit presence at run time will first check the requested family (which will always fail for !PF_UNSPEC as no callsite registers this), then falls back to checking PF_UNSPEC. Therefore we can just check if type is RTM_GETLINK and then do a direct call. Because of fallback to PF_UNSPEC all RTM_GETLINK types used this regardless of family. This has the advantage that we don't need to allocate space for the function pointer for all the other families. A followup patch will drop the calcit function pointer from the rtnl_link callback structure. Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org> --- No changes since v1. net/core/rtnetlink.c | 29 ++++------------------------- 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 9201e3621351..8c9d34deea7d 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -62,7 +62,6 @@ struct rtnl_link { rtnl_doit_func doit; rtnl_dumpit_func dumpit; - rtnl_calcit_func calcit; }; static DEFINE_MUTEX(rtnl_mutex); @@ -173,21 +172,6 @@ static rtnl_dumpit_func rtnl_get_dumpit(int protocol, int msgindex) return tab[msgindex].dumpit; } -static rtnl_calcit_func rtnl_get_calcit(int protocol, int msgindex) -{ - struct rtnl_link *tab; - - if (protocol <= RTNL_FAMILY_MAX) - tab = rtnl_msg_handlers[protocol]; - else - tab = NULL; - - if (tab == NULL || tab[msgindex].calcit == NULL) - tab = rtnl_msg_handlers[PF_UNSPEC]; - - return tab[msgindex].calcit; -} - /** * __rtnl_register - Register a rtnetlink message type * @protocol: Protocol family or PF_UNSPEC @@ -231,9 +215,6 @@ int __rtnl_register(int protocol, int msgtype, if (dumpit) tab[msgindex].dumpit = dumpit; - if (calcit) - tab[msgindex].calcit = calcit; - return 0; } EXPORT_SYMBOL_GPL(__rtnl_register); @@ -277,7 +258,6 @@ int rtnl_unregister(int protocol, int msgtype) rtnl_msg_handlers[protocol][msgindex].doit = NULL; rtnl_msg_handlers[protocol][msgindex].dumpit = NULL; - rtnl_msg_handlers[protocol][msgindex].calcit = NULL; return 0; } @@ -4187,15 +4167,14 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, if (kind == 2 && nlh->nlmsg_flags&NLM_F_DUMP) { struct sock *rtnl; rtnl_dumpit_func dumpit; - rtnl_calcit_func calcit; u16 min_dump_alloc = 0; dumpit = rtnl_get_dumpit(family, type); if (dumpit == NULL) return -EOPNOTSUPP; - calcit = rtnl_get_calcit(family, type); - if (calcit) - min_dump_alloc = calcit(skb, nlh); + + if (type == RTM_GETLINK) + min_dump_alloc = rtnl_calcit(skb, nlh); __rtnl_unlock(); rtnl = net->rtnl; @@ -4300,7 +4279,7 @@ void __init rtnetlink_init(void) register_netdevice_notifier(&rtnetlink_dev_notifier); rtnl_register(PF_UNSPEC, RTM_GETLINK, rtnl_getlink, - rtnl_dump_ifinfo, rtnl_calcit); + rtnl_dump_ifinfo, NULL); rtnl_register(PF_UNSPEC, RTM_SETLINK, rtnl_setlink, NULL, NULL); rtnl_register(PF_UNSPEC, RTM_NEWLINK, rtnl_newlink, NULL, NULL); rtnl_register(PF_UNSPEC, RTM_DELLINK, rtnl_dellink, NULL, NULL); -- 2.13.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 net-next 2/7] rtnetlink: make rtnl_register accept a flags parameter 2017-08-09 18:41 [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl Florian Westphal 2017-08-09 18:41 ` [PATCH v2 net-next 1/7] rtnetlink: call rtnl_calcit directly Florian Westphal @ 2017-08-09 18:41 ` Florian Westphal 2017-08-09 18:41 ` [PATCH v2 net-next 3/7] rtnetlink: add reference counting to prevent module unload while dump is in progress Florian Westphal ` (5 subsequent siblings) 7 siblings, 0 replies; 13+ messages in thread From: Florian Westphal @ 2017-08-09 18:41 UTC (permalink / raw) To: netdev; +Cc: Florian Westphal This change allows us to later indicate to rtnetlink core that certain doit functions should be called without acquiring rtnl_mutex. This change should have no effect, we simply replace the last (now unused) calcit argument with the new flag. Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org> --- No changes since v1. include/net/rtnetlink.h | 5 ++--- net/bridge/br_mdb.c | 6 +++--- net/can/gw.c | 6 +++--- net/core/fib_rules.c | 6 +++--- net/core/neighbour.c | 10 +++++----- net/core/net_namespace.c | 4 ++-- net/core/rtnetlink.c | 36 ++++++++++++++++++------------------ net/dcb/dcbnl.c | 4 ++-- net/decnet/dn_dev.c | 6 +++--- net/decnet/dn_fib.c | 4 ++-- net/decnet/dn_route.c | 4 ++-- net/ipv4/devinet.c | 8 ++++---- net/ipv4/fib_frontend.c | 6 +++--- net/ipv4/ipmr.c | 8 ++++---- net/ipv4/route.c | 2 +- net/ipv6/addrconf.c | 14 +++++++------- net/ipv6/addrlabel.c | 6 +++--- net/ipv6/ip6_fib.c | 2 +- net/ipv6/ip6mr.c | 2 +- net/ipv6/route.c | 6 +++--- net/mpls/af_mpls.c | 8 ++++---- net/phonet/pn_netlink.c | 12 ++++++------ net/qrtr/qrtr.c | 2 +- net/sched/act_api.c | 6 +++--- net/sched/cls_api.c | 6 +++--- net/sched/sch_api.c | 12 ++++++------ 26 files changed, 95 insertions(+), 96 deletions(-) diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h index abe6b733d473..ac32460a0adb 100644 --- a/include/net/rtnetlink.h +++ b/include/net/rtnetlink.h @@ -7,12 +7,11 @@ typedef int (*rtnl_doit_func)(struct sk_buff *, struct nlmsghdr *, struct netlink_ext_ack *); typedef int (*rtnl_dumpit_func)(struct sk_buff *, struct netlink_callback *); -typedef u16 (*rtnl_calcit_func)(struct sk_buff *, struct nlmsghdr *); int __rtnl_register(int protocol, int msgtype, - rtnl_doit_func, rtnl_dumpit_func, rtnl_calcit_func); + rtnl_doit_func, rtnl_dumpit_func, unsigned int flags); void rtnl_register(int protocol, int msgtype, - rtnl_doit_func, rtnl_dumpit_func, rtnl_calcit_func); + rtnl_doit_func, rtnl_dumpit_func, unsigned int flags); int rtnl_unregister(int protocol, int msgtype); void rtnl_unregister_all(int protocol); diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c index a0b11e7d67d9..ca01def49af0 100644 --- a/net/bridge/br_mdb.c +++ b/net/bridge/br_mdb.c @@ -713,9 +713,9 @@ static int br_mdb_del(struct sk_buff *skb, struct nlmsghdr *nlh, void br_mdb_init(void) { - rtnl_register(PF_BRIDGE, RTM_GETMDB, NULL, br_mdb_dump, NULL); - rtnl_register(PF_BRIDGE, RTM_NEWMDB, br_mdb_add, NULL, NULL); - rtnl_register(PF_BRIDGE, RTM_DELMDB, br_mdb_del, NULL, NULL); + rtnl_register(PF_BRIDGE, RTM_GETMDB, NULL, br_mdb_dump, 0); + rtnl_register(PF_BRIDGE, RTM_NEWMDB, br_mdb_add, NULL, 0); + rtnl_register(PF_BRIDGE, RTM_DELMDB, br_mdb_del, NULL, 0); } void br_mdb_uninit(void) diff --git a/net/can/gw.c b/net/can/gw.c index 29748d844c3f..73a02af4b5d7 100644 --- a/net/can/gw.c +++ b/net/can/gw.c @@ -1031,15 +1031,15 @@ static __init int cgw_module_init(void) notifier.notifier_call = cgw_notifier; register_netdevice_notifier(¬ifier); - if (__rtnl_register(PF_CAN, RTM_GETROUTE, NULL, cgw_dump_jobs, NULL)) { + if (__rtnl_register(PF_CAN, RTM_GETROUTE, NULL, cgw_dump_jobs, 0)) { unregister_netdevice_notifier(¬ifier); kmem_cache_destroy(cgw_cache); return -ENOBUFS; } /* Only the first call to __rtnl_register can fail */ - __rtnl_register(PF_CAN, RTM_NEWROUTE, cgw_create_job, NULL, NULL); - __rtnl_register(PF_CAN, RTM_DELROUTE, cgw_remove_job, NULL, NULL); + __rtnl_register(PF_CAN, RTM_NEWROUTE, cgw_create_job, NULL, 0); + __rtnl_register(PF_CAN, RTM_DELROUTE, cgw_remove_job, NULL, 0); return 0; } diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c index fc0b65093417..9a6d97c1d810 100644 --- a/net/core/fib_rules.c +++ b/net/core/fib_rules.c @@ -1026,9 +1026,9 @@ static struct pernet_operations fib_rules_net_ops = { static int __init fib_rules_init(void) { int err; - rtnl_register(PF_UNSPEC, RTM_NEWRULE, fib_nl_newrule, NULL, NULL); - rtnl_register(PF_UNSPEC, RTM_DELRULE, fib_nl_delrule, NULL, NULL); - rtnl_register(PF_UNSPEC, RTM_GETRULE, NULL, fib_nl_dumprule, NULL); + rtnl_register(PF_UNSPEC, RTM_NEWRULE, fib_nl_newrule, NULL, 0); + rtnl_register(PF_UNSPEC, RTM_DELRULE, fib_nl_delrule, NULL, 0); + rtnl_register(PF_UNSPEC, RTM_GETRULE, NULL, fib_nl_dumprule, 0); err = register_pernet_subsys(&fib_rules_net_ops); if (err < 0) diff --git a/net/core/neighbour.c b/net/core/neighbour.c index d0713627deb6..16a1a4c4eb57 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -3261,13 +3261,13 @@ EXPORT_SYMBOL(neigh_sysctl_unregister); static int __init neigh_init(void) { - rtnl_register(PF_UNSPEC, RTM_NEWNEIGH, neigh_add, NULL, NULL); - rtnl_register(PF_UNSPEC, RTM_DELNEIGH, neigh_delete, NULL, NULL); - rtnl_register(PF_UNSPEC, RTM_GETNEIGH, NULL, neigh_dump_info, NULL); + rtnl_register(PF_UNSPEC, RTM_NEWNEIGH, neigh_add, NULL, 0); + rtnl_register(PF_UNSPEC, RTM_DELNEIGH, neigh_delete, NULL, 0); + rtnl_register(PF_UNSPEC, RTM_GETNEIGH, NULL, neigh_dump_info, 0); rtnl_register(PF_UNSPEC, RTM_GETNEIGHTBL, NULL, neightbl_dump_info, - NULL); - rtnl_register(PF_UNSPEC, RTM_SETNEIGHTBL, neightbl_set, NULL, NULL); + 0); + rtnl_register(PF_UNSPEC, RTM_SETNEIGHTBL, neightbl_set, NULL, 0); return 0; } diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c index 8726d051f31d..a7f06d706aa0 100644 --- a/net/core/net_namespace.c +++ b/net/core/net_namespace.c @@ -855,9 +855,9 @@ static int __init net_ns_init(void) register_pernet_subsys(&net_ns_ops); - rtnl_register(PF_UNSPEC, RTM_NEWNSID, rtnl_net_newid, NULL, NULL); + rtnl_register(PF_UNSPEC, RTM_NEWNSID, rtnl_net_newid, NULL, 0); rtnl_register(PF_UNSPEC, RTM_GETNSID, rtnl_net_getid, rtnl_net_dumpid, - NULL); + 0); return 0; } diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 8c9d34deea7d..67607c540c03 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -178,7 +178,7 @@ static rtnl_dumpit_func rtnl_get_dumpit(int protocol, int msgindex) * @msgtype: rtnetlink message type * @doit: Function pointer called for each request message * @dumpit: Function pointer called for each dump request (NLM_F_DUMP) message - * @calcit: Function pointer to calc size of dump message + * @flags: rtnl_link_flags to modifiy behaviour of doit/dumpit functions * * Registers the specified function pointers (at least one of them has * to be non-NULL) to be called whenever a request message for the @@ -192,7 +192,7 @@ static rtnl_dumpit_func rtnl_get_dumpit(int protocol, int msgindex) */ int __rtnl_register(int protocol, int msgtype, rtnl_doit_func doit, rtnl_dumpit_func dumpit, - rtnl_calcit_func calcit) + unsigned int flags) { struct rtnl_link *tab; int msgindex; @@ -230,9 +230,9 @@ EXPORT_SYMBOL_GPL(__rtnl_register); */ void rtnl_register(int protocol, int msgtype, rtnl_doit_func doit, rtnl_dumpit_func dumpit, - rtnl_calcit_func calcit) + unsigned int flags) { - if (__rtnl_register(protocol, msgtype, doit, dumpit, calcit) < 0) + if (__rtnl_register(protocol, msgtype, doit, dumpit, flags) < 0) panic("Unable to register rtnetlink message handler, " "protocol = %d, message type = %d\n", protocol, msgtype); @@ -4279,23 +4279,23 @@ void __init rtnetlink_init(void) register_netdevice_notifier(&rtnetlink_dev_notifier); rtnl_register(PF_UNSPEC, RTM_GETLINK, rtnl_getlink, - rtnl_dump_ifinfo, NULL); - rtnl_register(PF_UNSPEC, RTM_SETLINK, rtnl_setlink, NULL, NULL); - rtnl_register(PF_UNSPEC, RTM_NEWLINK, rtnl_newlink, NULL, NULL); - rtnl_register(PF_UNSPEC, RTM_DELLINK, rtnl_dellink, NULL, NULL); + rtnl_dump_ifinfo, 0); + rtnl_register(PF_UNSPEC, RTM_SETLINK, rtnl_setlink, NULL, 0); + rtnl_register(PF_UNSPEC, RTM_NEWLINK, rtnl_newlink, NULL, 0); + rtnl_register(PF_UNSPEC, RTM_DELLINK, rtnl_dellink, NULL, 0); - rtnl_register(PF_UNSPEC, RTM_GETADDR, NULL, rtnl_dump_all, NULL); - rtnl_register(PF_UNSPEC, RTM_GETROUTE, NULL, rtnl_dump_all, NULL); - rtnl_register(PF_UNSPEC, RTM_GETNETCONF, NULL, rtnl_dump_all, NULL); + rtnl_register(PF_UNSPEC, RTM_GETADDR, NULL, rtnl_dump_all, 0); + rtnl_register(PF_UNSPEC, RTM_GETROUTE, NULL, rtnl_dump_all, 0); + rtnl_register(PF_UNSPEC, RTM_GETNETCONF, NULL, rtnl_dump_all, 0); - rtnl_register(PF_BRIDGE, RTM_NEWNEIGH, rtnl_fdb_add, NULL, NULL); - rtnl_register(PF_BRIDGE, RTM_DELNEIGH, rtnl_fdb_del, NULL, NULL); - rtnl_register(PF_BRIDGE, RTM_GETNEIGH, NULL, rtnl_fdb_dump, NULL); + rtnl_register(PF_BRIDGE, RTM_NEWNEIGH, rtnl_fdb_add, NULL, 0); + rtnl_register(PF_BRIDGE, RTM_DELNEIGH, rtnl_fdb_del, NULL, 0); + rtnl_register(PF_BRIDGE, RTM_GETNEIGH, NULL, rtnl_fdb_dump, 0); - rtnl_register(PF_BRIDGE, RTM_GETLINK, NULL, rtnl_bridge_getlink, NULL); - rtnl_register(PF_BRIDGE, RTM_DELLINK, rtnl_bridge_dellink, NULL, NULL); - rtnl_register(PF_BRIDGE, RTM_SETLINK, rtnl_bridge_setlink, NULL, NULL); + rtnl_register(PF_BRIDGE, RTM_GETLINK, NULL, rtnl_bridge_getlink, 0); + rtnl_register(PF_BRIDGE, RTM_DELLINK, rtnl_bridge_dellink, NULL, 0); + rtnl_register(PF_BRIDGE, RTM_SETLINK, rtnl_bridge_setlink, NULL, 0); rtnl_register(PF_UNSPEC, RTM_GETSTATS, rtnl_stats_get, rtnl_stats_dump, - NULL); + 0); } diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c index 733f523707ac..bae7d78aa068 100644 --- a/net/dcb/dcbnl.c +++ b/net/dcb/dcbnl.c @@ -1938,8 +1938,8 @@ static int __init dcbnl_init(void) { INIT_LIST_HEAD(&dcb_app_list); - rtnl_register(PF_UNSPEC, RTM_GETDCB, dcb_doit, NULL, NULL); - rtnl_register(PF_UNSPEC, RTM_SETDCB, dcb_doit, NULL, NULL); + rtnl_register(PF_UNSPEC, RTM_GETDCB, dcb_doit, NULL, 0); + rtnl_register(PF_UNSPEC, RTM_SETDCB, dcb_doit, NULL, 0); return 0; } diff --git a/net/decnet/dn_dev.c b/net/decnet/dn_dev.c index fa0110b57ca1..4d339de56862 100644 --- a/net/decnet/dn_dev.c +++ b/net/decnet/dn_dev.c @@ -1419,9 +1419,9 @@ void __init dn_dev_init(void) dn_dev_devices_on(); - rtnl_register(PF_DECnet, RTM_NEWADDR, dn_nl_newaddr, NULL, NULL); - rtnl_register(PF_DECnet, RTM_DELADDR, dn_nl_deladdr, NULL, NULL); - rtnl_register(PF_DECnet, RTM_GETADDR, NULL, dn_nl_dump_ifaddr, NULL); + rtnl_register(PF_DECnet, RTM_NEWADDR, dn_nl_newaddr, NULL, 0); + rtnl_register(PF_DECnet, RTM_DELADDR, dn_nl_deladdr, NULL, 0); + rtnl_register(PF_DECnet, RTM_GETADDR, NULL, dn_nl_dump_ifaddr, 0); proc_create("decnet_dev", S_IRUGO, init_net.proc_net, &dn_dev_seq_fops); diff --git a/net/decnet/dn_fib.c b/net/decnet/dn_fib.c index f9f6fb3f3c5b..3d37464c8b4a 100644 --- a/net/decnet/dn_fib.c +++ b/net/decnet/dn_fib.c @@ -791,8 +791,8 @@ void __init dn_fib_init(void) register_dnaddr_notifier(&dn_fib_dnaddr_notifier); - rtnl_register(PF_DECnet, RTM_NEWROUTE, dn_fib_rtm_newroute, NULL, NULL); - rtnl_register(PF_DECnet, RTM_DELROUTE, dn_fib_rtm_delroute, NULL, NULL); + rtnl_register(PF_DECnet, RTM_NEWROUTE, dn_fib_rtm_newroute, NULL, 0); + rtnl_register(PF_DECnet, RTM_DELROUTE, dn_fib_rtm_delroute, NULL, 0); } diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c index bcbe548f8854..0bd3afd01dd2 100644 --- a/net/decnet/dn_route.c +++ b/net/decnet/dn_route.c @@ -1922,10 +1922,10 @@ void __init dn_route_init(void) #ifdef CONFIG_DECNET_ROUTER rtnl_register(PF_DECnet, RTM_GETROUTE, dn_cache_getroute, - dn_fib_dump, NULL); + dn_fib_dump, 0); #else rtnl_register(PF_DECnet, RTM_GETROUTE, dn_cache_getroute, - dn_cache_dump, NULL); + dn_cache_dump, 0); #endif } diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c index 38d9af9b917c..d7adc0616599 100644 --- a/net/ipv4/devinet.c +++ b/net/ipv4/devinet.c @@ -2491,9 +2491,9 @@ void __init devinet_init(void) rtnl_af_register(&inet_af_ops); - rtnl_register(PF_INET, RTM_NEWADDR, inet_rtm_newaddr, NULL, NULL); - rtnl_register(PF_INET, RTM_DELADDR, inet_rtm_deladdr, NULL, NULL); - rtnl_register(PF_INET, RTM_GETADDR, NULL, inet_dump_ifaddr, NULL); + rtnl_register(PF_INET, RTM_NEWADDR, inet_rtm_newaddr, NULL, 0); + rtnl_register(PF_INET, RTM_DELADDR, inet_rtm_deladdr, NULL, 0); + rtnl_register(PF_INET, RTM_GETADDR, NULL, inet_dump_ifaddr, 0); rtnl_register(PF_INET, RTM_GETNETCONF, inet_netconf_get_devconf, - inet_netconf_dump_devconf, NULL); + inet_netconf_dump_devconf, 0); } diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c index 2cba559f14df..37819ab4cc74 100644 --- a/net/ipv4/fib_frontend.c +++ b/net/ipv4/fib_frontend.c @@ -1348,7 +1348,7 @@ void __init ip_fib_init(void) register_netdevice_notifier(&fib_netdev_notifier); register_inetaddr_notifier(&fib_inetaddr_notifier); - rtnl_register(PF_INET, RTM_NEWROUTE, inet_rtm_newroute, NULL, NULL); - rtnl_register(PF_INET, RTM_DELROUTE, inet_rtm_delroute, NULL, NULL); - rtnl_register(PF_INET, RTM_GETROUTE, NULL, inet_dump_fib, NULL); + rtnl_register(PF_INET, RTM_NEWROUTE, inet_rtm_newroute, NULL, 0); + rtnl_register(PF_INET, RTM_DELROUTE, inet_rtm_delroute, NULL, 0); + rtnl_register(PF_INET, RTM_GETROUTE, NULL, inet_dump_fib, 0); } diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c index 06863ea3fc5b..c9b3e6e069ae 100644 --- a/net/ipv4/ipmr.c +++ b/net/ipv4/ipmr.c @@ -3114,14 +3114,14 @@ int __init ip_mr_init(void) } #endif rtnl_register(RTNL_FAMILY_IPMR, RTM_GETROUTE, - ipmr_rtm_getroute, ipmr_rtm_dumproute, NULL); + ipmr_rtm_getroute, ipmr_rtm_dumproute, 0); rtnl_register(RTNL_FAMILY_IPMR, RTM_NEWROUTE, - ipmr_rtm_route, NULL, NULL); + ipmr_rtm_route, NULL, 0); rtnl_register(RTNL_FAMILY_IPMR, RTM_DELROUTE, - ipmr_rtm_route, NULL, NULL); + ipmr_rtm_route, NULL, 0); rtnl_register(RTNL_FAMILY_IPMR, RTM_GETLINK, - NULL, ipmr_rtm_dumplink, NULL); + NULL, ipmr_rtm_dumplink, 0); return 0; #ifdef CONFIG_IP_PIMSM_V2 diff --git a/net/ipv4/route.c b/net/ipv4/route.c index 0383e66f59bc..2ef46294475f 100644 --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -3067,7 +3067,7 @@ int __init ip_rt_init(void) xfrm_init(); xfrm4_init(); #endif - rtnl_register(PF_INET, RTM_GETROUTE, inet_rtm_getroute, NULL, NULL); + rtnl_register(PF_INET, RTM_GETROUTE, inet_rtm_getroute, NULL, 0); #ifdef CONFIG_SYSCTL register_pernet_subsys(&sysctl_route_ops); diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 30ee23eef268..640792e1ecb7 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -6605,21 +6605,21 @@ int __init addrconf_init(void) rtnl_af_register(&inet6_ops); err = __rtnl_register(PF_INET6, RTM_GETLINK, NULL, inet6_dump_ifinfo, - NULL); + 0); if (err < 0) goto errout; /* Only the first call to __rtnl_register can fail */ - __rtnl_register(PF_INET6, RTM_NEWADDR, inet6_rtm_newaddr, NULL, NULL); - __rtnl_register(PF_INET6, RTM_DELADDR, inet6_rtm_deladdr, NULL, NULL); + __rtnl_register(PF_INET6, RTM_NEWADDR, inet6_rtm_newaddr, NULL, 0); + __rtnl_register(PF_INET6, RTM_DELADDR, inet6_rtm_deladdr, NULL, 0); __rtnl_register(PF_INET6, RTM_GETADDR, inet6_rtm_getaddr, - inet6_dump_ifaddr, NULL); + inet6_dump_ifaddr, 0); __rtnl_register(PF_INET6, RTM_GETMULTICAST, NULL, - inet6_dump_ifmcaddr, NULL); + inet6_dump_ifmcaddr, 0); __rtnl_register(PF_INET6, RTM_GETANYCAST, NULL, - inet6_dump_ifacaddr, NULL); + inet6_dump_ifacaddr, 0); __rtnl_register(PF_INET6, RTM_GETNETCONF, inet6_netconf_get_devconf, - inet6_netconf_dump_devconf, NULL); + inet6_netconf_dump_devconf, 0); ipv6_addr_label_rtnl_register(); diff --git a/net/ipv6/addrlabel.c b/net/ipv6/addrlabel.c index 7a428f65c7ec..cea5eb488013 100644 --- a/net/ipv6/addrlabel.c +++ b/net/ipv6/addrlabel.c @@ -593,10 +593,10 @@ static int ip6addrlbl_get(struct sk_buff *in_skb, struct nlmsghdr *nlh, void __init ipv6_addr_label_rtnl_register(void) { __rtnl_register(PF_INET6, RTM_NEWADDRLABEL, ip6addrlbl_newdel, - NULL, NULL); + NULL, 0); __rtnl_register(PF_INET6, RTM_DELADDRLABEL, ip6addrlbl_newdel, - NULL, NULL); + NULL, 0); __rtnl_register(PF_INET6, RTM_GETADDRLABEL, ip6addrlbl_get, - ip6addrlbl_dump, NULL); + ip6addrlbl_dump, 0); } diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c index 69ed0043d117..8c58c7558de0 100644 --- a/net/ipv6/ip6_fib.c +++ b/net/ipv6/ip6_fib.c @@ -2038,7 +2038,7 @@ int __init fib6_init(void) goto out_kmem_cache_create; ret = __rtnl_register(PF_INET6, RTM_GETROUTE, NULL, inet6_dump_fib, - NULL); + 0); if (ret) goto out_unregister_subsys; diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c index 7454850f2098..f5500f5444e9 100644 --- a/net/ipv6/ip6mr.c +++ b/net/ipv6/ip6mr.c @@ -1427,7 +1427,7 @@ int __init ip6_mr_init(void) } #endif rtnl_register(RTNL_FAMILY_IP6MR, RTM_GETROUTE, NULL, - ip6mr_rtm_dumproute, NULL); + ip6mr_rtm_dumproute, 0); return 0; #ifdef CONFIG_IPV6_PIMSM_V2 add_proto_fail: diff --git a/net/ipv6/route.c b/net/ipv6/route.c index 7ecbe5eb19f8..0ddb966d2e13 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -4110,9 +4110,9 @@ int __init ip6_route_init(void) goto fib6_rules_init; ret = -ENOBUFS; - if (__rtnl_register(PF_INET6, RTM_NEWROUTE, inet6_rtm_newroute, NULL, NULL) || - __rtnl_register(PF_INET6, RTM_DELROUTE, inet6_rtm_delroute, NULL, NULL) || - __rtnl_register(PF_INET6, RTM_GETROUTE, inet6_rtm_getroute, NULL, NULL)) + if (__rtnl_register(PF_INET6, RTM_NEWROUTE, inet6_rtm_newroute, NULL, 0) || + __rtnl_register(PF_INET6, RTM_DELROUTE, inet6_rtm_delroute, NULL, 0) || + __rtnl_register(PF_INET6, RTM_GETROUTE, inet6_rtm_getroute, NULL, 0)) goto out_register_late_subsys; ret = register_netdevice_notifier(&ip6_route_dev_notifier); diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c index ea4f481839dd..c5b9ce41d66f 100644 --- a/net/mpls/af_mpls.c +++ b/net/mpls/af_mpls.c @@ -2479,12 +2479,12 @@ static int __init mpls_init(void) rtnl_af_register(&mpls_af_ops); - rtnl_register(PF_MPLS, RTM_NEWROUTE, mpls_rtm_newroute, NULL, NULL); - rtnl_register(PF_MPLS, RTM_DELROUTE, mpls_rtm_delroute, NULL, NULL); + rtnl_register(PF_MPLS, RTM_NEWROUTE, mpls_rtm_newroute, NULL, 0); + rtnl_register(PF_MPLS, RTM_DELROUTE, mpls_rtm_delroute, NULL, 0); rtnl_register(PF_MPLS, RTM_GETROUTE, mpls_getroute, mpls_dump_routes, - NULL); + 0); rtnl_register(PF_MPLS, RTM_GETNETCONF, mpls_netconf_get_devconf, - mpls_netconf_dump_devconf, NULL); + mpls_netconf_dump_devconf, 0); err = 0; out: return err; diff --git a/net/phonet/pn_netlink.c b/net/phonet/pn_netlink.c index 45b3af3080d8..da754fc926e7 100644 --- a/net/phonet/pn_netlink.c +++ b/net/phonet/pn_netlink.c @@ -300,15 +300,15 @@ static int route_dumpit(struct sk_buff *skb, struct netlink_callback *cb) int __init phonet_netlink_register(void) { int err = __rtnl_register(PF_PHONET, RTM_NEWADDR, addr_doit, - NULL, NULL); + NULL, 0); if (err) return err; /* Further __rtnl_register() cannot fail */ - __rtnl_register(PF_PHONET, RTM_DELADDR, addr_doit, NULL, NULL); - __rtnl_register(PF_PHONET, RTM_GETADDR, NULL, getaddr_dumpit, NULL); - __rtnl_register(PF_PHONET, RTM_NEWROUTE, route_doit, NULL, NULL); - __rtnl_register(PF_PHONET, RTM_DELROUTE, route_doit, NULL, NULL); - __rtnl_register(PF_PHONET, RTM_GETROUTE, NULL, route_dumpit, NULL); + __rtnl_register(PF_PHONET, RTM_DELADDR, addr_doit, NULL, 0); + __rtnl_register(PF_PHONET, RTM_GETADDR, NULL, getaddr_dumpit, 0); + __rtnl_register(PF_PHONET, RTM_NEWROUTE, route_doit, NULL, 0); + __rtnl_register(PF_PHONET, RTM_DELROUTE, route_doit, NULL, 0); + __rtnl_register(PF_PHONET, RTM_GETROUTE, NULL, route_dumpit, 0); return 0; } diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c index 5586609afa27..c2f5c13550c0 100644 --- a/net/qrtr/qrtr.c +++ b/net/qrtr/qrtr.c @@ -1081,7 +1081,7 @@ static int __init qrtr_proto_init(void) return rc; } - rtnl_register(PF_QIPCRTR, RTM_NEWADDR, qrtr_addr_doit, NULL, NULL); + rtnl_register(PF_QIPCRTR, RTM_NEWADDR, qrtr_addr_doit, NULL, 0); return 0; } diff --git a/net/sched/act_api.c b/net/sched/act_api.c index a2915d958279..02fcb0c78a28 100644 --- a/net/sched/act_api.c +++ b/net/sched/act_api.c @@ -1255,10 +1255,10 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb) static int __init tc_action_init(void) { - rtnl_register(PF_UNSPEC, RTM_NEWACTION, tc_ctl_action, NULL, NULL); - rtnl_register(PF_UNSPEC, RTM_DELACTION, tc_ctl_action, NULL, NULL); + rtnl_register(PF_UNSPEC, RTM_NEWACTION, tc_ctl_action, NULL, 0); + rtnl_register(PF_UNSPEC, RTM_DELACTION, tc_ctl_action, NULL, 0); rtnl_register(PF_UNSPEC, RTM_GETACTION, tc_ctl_action, tc_dump_action, - NULL); + 0); return 0; } diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 8d1157aebaf7..ebeeb87e6d44 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -1010,10 +1010,10 @@ EXPORT_SYMBOL(tcf_exts_get_dev); static int __init tc_filter_init(void) { - rtnl_register(PF_UNSPEC, RTM_NEWTFILTER, tc_ctl_tfilter, NULL, NULL); - rtnl_register(PF_UNSPEC, RTM_DELTFILTER, tc_ctl_tfilter, NULL, NULL); + rtnl_register(PF_UNSPEC, RTM_NEWTFILTER, tc_ctl_tfilter, NULL, 0); + rtnl_register(PF_UNSPEC, RTM_DELTFILTER, tc_ctl_tfilter, NULL, 0); rtnl_register(PF_UNSPEC, RTM_GETTFILTER, tc_ctl_tfilter, - tc_dump_tfilter, NULL); + tc_dump_tfilter, 0); return 0; } diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c index bd24a550e0f9..816c8092e601 100644 --- a/net/sched/sch_api.c +++ b/net/sched/sch_api.c @@ -1952,14 +1952,14 @@ static int __init pktsched_init(void) register_qdisc(&mq_qdisc_ops); register_qdisc(&noqueue_qdisc_ops); - rtnl_register(PF_UNSPEC, RTM_NEWQDISC, tc_modify_qdisc, NULL, NULL); - rtnl_register(PF_UNSPEC, RTM_DELQDISC, tc_get_qdisc, NULL, NULL); + rtnl_register(PF_UNSPEC, RTM_NEWQDISC, tc_modify_qdisc, NULL, 0); + rtnl_register(PF_UNSPEC, RTM_DELQDISC, tc_get_qdisc, NULL, 0); rtnl_register(PF_UNSPEC, RTM_GETQDISC, tc_get_qdisc, tc_dump_qdisc, - NULL); - rtnl_register(PF_UNSPEC, RTM_NEWTCLASS, tc_ctl_tclass, NULL, NULL); - rtnl_register(PF_UNSPEC, RTM_DELTCLASS, tc_ctl_tclass, NULL, NULL); + 0); + rtnl_register(PF_UNSPEC, RTM_NEWTCLASS, tc_ctl_tclass, NULL, 0); + rtnl_register(PF_UNSPEC, RTM_DELTCLASS, tc_ctl_tclass, NULL, 0); rtnl_register(PF_UNSPEC, RTM_GETTCLASS, tc_ctl_tclass, tc_dump_tclass, - NULL); + 0); return 0; } -- 2.13.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 net-next 3/7] rtnetlink: add reference counting to prevent module unload while dump is in progress 2017-08-09 18:41 [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl Florian Westphal 2017-08-09 18:41 ` [PATCH v2 net-next 1/7] rtnetlink: call rtnl_calcit directly Florian Westphal 2017-08-09 18:41 ` [PATCH v2 net-next 2/7] rtnetlink: make rtnl_register accept a flags parameter Florian Westphal @ 2017-08-09 18:41 ` Florian Westphal 2017-08-09 18:41 ` [PATCH v2 net-next 4/7] rtnetlink: small rtnl lock pushdown Florian Westphal ` (4 subsequent siblings) 7 siblings, 0 replies; 13+ messages in thread From: Florian Westphal @ 2017-08-09 18:41 UTC (permalink / raw) To: netdev; +Cc: Florian Westphal I don't see what prevents rmmod (unregister_all is called) while a dump is active. Even if we'd add rtnl lock/unlock pair to unregister_all (as done here), thats not enough either as rtnl_lock is released right before the dump process starts. So this adds a refcount: * acquire rtnl mutex * bump refcount * release mutex * start the dump ... and make unregister_all remove the callbacks (no new dumps possible) and then wait until refcount is 0. Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org> --- No changes since v1. net/core/rtnetlink.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 67607c540c03..c45a7c5e3232 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -127,6 +127,7 @@ EXPORT_SYMBOL(lockdep_rtnl_is_held); #endif /* #ifdef CONFIG_PROVE_LOCKING */ static struct rtnl_link *rtnl_msg_handlers[RTNL_FAMILY_MAX + 1]; +static refcount_t rtnl_msg_handlers_ref[RTNL_FAMILY_MAX + 1]; static inline int rtm_msgindex(int msgtype) { @@ -272,10 +273,18 @@ EXPORT_SYMBOL_GPL(rtnl_unregister); */ void rtnl_unregister_all(int protocol) { + struct rtnl_link *handlers; + BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX); - kfree(rtnl_msg_handlers[protocol]); + rtnl_lock(); + handlers = rtnl_msg_handlers[protocol]; rtnl_msg_handlers[protocol] = NULL; + rtnl_unlock(); + + while (refcount_read(&rtnl_msg_handlers_ref[protocol]) > 0) + schedule(); + kfree(handlers); } EXPORT_SYMBOL_GPL(rtnl_unregister_all); @@ -4173,6 +4182,8 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, if (dumpit == NULL) return -EOPNOTSUPP; + refcount_inc(&rtnl_msg_handlers_ref[family]); + if (type == RTM_GETLINK) min_dump_alloc = rtnl_calcit(skb, nlh); @@ -4186,6 +4197,7 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, err = netlink_dump_start(rtnl, skb, nlh, &c); } rtnl_lock(); + refcount_dec(&rtnl_msg_handlers_ref[family]); return err; } -- 2.13.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 net-next 4/7] rtnetlink: small rtnl lock pushdown 2017-08-09 18:41 [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl Florian Westphal ` (2 preceding siblings ...) 2017-08-09 18:41 ` [PATCH v2 net-next 3/7] rtnetlink: add reference counting to prevent module unload while dump is in progress Florian Westphal @ 2017-08-09 18:41 ` Florian Westphal 2017-08-09 18:41 ` [PATCH v2 net-next 5/7] rtnetlink: protect handler table with rcu Florian Westphal ` (3 subsequent siblings) 7 siblings, 0 replies; 13+ messages in thread From: Florian Westphal @ 2017-08-09 18:41 UTC (permalink / raw) To: netdev; +Cc: Florian Westphal instead of rtnl lock/unload at the top level, push it down to the called function. This is just an intermediate step, next commit switches protection of the rtnl_link ops table to rcu, in which case (for dumps) the rtnl lock is acquired only by the netlink dumper infrastructure (current lock/unlock/dump/lock/unlock rtnl sequence becomes rcu lock/rcu unlock/dump). Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org> --- No changes since v1. net/core/rtnetlink.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index c45a7c5e3232..be01d8e48661 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -4178,9 +4178,11 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, rtnl_dumpit_func dumpit; u16 min_dump_alloc = 0; + rtnl_lock(); + dumpit = rtnl_get_dumpit(family, type); if (dumpit == NULL) - return -EOPNOTSUPP; + goto err_unlock; refcount_inc(&rtnl_msg_handlers_ref[family]); @@ -4196,23 +4198,28 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, }; err = netlink_dump_start(rtnl, skb, nlh, &c); } - rtnl_lock(); refcount_dec(&rtnl_msg_handlers_ref[family]); return err; } + rtnl_lock(); doit = rtnl_get_doit(family, type); if (doit == NULL) - return -EOPNOTSUPP; + goto err_unlock; - return doit(skb, nlh, extack); + err = doit(skb, nlh, extack); + rtnl_unlock(); + + return err; + +err_unlock: + rtnl_unlock(); + return -EOPNOTSUPP; } static void rtnetlink_rcv(struct sk_buff *skb) { - rtnl_lock(); netlink_rcv_skb(skb, &rtnetlink_rcv_msg); - rtnl_unlock(); } static int rtnetlink_bind(struct net *net, int group) -- 2.13.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 net-next 5/7] rtnetlink: protect handler table with rcu 2017-08-09 18:41 [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl Florian Westphal ` (3 preceding siblings ...) 2017-08-09 18:41 ` [PATCH v2 net-next 4/7] rtnetlink: small rtnl lock pushdown Florian Westphal @ 2017-08-09 18:41 ` Florian Westphal 2017-08-10 8:26 ` Ido Schimmel 2017-08-09 18:41 ` [PATCH v2 net-next 6/7] rtnetlink: add RTNL_FLAG_DOIT_UNLOCKED Florian Westphal ` (2 subsequent siblings) 7 siblings, 1 reply; 13+ messages in thread From: Florian Westphal @ 2017-08-09 18:41 UTC (permalink / raw) To: netdev; +Cc: Florian Westphal Note that netlink dumps still acquire rtnl mutex via the netlink dump infrastructure. Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org> --- No changes since v1. net/core/rtnetlink.c | 121 +++++++++++++++++++++++++++------------------------ 1 file changed, 65 insertions(+), 56 deletions(-) diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index be01d8e48661..d45946177bc8 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -126,7 +126,7 @@ bool lockdep_rtnl_is_held(void) EXPORT_SYMBOL(lockdep_rtnl_is_held); #endif /* #ifdef CONFIG_PROVE_LOCKING */ -static struct rtnl_link *rtnl_msg_handlers[RTNL_FAMILY_MAX + 1]; +static struct rtnl_link __rcu *rtnl_msg_handlers[RTNL_FAMILY_MAX + 1]; static refcount_t rtnl_msg_handlers_ref[RTNL_FAMILY_MAX + 1]; static inline int rtm_msgindex(int msgtype) @@ -143,36 +143,6 @@ static inline int rtm_msgindex(int msgtype) return msgindex; } -static rtnl_doit_func rtnl_get_doit(int protocol, int msgindex) -{ - struct rtnl_link *tab; - - if (protocol <= RTNL_FAMILY_MAX) - tab = rtnl_msg_handlers[protocol]; - else - tab = NULL; - - if (tab == NULL || tab[msgindex].doit == NULL) - tab = rtnl_msg_handlers[PF_UNSPEC]; - - return tab[msgindex].doit; -} - -static rtnl_dumpit_func rtnl_get_dumpit(int protocol, int msgindex) -{ - struct rtnl_link *tab; - - if (protocol <= RTNL_FAMILY_MAX) - tab = rtnl_msg_handlers[protocol]; - else - tab = NULL; - - if (tab == NULL || tab[msgindex].dumpit == NULL) - tab = rtnl_msg_handlers[PF_UNSPEC]; - - return tab[msgindex].dumpit; -} - /** * __rtnl_register - Register a rtnetlink message type * @protocol: Protocol family or PF_UNSPEC @@ -201,18 +171,17 @@ int __rtnl_register(int protocol, int msgtype, BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX); msgindex = rtm_msgindex(msgtype); - tab = rtnl_msg_handlers[protocol]; + tab = rcu_dereference(rtnl_msg_handlers[protocol]); if (tab == NULL) { tab = kcalloc(RTM_NR_MSGTYPES, sizeof(*tab), GFP_KERNEL); if (tab == NULL) return -ENOBUFS; - rtnl_msg_handlers[protocol] = tab; + rcu_assign_pointer(rtnl_msg_handlers[protocol], tab); } if (doit) tab[msgindex].doit = doit; - if (dumpit) tab[msgindex].dumpit = dumpit; @@ -249,16 +218,22 @@ EXPORT_SYMBOL_GPL(rtnl_register); */ int rtnl_unregister(int protocol, int msgtype) { + struct rtnl_link *handlers; int msgindex; BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX); msgindex = rtm_msgindex(msgtype); - if (rtnl_msg_handlers[protocol] == NULL) + rtnl_lock(); + handlers = rtnl_dereference(rtnl_msg_handlers[protocol]); + if (!handlers) { + rtnl_unlock(); return -ENOENT; + } - rtnl_msg_handlers[protocol][msgindex].doit = NULL; - rtnl_msg_handlers[protocol][msgindex].dumpit = NULL; + handlers[msgindex].doit = NULL; + handlers[msgindex].dumpit = NULL; + rtnl_unlock(); return 0; } @@ -278,10 +253,12 @@ void rtnl_unregister_all(int protocol) BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX); rtnl_lock(); - handlers = rtnl_msg_handlers[protocol]; - rtnl_msg_handlers[protocol] = NULL; + handlers = rtnl_dereference(rtnl_msg_handlers[protocol]); + RCU_INIT_POINTER(rtnl_msg_handlers[protocol], NULL); rtnl_unlock(); + synchronize_net(); + while (refcount_read(&rtnl_msg_handlers_ref[protocol]) > 0) schedule(); kfree(handlers); @@ -2820,11 +2797,13 @@ static u16 rtnl_calcit(struct sk_buff *skb, struct nlmsghdr *nlh) * traverse the list of net devices and compute the minimum * buffer size based upon the filter mask. */ - list_for_each_entry(dev, &net->dev_base_head, dev_list) { + rcu_read_lock(); + for_each_netdev_rcu(net, dev) { min_ifinfo_dump_size = max_t(u16, min_ifinfo_dump_size, if_nlmsg_size(dev, ext_filter_mask)); } + rcu_read_unlock(); return nlmsg_total_size(min_ifinfo_dump_size); } @@ -2836,19 +2815,29 @@ static int rtnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb) if (s_idx == 0) s_idx = 1; + for (idx = 1; idx <= RTNL_FAMILY_MAX; idx++) { int type = cb->nlh->nlmsg_type-RTM_BASE; + struct rtnl_link *handlers; + rtnl_dumpit_func dumpit; + if (idx < s_idx || idx == PF_PACKET) continue; - if (rtnl_msg_handlers[idx] == NULL || - rtnl_msg_handlers[idx][type].dumpit == NULL) + + handlers = rtnl_dereference(rtnl_msg_handlers[idx]); + if (!handlers) continue; + + dumpit = READ_ONCE(handlers[type].dumpit); + if (!dumpit) + continue; + if (idx > s_idx) { memset(&cb->args[0], 0, sizeof(cb->args)); cb->prev_seq = 0; cb->seq = 0; } - if (rtnl_msg_handlers[idx][type].dumpit(skb, cb)) + if (dumpit(skb, cb)) break; } cb->family = idx; @@ -4151,11 +4140,12 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, struct netlink_ext_ack *extack) { struct net *net = sock_net(skb->sk); + struct rtnl_link *handlers; + int err = -EOPNOTSUPP; rtnl_doit_func doit; int kind; int family; int type; - int err; type = nlh->nlmsg_type; if (type > RTM_MAX) @@ -4173,23 +4163,40 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, if (kind != 2 && !netlink_net_capable(skb, CAP_NET_ADMIN)) return -EPERM; + if (family > ARRAY_SIZE(rtnl_msg_handlers)) + family = PF_UNSPEC; + + rcu_read_lock(); + handlers = rcu_dereference(rtnl_msg_handlers[family]); + if (!handlers) { + family = PF_UNSPEC; + handlers = rcu_dereference(rtnl_msg_handlers[family]); + } + if (kind == 2 && nlh->nlmsg_flags&NLM_F_DUMP) { struct sock *rtnl; rtnl_dumpit_func dumpit; u16 min_dump_alloc = 0; - rtnl_lock(); + dumpit = READ_ONCE(handlers[type].dumpit); + if (!dumpit) { + family = PF_UNSPEC; + handlers = rcu_dereference(rtnl_msg_handlers[PF_UNSPEC]); + if (!handlers) + goto err_unlock; - dumpit = rtnl_get_dumpit(family, type); - if (dumpit == NULL) - goto err_unlock; + dumpit = READ_ONCE(handlers[type].dumpit); + if (!dumpit) + goto err_unlock; + } refcount_inc(&rtnl_msg_handlers_ref[family]); if (type == RTM_GETLINK) min_dump_alloc = rtnl_calcit(skb, nlh); - __rtnl_unlock(); + rcu_read_unlock(); + rtnl = net->rtnl; { struct netlink_dump_control c = { @@ -4202,18 +4209,20 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, return err; } - rtnl_lock(); - doit = rtnl_get_doit(family, type); - if (doit == NULL) - goto err_unlock; + rcu_read_unlock(); - err = doit(skb, nlh, extack); + rtnl_lock(); + handlers = rtnl_dereference(rtnl_msg_handlers[family]); + if (handlers) { + doit = READ_ONCE(handlers[type].doit); + if (doit) + err = doit(skb, nlh, extack); + } rtnl_unlock(); - return err; err_unlock: - rtnl_unlock(); + rcu_read_unlock(); return -EOPNOTSUPP; } -- 2.13.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 net-next 5/7] rtnetlink: protect handler table with rcu 2017-08-09 18:41 ` [PATCH v2 net-next 5/7] rtnetlink: protect handler table with rcu Florian Westphal @ 2017-08-10 8:26 ` Ido Schimmel 0 siblings, 0 replies; 13+ messages in thread From: Ido Schimmel @ 2017-08-10 8:26 UTC (permalink / raw) To: Florian Westphal; +Cc: netdev On Wed, Aug 09, 2017 at 08:41:51PM +0200, Florian Westphal wrote: > Note that netlink dumps still acquire rtnl mutex via the netlink > dump infrastructure. > > Signed-off-by: Florian Westphal <fw@strlen.de> > Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org> Florian, can you please check the following splat? I didn't bisect, but looks like it's related to this patch. Thanks [ 2.940093] ============================= [ 2.944145] WARNING: suspicious RCU usage [ 2.948147] 4.13.0-rc4-idosch-next-custom #565 Not tainted [ 2.954116] ----------------------------- [ 2.958113] net/core/rtnetlink.c:175 suspicious rcu_dereference_check() usage! [ 2.965116] [ 2.965116] other info that might help us debug this: [ 2.965116] [ 2.973123] [ 2.973123] rcu_scheduler_active = 1, debug_locks = 1 [ 2.979119] no locks held by swapper/0/1. [ 2.983109] [ 2.983109] stack backtrace: [ 2.988117] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.13.0-rc4-idosch-next-custom #565 [ 2.989000] Hardware name: Mellanox Technologies Ltd. Mellanox switch/Mellanox switch, BIOS 4.6.5 05/21/2015 [ 2.989000] Call Trace: [ 2.989000] dump_stack+0xb1/0x10c [ 2.989000] ? _atomic_dec_and_lock+0x124/0x124 [ 2.989000] ? lockdep_print_held_locks+0x9b/0xf0 [ 2.989000] lockdep_rcu_suspicious+0xdd/0x110 [ 2.989000] __rtnl_register+0x1c1/0x320 [ 2.989000] ? rtnetlink_bind+0x50/0x50 [ 2.989000] ? net_defaults_init+0x29/0x29 [ 2.989000] rtnl_register+0x11/0x30 [ 2.989000] net_ns_init+0x1bd/0x1fd [ 2.989000] ? net_defaults_init+0x29/0x29 [ 2.989000] ? __init_srcu_struct+0x54/0x60 [ 2.989000] ? srcu_init_notifier_head+0x4a/0x80 [ 2.989000] do_one_initcall+0x9a/0x241 [ 2.989000] ? initcall_blacklisted+0x140/0x140 [ 2.989000] ? up_read+0x40/0x40 [ 2.989000] ? _raw_spin_unlock_irqrestore+0x3d/0x60 [ 2.989000] ? __wake_up+0x3f/0x50 [ 2.989000] kernel_init_freeable+0x3ea/0x485 [ 2.989000] ? rest_init+0x100/0x100 [ 2.989000] kernel_init+0xe/0x140 [ 2.989000] ? rest_init+0x100/0x100 [ 2.989000] ? rest_init+0x100/0x100 [ 2.989000] ret_from_fork+0x27/0x40 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 net-next 6/7] rtnetlink: add RTNL_FLAG_DOIT_UNLOCKED 2017-08-09 18:41 [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl Florian Westphal ` (4 preceding siblings ...) 2017-08-09 18:41 ` [PATCH v2 net-next 5/7] rtnetlink: protect handler table with rcu Florian Westphal @ 2017-08-09 18:41 ` Florian Westphal 2017-08-09 18:41 ` [PATCH v2 net-next 7/7] net: call newid/getid without rtnl mutex held Florian Westphal 2017-08-10 0:21 ` [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl David Miller 7 siblings, 0 replies; 13+ messages in thread From: Florian Westphal @ 2017-08-09 18:41 UTC (permalink / raw) To: netdev; +Cc: Florian Westphal Allow callers to tell rtnetlink core that its doit callback should be invoked without holding rtnl mutex. Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org> --- change since v1: don't make ipv6 route rtnl handlers lockless include/net/rtnetlink.h | 4 ++++ net/core/rtnetlink.c | 15 +++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h index ac32460a0adb..21837ca68ecc 100644 --- a/include/net/rtnetlink.h +++ b/include/net/rtnetlink.h @@ -8,6 +8,10 @@ typedef int (*rtnl_doit_func)(struct sk_buff *, struct nlmsghdr *, struct netlink_ext_ack *); typedef int (*rtnl_dumpit_func)(struct sk_buff *, struct netlink_callback *); +enum rtnl_link_flags { + RTNL_FLAG_DOIT_UNLOCKED = 1, +}; + int __rtnl_register(int protocol, int msgtype, rtnl_doit_func, rtnl_dumpit_func, unsigned int flags); void rtnl_register(int protocol, int msgtype, diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index d45946177bc8..dd4e50dfa248 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -62,6 +62,7 @@ struct rtnl_link { rtnl_doit_func doit; rtnl_dumpit_func dumpit; + unsigned int flags; }; static DEFINE_MUTEX(rtnl_mutex); @@ -184,6 +185,7 @@ int __rtnl_register(int protocol, int msgtype, tab[msgindex].doit = doit; if (dumpit) tab[msgindex].dumpit = dumpit; + tab[msgindex].flags |= flags; return 0; } @@ -233,6 +235,7 @@ int rtnl_unregister(int protocol, int msgtype) handlers[msgindex].doit = NULL; handlers[msgindex].dumpit = NULL; + handlers[msgindex].flags = 0; rtnl_unlock(); return 0; @@ -4143,6 +4146,7 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, struct rtnl_link *handlers; int err = -EOPNOTSUPP; rtnl_doit_func doit; + unsigned int flags; int kind; int family; int type; @@ -4209,6 +4213,17 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, return err; } + flags = READ_ONCE(handlers[type].flags); + if (flags & RTNL_FLAG_DOIT_UNLOCKED) { + refcount_inc(&rtnl_msg_handlers_ref[family]); + doit = READ_ONCE(handlers[type].doit); + rcu_read_unlock(); + if (doit) + err = doit(skb, nlh, extack); + refcount_dec(&rtnl_msg_handlers_ref[family]); + return err; + } + rcu_read_unlock(); rtnl_lock(); -- 2.13.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 net-next 7/7] net: call newid/getid without rtnl mutex held 2017-08-09 18:41 [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl Florian Westphal ` (5 preceding siblings ...) 2017-08-09 18:41 ` [PATCH v2 net-next 6/7] rtnetlink: add RTNL_FLAG_DOIT_UNLOCKED Florian Westphal @ 2017-08-09 18:41 ` Florian Westphal 2017-08-10 0:21 ` [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl David Miller 7 siblings, 0 replies; 13+ messages in thread From: Florian Westphal @ 2017-08-09 18:41 UTC (permalink / raw) To: netdev; +Cc: Florian Westphal Both functions take nsid_lock and don't rely on rtnl lock. Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Hannes Frederic Sowa <hannes@stressinduktion.org> --- No changes since v1. net/core/net_namespace.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c index a7f06d706aa0..6cfdc7c84c48 100644 --- a/net/core/net_namespace.c +++ b/net/core/net_namespace.c @@ -855,9 +855,10 @@ static int __init net_ns_init(void) register_pernet_subsys(&net_ns_ops); - rtnl_register(PF_UNSPEC, RTM_NEWNSID, rtnl_net_newid, NULL, 0); + rtnl_register(PF_UNSPEC, RTM_NEWNSID, rtnl_net_newid, NULL, + RTNL_FLAG_DOIT_UNLOCKED); rtnl_register(PF_UNSPEC, RTM_GETNSID, rtnl_net_getid, rtnl_net_dumpid, - 0); + RTNL_FLAG_DOIT_UNLOCKED); return 0; } -- 2.13.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl 2017-08-09 18:41 [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl Florian Westphal ` (6 preceding siblings ...) 2017-08-09 18:41 ` [PATCH v2 net-next 7/7] net: call newid/getid without rtnl mutex held Florian Westphal @ 2017-08-10 0:21 ` David Miller 2017-08-10 1:27 ` David Ahern 7 siblings, 1 reply; 13+ messages in thread From: David Miller @ 2017-08-10 0:21 UTC (permalink / raw) To: fw; +Cc: netdev From: Florian Westphal <fw@strlen.de> Date: Wed, 9 Aug 2017 20:41:46 +0200 > Changes since v1: > In patch 6, don't make ipv6 route handlers lockless, they all have > assumptions on rtnl being held. Other patches are unchanged. > > The RTNL mutex is used to serialize both rtnetlink calls and > dump requests. > Its also used to protect other things such as the list of current > net namespaces. > > Unfortunately RTNL mutex is a performance issue, e.g. a cpu adding an > ip address prevents other cpus from seemingly unrelated tasks such as > dumping tc classifiers or doing rtnetlink route lookups. > > This patch set adds basic infrastructure to start pushing the rtnl lock > down to those places that need it, or even elide it entirely in some cases. > > Subsystems can now indicate that their doit() callback can run without > RTNL mutex, such callbacks can then run in parallel. > > This will obviously need a lot of followup work; all current > users need to be audited/changed to benefit from this. > Initial no-rtnl spot is netns new/getid. > > We have various 'get' handlers that are also a tempting target, > however, several of these depend on rtnl mutex to prevent information > from changing while objects are being read by rtnl handlers; however, > it doesn't appear impossible to change this. > > Dumps are another problem entirely, see > commit 2907c35ff64708065 ("net: hold rtnl again in dump callbacks"), > this patchset doesn't touch dump requests. Ok series applied, let's see where this goes :-) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl 2017-08-10 0:21 ` [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl David Miller @ 2017-08-10 1:27 ` David Ahern 2017-08-10 11:29 ` Florian Westphal 0 siblings, 1 reply; 13+ messages in thread From: David Ahern @ 2017-08-10 1:27 UTC (permalink / raw) To: David Miller, fw; +Cc: netdev On 8/9/17 6:21 PM, David Miller wrote: > > Ok series applied, let's see where this goes :-) > 1 hour in, 1 problem reported ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl 2017-08-10 1:27 ` David Ahern @ 2017-08-10 11:29 ` Florian Westphal 2017-08-10 16:23 ` David Ahern 0 siblings, 1 reply; 13+ messages in thread From: Florian Westphal @ 2017-08-10 11:29 UTC (permalink / raw) To: David Ahern; +Cc: netdev David Ahern <dsahern@gmail.com> wrote: > On 8/9/17 6:21 PM, David Miller wrote: > > > > Ok series applied, let's see where this goes :-) > > > > 1 hour in, 1 problem reported Its even worse. Would you rather see a revert? I'm sure that you are aware that the widespread rtnl usage is a problem because it does hurt scalability. I still think its worth trying to disentangle this where possible. I should have fixes for the reported fallout soon. Alternatively, I can send a revert plus patch to add ASSERT_RTNL in one more spot. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl 2017-08-10 11:29 ` Florian Westphal @ 2017-08-10 16:23 ` David Ahern 0 siblings, 0 replies; 13+ messages in thread From: David Ahern @ 2017-08-10 16:23 UTC (permalink / raw) To: Florian Westphal; +Cc: netdev On 8/10/17 5:29 AM, Florian Westphal wrote: > David Ahern <dsahern@gmail.com> wrote: > >> On 8/9/17 6:21 PM, David Miller wrote: >>> >>> Ok series applied, let's see where this goes :-) >>> >> >> 1 hour in, 1 problem reported > > Its even worse. Would you rather see a revert? no. > > I'm sure that you are aware that the widespread rtnl usage is a > problem because it does hurt scalability. I still think its worth trying > to disentangle this where possible. agreed. Going to be pain along the way, but it needs to be done. Thanks for taking the initiative on it. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-08-10 16:23 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-08-09 18:41 [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl Florian Westphal 2017-08-09 18:41 ` [PATCH v2 net-next 1/7] rtnetlink: call rtnl_calcit directly Florian Westphal 2017-08-09 18:41 ` [PATCH v2 net-next 2/7] rtnetlink: make rtnl_register accept a flags parameter Florian Westphal 2017-08-09 18:41 ` [PATCH v2 net-next 3/7] rtnetlink: add reference counting to prevent module unload while dump is in progress Florian Westphal 2017-08-09 18:41 ` [PATCH v2 net-next 4/7] rtnetlink: small rtnl lock pushdown Florian Westphal 2017-08-09 18:41 ` [PATCH v2 net-next 5/7] rtnetlink: protect handler table with rcu Florian Westphal 2017-08-10 8:26 ` Ido Schimmel 2017-08-09 18:41 ` [PATCH v2 net-next 6/7] rtnetlink: add RTNL_FLAG_DOIT_UNLOCKED Florian Westphal 2017-08-09 18:41 ` [PATCH v2 net-next 7/7] net: call newid/getid without rtnl mutex held Florian Westphal 2017-08-10 0:21 ` [PATCH v2 net-next 0/7] rtnetlink: allow selected handlers to run without rtnl David Miller 2017-08-10 1:27 ` David Ahern 2017-08-10 11:29 ` Florian Westphal 2017-08-10 16:23 ` David Ahern
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).