All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: David Lamparter <equinox@diac24.net>
Cc: Daniel Lezcano <daniel.lezcano@free.fr>,
	netdev@vger.kernel.org, Patrick McHardy <kaber@trash.net>
Subject: Re: [PATCH RFC] netns: introduce NETREG_NETNS_MOVING reg_state
Date: Wed, 25 Aug 2010 10:39:01 -0700	[thread overview]
Message-ID: <m139u2fv7u.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <20100825135254.GC235835@jupiter.n2.diac24.net> (David Lamparter's message of "Wed, 25 Aug 2010 15:52:55 +0200")

David Lamparter <equinox@diac24.net> writes:

> On Wed, Aug 25, 2010 at 03:03:00PM +0200, Daniel Lezcano wrote:
>> > Please note that i'm quite unsure about the correctness of this
>> > patch due to me not having much experience in this kind of
>> > modification...
>> >
>> > In particular, the following might get broken by the new reg_state:
>> > drivers/net/ixgbe/ixgbe_main.c (hot-unplug)
>> > drivers/net/wimax/i2400m/driver.c (device reset)
>> >    
>> 
>> IMHO, that should be ok. NETREG_NETNS_MOVING is a transient state where 
>> the network device is not registered and not in the netdev list.
>> 
>> I think you should take care of this kind of comparison:
>> 
>> net/core/net-sysfs.c
>> 
>> static inline int dev_isalive(const struct net_device *dev)
>> {
>>          return dev->reg_state <= NETREG_REGISTERED;
>> }
>> 
>> Not sure having NETREG_NETNS_MOVING < NETREG_REGISTERED is right.
>
> I can't say I fully understand the code and all of its implications, but
> I think all of the attributes sysfs provides access to can theoretically
> be safely accessed while the device is moving.
>
>> > What could be done, alternatively, is split the notification:
>> >   * NETDEV_UNREGISTER - now changed to mean "logical unregister"
>> >   * NETDEV_UNREGISTER_PHYSICAL - _not_ sent by netns move, means
>> >      "the device is going for good"
>> >    
>> 
>> Maybe I miss something but that will have a big impact to all the 
>> network stacks no ?
>
> No; since NETDEV_UNREGISTER is still sent in all places where it is
> currently sent, none of the stacks would require changing. However,
> users of the actual, physical device (independent of its network
> namespace) would need changing to NETDEV_UNREGISTER_PHYSICAL, which is a
> new notification that is only sent if the device really disappears.
>
> TBH, I'm not quite sure which is the better solution here;
> NETDEV_UNREGISTER_PHYSICAL or NETREG_NETNS_MOVING...
> ... or the initial one?

Before we get too far down any of these paths, what is it that caused
you to notice that moving the physical device broke the connection to
the vlan and macvlan devices?

I just want to understand this case a little better before we walk down
any of these paths.

Eric

  reply	other threads:[~2010-08-25 17:39 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-24 11:50 [PATCH RFC] netns: keep vlan slaves on master netns move David Lamparter
2010-08-24 17:00 ` Daniel Lezcano
2010-08-24 19:14   ` Eric W. Biederman
2010-08-25 11:52     ` [PATCH RFC] netns: introduce NETREG_NETNS_MOVING reg_state David Lamparter
2010-08-25 13:03       ` Daniel Lezcano
2010-08-25 13:52         ` David Lamparter
2010-08-25 17:39           ` Eric W. Biederman [this message]
2010-08-26  8:21             ` David Lamparter
2010-09-13 11:43 ` [PATCH RFC] netns: keep vlan slaves on master netns move David Lamparter
2010-09-15  2:10   ` Eric W. Biederman
2010-09-17 12:49     ` Patrick McHardy
2010-09-27 16:23       ` Eric W. Biederman
2010-09-17 13:22     ` [PATCH] " David Lamparter
2010-09-17 17:56       ` David Miller
2010-09-15  6:47   ` [PATCH RFC] " Daniel Lezcano

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=m139u2fv7u.fsf@fess.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=daniel.lezcano@free.fr \
    --cc=equinox@diac24.net \
    --cc=kaber@trash.net \
    --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.