All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Mahesh Bandewar <mahesh@bandewar.net>
Cc: James Morris <jmorris@namei.org>,
	Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
	Patrick McHardy <kaber@trash.net>,
	David Miller <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	netdev <netdev@vger.kernel.org>,
	Mahesh Bandewar <maheshb@google.com>
Subject: Re: [PATCH 1/2] ipv4: initialize fib_trie prior to register_netdev_notifier call.
Date: Wed, 05 Jul 2017 11:24:22 -0500	[thread overview]
Message-ID: <87lgo2g8cp.fsf@xmission.com> (raw)
In-Reply-To: <20170704191620.6503-1-mahesh@bandewar.net> (Mahesh Bandewar's message of "Tue, 4 Jul 2017 12:16:20 -0700")

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

  reply	other threads:[~2017-07-05 16:32 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-04 19:16 [PATCH 1/2] ipv4: initialize fib_trie prior to register_netdev_notifier call Mahesh Bandewar
2017-07-05 16:24 ` Eric W. Biederman [this message]
2017-07-05 17:23   ` Mahesh Bandewar (महेश बंडेवार)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87lgo2g8cp.fsf@xmission.com \
    --to=ebiederm@xmission.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jmorris@namei.org \
    --cc=kaber@trash.net \
    --cc=mahesh@bandewar.net \
    --cc=maheshb@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=yoshfuji@linux-ipv6.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.