Message ID | 55F1A6AE.2010804@gmail.com |
---|---|
State | RFC, archived |
Delegated to: | David Miller |
Headers | show |
On Thu, Sep 10, 2015 at 11:50:06AM -0400, Vlad Yasevich wrote: > On 09/10/2015 10:22 AM, Marcelo Ricardo Leitner wrote: > > Em 10-09-2015 10:24, Vlad Yasevich escreveu: ... > >> Then you can order sctp_net_init() such that it happens first, then protosw registration > >> happens, then control socket initialization happens, then inet protocol registration > >> happens. > >> > >> This way, we are always guaranteed that by the time user calls socket(), protocol > >> defaults are fully initialized. > > > > Okay, that works for module loading stage, but then how would we handle new netns's? We > > have to create the control socket per netns and AFAICT sctp_net_init() is the only hook > > called when a new netns is being created. > > > > Then if we move it a workqueue that is scheduled by sctp_net_init(), we loose the ability > > to handle its errors by propagating through sctp_net_init() return value, not good. > > Here is kind of what I had in mind. It's incomplete and completely untested (not even > compiled), but good enough to describe the idea: ... Ohh, ok now I get it, thanks. If having two pernet_subsys for a given module is fine, that works for me. It's clearer and has no moment of temporary failure. I can finish this patch if everybody agrees with it. > >>> I used the list pointer because that's null as that memory is entirely zeroed when alloced > >>> and, after initialization, it's never null again. Works like a lock/condition without > >>> using an extra field. > >>> > >> > >> I understand this a well. What I don't particularly like is that we are re-using > >> a list without really stating why it's now done this way. Additionally, it's not really > >> the last that happens so it's seems kind of hacky... If we need to add new > >> per-net initializers, we now need to make sure that the code is put in the right > >> place. I'd just really like to have a cleaner solution... > > > > Ok, got you. We could add a dedicated flag/bit for that then, if reusing the list is not > > clear enough. Or, as we are discussing on the other part of thread, we could make it block > > and wait for the initialization, probably using some wait_queue. I'm still thinking on > > something this way, likely something more below than sctp then. > > > > I think if we don the above, the second process calling socket() will either find the > the protosw or will try to load the module also. I think either is ok after > request_module returns we'll look at the protosw and will find find things. Seems so, yes. Nice. Marcelo -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c index 59e8035..970bdce 100644 --- a/net/sctp/protocol.c +++ b/net/sctp/protocol.c @@ -1166,7 +1166,7 @@ static void sctp_v4_del_protocol(void) unregister_inetaddr_notifier(&sctp_inetaddr_notifier); } -static int __net_init sctp_net_init(struct net *net) +static int __net_init sctp_defauls_init(struct net *net) { int status; @@ -1259,12 +1259,6 @@ static int __net_init sctp_net_init(struct net *net) sctp_dbg_objcnt_init(net); - /* Initialize the control inode/socket for handling OOTB packets. */ - if ((status = sctp_ctl_sock_init(net))) { - pr_err("Failed to initialize the SCTP control sock\n"); - goto err_ctl_sock_init; - } - /* Initialize the local address list. */ INIT_LIST_HEAD(&net->sctp.local_addr_list); spin_lock_init(&net->sctp.local_addr_lock); @@ -1291,15 +1285,12 @@ err_sysctl_register: return status; } -static void __net_exit sctp_net_exit(struct net *net) +static void __net_exit sctp_defautls_exit(struct net *net) { /* Free the local address list */ sctp_free_addr_wq(net); sctp_free_local_addr_list(net); - /* Free the control endpoint. */ - inet_ctl_sock_destroy(net->sctp.ctl_sock); - sctp_dbg_objcnt_exit(net); sctp_proc_exit(net); @@ -1307,9 +1298,31 @@ static void __net_exit sctp_net_exit(struct net *net) sctp_sysctl_net_unregister(net); } -static struct pernet_operations sctp_net_ops = { - .init = sctp_net_init, - .exit = sctp_net_exit, +static struct pernet_operations sctp_defaults_ops = { + .init = sctp_net_defaults_init, + .exit = sctp_net_defaults_exit, +}; + +static int __net_init sctp_net_ctrlsock_init(struct net *net) +{ + int status; + + /* Initialize the control inode/socket for handling OOTB packets. */ + if ((status = sctp_ctl_sock_init(net))) + pr_err("Failed to initialize the SCTP control sock\n"); + + return status; +} + +static void __net_exit sctp_defautls_exit(struct net *net) +{ + /* Free the control endpoint. */ + inet_ctl_sock_destroy(net->sctp.ctl_sock); +} + +static struct pernet_operations sctp_ctrlsock_opts = { + .init = sctp_net_ctrlsock_init, + .exit = sctp_net_ctrlsock_exit, }; /* Initialize the universe into something sensible. */ @@ -1442,6 +1455,10 @@ static __init int sctp_init(void)