All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Antoine Tenart <atenart@kernel.org>
Cc: davem@davemloft.net, alexander.duyck@gmail.com,
	netdev@vger.kernel.org, Michal Kubecek <mkubecek@suse.cz>
Subject: Re: [PATCH net v2] net: avoid registering new queue objects after device unregistration
Date: Mon, 22 Nov 2021 11:37:42 -0800	[thread overview]
Message-ID: <20211122113742.16705a54@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <163760016601.3195.14673180551504824633@kwain>

On Mon, 22 Nov 2021 17:56:06 +0100 Antoine Tenart wrote:
> Quoting Jakub Kicinski (2021-11-22 17:31:44)
> > On Mon, 22 Nov 2021 17:20:07 +0100 Antoine Tenart wrote:  
> > >    veth_set_channels+0x1c3/0x550
> > >    ethnl_set_channels+0x524/0x610  
> > 
> > The patch looks fine, but ethtool calls should not happen after
> > unregister IMHO. Too many drivers would be surprised. 
> > 
> > Maybe we should catch unregistered devices in ethnl_ops_begin()?  
> 
> That might be good to have indeed, I'll have a look. I'm not sure about
> other paths that could do the same: if there are, we might want to keep
> the check in this patch as well. Or WARN to catch future misuses?

My knee jerk reaction was to add a WARN() just because I can't think
why changing queue count after unregister would be needed. But it's not
really illegal and we do support it before register, so why not after
unregister..

We can venture a warning with a comment saying that this is just for
catching bad uses and see if any driver hits it on a normal path?

  reply	other threads:[~2021-11-22 19:37 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-22 16:20 [PATCH net v2] net: avoid registering new queue objects after device unregistration Antoine Tenart
2021-11-22 16:31 ` Jakub Kicinski
2021-11-22 16:56   ` Antoine Tenart
2021-11-22 19:37     ` Jakub Kicinski [this message]
2021-11-23 11:18       ` Antoine Tenart

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=20211122113742.16705a54@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
    --to=kuba@kernel.org \
    --cc=alexander.duyck@gmail.com \
    --cc=atenart@kernel.org \
    --cc=davem@davemloft.net \
    --cc=mkubecek@suse.cz \
    --cc=netdev@vger.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
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.