All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roopa Prabhu <roopa@cumulusnetworks.com>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Mahesh Bandewar <maheshb@google.com>,
	netdev <netdev@vger.kernel.org>,
	David Miller <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Subject: Re: [PATCH net-next] rtnetlink: delay RTM_DELLINK notification until after ndo_uninit()
Date: Tue, 02 Dec 2014 09:19:29 -0800	[thread overview]
Message-ID: <547DF4A1.6070900@cumulusnetworks.com> (raw)
In-Reply-To: <1417539160.5303.73.camel@edumazet-glaptop2.roam.corp.google.com>

On 12/2/14, 8:52 AM, Eric Dumazet wrote:
> On Tue, 2014-12-02 at 08:41 -0800, Roopa Prabhu wrote:
>
>> fair point. But the commit that moved things around was done to handle
>> cases where,
>> the ndo_uninit() already sends some notifications to userspace for the
>> changes
>> during uninit (example bond driver).
>>
>> The only point i was making was that the dellink after the ndo_uninit in
>> your
>> case now contains state that was prior to uninit for these drivers.
> I think Mahesh forgot to mention your patch probably broke some drivers.
>
> calling rtmsg_ifinfo() after uninit() is probably breaking dummy device,
> as it does :
>
> static void dummy_dev_uninit(struct net_device *dev)
> {
> 	free_percpu(dev->dstats);
> }
>
> It looks like 'fixing' ipvlan is not going to help.
>
> Instead of checking all drivers for such interesting side effects,
> and revert your patch, we had the idea of this solution.

ok fair enough. I could go through all drivers again and check and fix them.
but dont want to miss any such cases.
The patch idea is good.
Worth a comment in the source regarding the state in the dellink. 
Hopefully the drivers that are affected by this are very few.

Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>

Thanks!.

      reply	other threads:[~2014-12-02 17:19 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-02  5:54 [PATCH net-next] rtnetlink: delay RTM_DELLINK notification until after ndo_uninit() Mahesh Bandewar
2014-12-02 10:07 ` Thomas Graf
2014-12-02 19:53   ` Alexei Starovoitov
2014-12-09  1:44   ` David Miller
2014-12-02 16:08 ` Roopa Prabhu
2014-12-02 16:19   ` Eric Dumazet
2014-12-02 16:41     ` Roopa Prabhu
2014-12-02 16:52       ` Eric Dumazet
2014-12-02 17:19         ` Roopa Prabhu [this message]

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=547DF4A1.6070900@cumulusnetworks.com \
    --to=roopa@cumulusnetworks.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=eric.dumazet@gmail.com \
    --cc=maheshb@google.com \
    --cc=makita.toshiaki@lab.ntt.co.jp \
    --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.