netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Calvin Owens <calvinowens@fb.com>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: <davem@davemloft.net>, <shm@cumulusnetworks.com>,
	<izumi.taku@jp.fujitsu.com>, <linville@tuxdriver.com>,
	<dsa@cumulusnetworks.com>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <kernel-team@fb.com>
Subject: Re: [PATCH] netconsole: Initialize after all core networking drivers
Date: Thu, 17 Dec 2015 17:46:15 -0800	[thread overview]
Message-ID: <20151218014615.GB1715233@devbig337.prn1.facebook.com> (raw)
In-Reply-To: <1450400894.8474.114.camel@edumazet-glaptop2.roam.corp.google.com>

On Thursday 12/17 at 17:08 -0800, Eric Dumazet wrote:
> On Thu, 2015-12-17 at 15:52 -0800, Calvin Owens wrote:
> > With built-in netconsole and IXGBE, configuring netconsole via the kernel
> > cmdline results in the following panic at boot:
> > 
> >     netpoll: netconsole: device eth0 not up yet, forcing it
> >     usb 2-1: new high-speed USB device number 2 using ehci-pci
> >     ixgbe 0000:03:00.0: registered PHC device on eth0
> >     BUG: unable to handle kernel NULL pointer dereference at 0000000000000810
> >     <snip>
> >     Call Trace:
> >      [<ffffffff81578661>] ? vxlan_get_rx_port+0x41/0xa0
> >      [<ffffffff81586828>] ixgbe_open+0x4e8/0x540
> >      [<ffffffff8168045c>] __dev_open+0xac/0x120
> >      [<ffffffff81680506>] dev_open+0x36/0x70
> >      [<ffffffff8169abec>] netpoll_setup+0x23c/0x300
> >      [<ffffffff8169a66a>] ? netpoll_parse_options+0x19a/0x200
> >      [<ffffffff81d797a8>] ? option_setup+0x1f/0x1f
> >      [<ffffffff81d79882>] init_netconsole+0xda/0x262
> >      [<ffffffff81d797a8>] ? option_setup+0x1f/0x1f
> >      [<ffffffff810003a8>] do_one_initcall+0x88/0x1b0
> >      [<ffffffff81d31144>] kernel_init_freeable+0x14a/0x1e3
> >      [<ffffffff81d308f1>] ? do_early_param+0x8c/0x8c
> >      [<ffffffff81778610>] ? rest_init+0x80/0x80
> >      [<ffffffff8177861e>] kernel_init+0xe/0xe0
> >      [<ffffffff8177dc5f>] ret_from_fork+0x3f/0x70
> >      [<ffffffff81778610>] ? rest_init+0x80/0x80
> > 
> > This happens because IXGBE assumes that vxlan has already been initialized.
> > The cleanest way to fix this is to just initialize netconsole after all the
> > other core networking stuff has completed.
> > 
> > Signed-off-by: Calvin Owens <calvinowens@fb.com>
> > ---
> >  drivers/net/Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> > index 900b0c5..31557d0 100644
> > --- a/drivers/net/Makefile
> > +++ b/drivers/net/Makefile
> > @@ -15,7 +15,6 @@ obj-$(CONFIG_MACVTAP) += macvtap.o
> >  obj-$(CONFIG_MII) += mii.o
> >  obj-$(CONFIG_MDIO) += mdio.o
> >  obj-$(CONFIG_NET) += Space.o loopback.o
> > -obj-$(CONFIG_NETCONSOLE) += netconsole.o
> >  obj-$(CONFIG_PHYLIB) += phy/
> >  obj-$(CONFIG_RIONET) += rionet.o
> >  obj-$(CONFIG_NET_TEAM) += team/
> > @@ -26,6 +25,7 @@ obj-$(CONFIG_VXLAN) += vxlan.o
> >  obj-$(CONFIG_GENEVE) += geneve.o
> >  obj-$(CONFIG_NLMON) += nlmon.o
> >  obj-$(CONFIG_NET_VRF) += vrf.o
> > +obj-$(CONFIG_NETCONSOLE) += netconsole.o
> >  
> >  #
> >  # Networking Drivers
> 
> 
> Looks odd to rely on link order, but we might already rely on this...
> 
> Have you considered using device_initcall() instead of late_initcall()
> in vxlan ?

I'll look. As-is though, I think a similar problem would happen if you
tried to use a virtio_net device with netconsole= cmdline (although that
is admittedly a bizarre use case). The Makefile patch seemed like the
best way to ensure this can't recur elsewhere.

> In any case, a comment would really be good to avoid future mistakes.

Good point, I'll add something.

  reply	other threads:[~2015-12-18  1:46 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-17 23:52 [PATCH] netconsole: Initialize after all core networking drivers Calvin Owens
2015-12-18  1:08 ` Eric Dumazet
2015-12-18  1:46   ` Calvin Owens [this message]
2015-12-23 23:03     ` Calvin Owens
2015-12-24  6:54       ` [PATCH] vxlan: Properly depend on ipv6 and revert to module_init() kbuild test robot
2015-12-24 10:25       ` [PATCH] netconsole: Initialize after all core networking drivers Hannes Frederic Sowa
2015-12-28 23:21         ` Cong Wang
2015-12-29 13:59           ` Hannes Frederic Sowa
2015-12-28 23:24       ` Cong Wang
2015-12-18  1:10 ` Stephen Hemminger
2015-12-18  1:39   ` Calvin Owens

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=20151218014615.GB1715233@devbig337.prn1.facebook.com \
    --to=calvinowens@fb.com \
    --cc=davem@davemloft.net \
    --cc=dsa@cumulusnetworks.com \
    --cc=eric.dumazet@gmail.com \
    --cc=izumi.taku@jp.fujitsu.com \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=netdev@vger.kernel.org \
    --cc=shm@cumulusnetworks.com \
    /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 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).