| Message ID | 20170704191620.6503-1-mahesh@bandewar.net |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
Mahesh Bandewar <mahesh@bandewar.net> writes: > From: Mahesh Bandewar <maheshb@google.com> > > Net stack initialization currently initializes fib-trie after the > first call to netdevice_notifier() call. It does not cause any problem > since there are no devices UP at this moment, but trying to bring 'lo' > UP at initialization would make this assumption wrong. However changing > order solves the issue. This looks like a real issue and you are part of the way to a real fix. The principle being you should not register things notifications until you are ready to handle them. As such fib_trie_init (which allocates the slabs) needs to come before rtnl_register. As a rtnl message can trigger slab allocation. I have not traced it through but I suspect register_pernet_subsys(&fib_net_ops) also needs to come before rtnl_register. Sigh. It looks like this patch can be labeled: Fixes: 7b1a74fdbb9e ("[NETNS]: Refactor fib initialization so it can handle multiple namespaces.") Fixes: 7f9b80529b8a ("[IPV4]: fib hash|trie initialization") So I really think the code needs to say: void __init ip_fib_init(void) { fib_trie_init(); register_pernet_subsys(&fib_net_ops); 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); } Eric
On Wed, Jul 5, 2017 at 9:24 AM, Eric W. Biederman <ebiederm@xmission.com> wrote: > Mahesh Bandewar <mahesh@bandewar.net> writes: > >> From: Mahesh Bandewar <maheshb@google.com> >> >> Net stack initialization currently initializes fib-trie after the >> first call to netdevice_notifier() call. It does not cause any problem >> since there are no devices UP at this moment, but trying to bring 'lo' >> UP at initialization would make this assumption wrong. However changing >> order solves the issue. > > This looks like a real issue and you are part of the way to a real fix. > The principle being you should not register things notifications until > you are ready to handle them. > > As such fib_trie_init (which allocates the slabs) needs to come before > rtnl_register. As a rtnl message can trigger slab allocation. > > I have not traced it through but I suspect > register_pernet_subsys(&fib_net_ops) also needs to come before > rtnl_register. > > Sigh. It looks like this patch can be labeled: > > Fixes: 7b1a74fdbb9e ("[NETNS]: Refactor fib initialization so it can handle multiple namespaces.") > Fixes: 7f9b80529b8a ("[IPV4]: fib hash|trie initialization") > > So I really think the code needs to say: > > void __init ip_fib_init(void) > { > fib_trie_init(); > register_pernet_subsys(&fib_net_ops); > > 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); > } > Will update in the next version. thanks, --mahesh.. > Eric
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c index 4e678fa892dd..ca22ca47019a 100644 --- a/net/ipv4/fib_frontend.c +++ b/net/ipv4/fib_frontend.c @@ -1338,9 +1338,9 @@ void __init ip_fib_init(void) rtnl_register(PF_INET, RTM_DELROUTE, inet_rtm_delroute, NULL, NULL); rtnl_register(PF_INET, RTM_GETROUTE, NULL, inet_dump_fib, NULL); + fib_trie_init(); + register_pernet_subsys(&fib_net_ops); register_netdevice_notifier(&fib_netdev_notifier); register_inetaddr_notifier(&fib_inetaddr_notifier); - - fib_trie_init(); }