Linux-Wireless Archive on lore.kernel.org
 help / color / Atom feed
From: Luis Chamberlain <mcgrof@kernel.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: johannes@sipsolutions.net, derosier@gmail.com,
	greearb@candelatech.com, jeyu@kernel.org,
	akpm@linux-foundation.org, arnd@arndb.de, rostedt@goodmis.org,
	mingo@redhat.com, aquini@redhat.com, cai@lca.pw,
	dyoung@redhat.com, bhe@redhat.com, peterz@infradead.org,
	tglx@linutronix.de, gpiccoli@canonical.com, pmladek@suse.com,
	tiwai@suse.de, schlad@suse.de, andriy.shevchenko@linux.intel.com,
	keescook@chromium.org, daniel.vetter@ffwll.ch, will@kernel.org,
	mchehab+samsung@kernel.org, kvalo@codeaurora.org,
	davem@davemloft.net, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
	ath10k@lists.infradead.org, jiri@resnulli.us,
	briannorris@chromium.org
Subject: Re: [RFC 1/2] devlink: add simple fw crash helpers
Date: Fri, 22 May 2020 21:49:29 +0000
Message-ID: <20200522214929.GB11244@42.do-not-panic.com> (raw)
In-Reply-To: <20200522101738.1495f4cc@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

On Fri, May 22, 2020 at 10:17:38AM -0700, Jakub Kicinski wrote:
> On Fri, 22 May 2020 05:20:46 +0000 Luis Chamberlain wrote:
> > > diff --git a/net/core/Makefile b/net/core/Makefile
> > > index 3e2c378e5f31..6f1513781c17 100644
> > > --- a/net/core/Makefile
> > > +++ b/net/core/Makefile
> > > @@ -31,7 +31,7 @@ obj-$(CONFIG_LWTUNNEL_BPF) += lwt_bpf.o
> > >  obj-$(CONFIG_BPF_STREAM_PARSER) += sock_map.o
> > >  obj-$(CONFIG_DST_CACHE) += dst_cache.o
> > >  obj-$(CONFIG_HWBM) += hwbm.o
> > > -obj-$(CONFIG_NET_DEVLINK) += devlink.o
> > > +obj-$(CONFIG_NET_DEVLINK) += devlink.o devlink_simple_fw_reporter.o  
> > 
> > This was looking super sexy up to here. This is networking specific.
> > We want something generic for *anything* that requests firmware.
> 
> You can't be serious. It's network specific because of how the Kconfig
> is named?

Kconfig? What has that to do with anything? The issue I have is that the
solution I am looking for is for it to be agnostic to the subsystem. I
have found similar firmware crashes on gpu, media, scsci.

> Working for a company operating large data centers I would strongly
> prefer if we didn't have ten different ways of reporting firmware
> problems in the fleet.

Indeed.

> > I'm afraid this won't work for something generic. I don't think its
> > throw-away work though, the idea to provide a generic interface to
> > dump firmware through netlink might be nice for networking, or other
> > things.
> > 
> > But I have a feeling we'll want something still more generic than this.
> 
> Please be specific. Saying generic a lot is not helpful. The code (as
> you can see in this patch) is in no way network specific. Or are you
> saying there are machines out there running without netlink sockets?

No, I am saying I want something to work with any struct device.

> > So networking may want to be aware that a firmware crash happened as
> > part of this network device health thing, but firmware crashing is a
> > generic thing.
> > 
> > I have now extended my patch set to include uvents and I am more set on
> > that we need the taint now more than ever.
> 
> Please expect my nack if you're trying to add this to networking
> drivers.

The uevent mechanism is not for networking.

The taint however is, and I'd like to undertand how it is you do not see
that an undesirable requirement for a reboot is a clear case for a taint.

> The irony is you have a problem with a networking device and all the
> devices your initial set touched are networking. Two of the drivers 
> you touched either have or will soon have devlink health reporters
> implemented.

That is all great, and I don't think its a bad idea to add
infrastructure / extend it to get more information about a firmware
crash dump. However, suggesting that devlink is the only solution we
need in the kernel without considering other subsystems is what I am
suggesting doesn't suit my needs. Networking was just the first
subsystem I am taclking now but I have patches where similar situations
happen across the kernel.

  Luis

  parent reply index

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200515212846.1347-1-mcgrof@kernel.org>
2020-05-15 21:28 ` [PATCH v2 12/15] ath10k: use new module_firmware_crashed() Luis Chamberlain
2020-05-16  4:11   ` Rafael Aquini
2020-05-16 13:24   ` Johannes Berg
2020-05-16 13:50     ` Johannes Berg
2020-05-18 16:56       ` Luis Chamberlain
2020-05-19  1:23       ` Brian Norris
2020-05-19 14:02         ` Luis Chamberlain
2020-05-20  0:47           ` Brian Norris
2020-05-20  5:37             ` Emmanuel Grumbach
2020-05-20  8:32               ` Andy Shevchenko
2020-05-21 19:01               ` Brian Norris
2020-05-22  5:12                 ` Emmanuel Grumbach
2020-05-22  5:23                   ` Luis Chamberlain
2020-05-18 16:51     ` Luis Chamberlain
2020-05-18 16:58       ` Ben Greear
2020-05-18 17:09         ` Luis Chamberlain
2020-05-18 17:15           ` Ben Greear
2020-05-18 17:18             ` Luis Chamberlain
2020-05-18 18:06               ` Steve deRosier
2020-05-18 19:09                 ` Luis Chamberlain
2020-05-18 19:25                   ` Johannes Berg
2020-05-18 19:59                     ` Luis Chamberlain
2020-05-18 20:07                       ` Johannes Berg
2020-05-18 21:18                         ` Luis Chamberlain
2020-05-18 20:28                     ` Jakub Kicinski
2020-05-18 20:29                       ` Johannes Berg
2020-05-18 20:35                         ` Jakub Kicinski
2020-05-18 20:41                           ` Johannes Berg
2020-05-18 20:46                             ` Jakub Kicinski
2020-05-18 21:22                               ` Luis Chamberlain
2020-05-18 22:16                                 ` Jakub Kicinski
2020-05-19  1:05                                   ` Luis Chamberlain
2020-05-19 21:15                                     ` [RFC 1/2] devlink: add simple fw crash helpers Jakub Kicinski
2020-05-22  5:20                                       ` Luis Chamberlain
2020-05-22 17:17                                         ` Jakub Kicinski
2020-05-22 20:46                                           ` Johannes Berg
2020-05-22 21:51                                             ` Luis Chamberlain
2020-05-22 23:23                                               ` Steve deRosier
2020-05-22 23:44                                                 ` Luis Chamberlain
2020-05-25  9:07                                                 ` Andy Shevchenko
2020-05-25 17:08                                                   ` Ben Greear
2020-05-25 20:57                                             ` Jakub Kicinski
2020-05-22 21:49                                           ` Luis Chamberlain [this message]
2020-05-19 21:15                                     ` [RFC 2/2] i2400m: use devlink health reporter Jakub Kicinski
2020-05-15 21:28 ` [PATCH v2 13/15] ath6kl: use new module_firmware_crashed() Luis Chamberlain
2020-05-16  4:12   ` Rafael Aquini
2020-05-15 21:28 ` [PATCH v2 14/15] brcm80211: " Luis Chamberlain
2020-05-16  4:13   ` Rafael Aquini
2020-05-15 21:28 ` [PATCH v2 15/15] mwl8k: " Luis Chamberlain
2020-05-16  4:13   ` Rafael Aquini

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=20200522214929.GB11244@42.do-not-panic.com \
    --to=mcgrof@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=aquini@redhat.com \
    --cc=arnd@arndb.de \
    --cc=ath10k@lists.infradead.org \
    --cc=bhe@redhat.com \
    --cc=briannorris@chromium.org \
    --cc=cai@lca.pw \
    --cc=daniel.vetter@ffwll.ch \
    --cc=davem@davemloft.net \
    --cc=derosier@gmail.com \
    --cc=dyoung@redhat.com \
    --cc=gpiccoli@canonical.com \
    --cc=greearb@candelatech.com \
    --cc=jeyu@kernel.org \
    --cc=jiri@resnulli.us \
    --cc=johannes@sipsolutions.net \
    --cc=keescook@chromium.org \
    --cc=kuba@kernel.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=mchehab+samsung@kernel.org \
    --cc=mingo@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=schlad@suse.de \
    --cc=tglx@linutronix.de \
    --cc=tiwai@suse.de \
    --cc=will@kernel.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

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org
	public-inbox-index linux-wireless

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wireless


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git