netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jay Vosburgh <fubar@us.ibm.com>
To: Ben Greear <greearb@candelatech.com>
Cc: Patrick McHardy <kaber@trash.net>,
	"David S. Miller" <davem@davemloft.net>,
	Linux Netdev List <netdev@vger.kernel.org>
Subject: Re: vlan: update vlan carrier state for admin up/down
Date: Mon, 27 Apr 2009 16:55:58 -0700	[thread overview]
Message-ID: <13072.1240876558@death.nxdomain.ibm.com> (raw)
In-Reply-To: <49F4BCEB.6040507@candelatech.com>

Ben Greear <greearb@candelatech.com> wrote:

>Jay Vosburgh wrote:
>>> If so, I think that is not a good idea and could possibly be considered
>>> a security issue.  I think instead there should be a new flag for VLANs
>>> that is 'preferred-admin-state'.  To be UP, both the underlying device
>>> and the preferred state must be UP.  That should allow bouncing eth0
>>> w/out affecting the eventual admin state of the VLANs on eth0 in
>>> the example above.
>>>     
>>
>> 	I dunno if it's a security issue, but I'd agree it's wrong.  I
>> looked, and I suspect that a couple of judiciously placed "if
>> (dev->flags & IFF_UP)" bits ought to sort things out by simply not
>> copying the carrier state to the upper level VLAN device if that device
>> is down.  Unless somebody works this up over the weekend, I'll work that
>> out on Monday.
>>   
>That may be fine.  I suppose you'll also be ignoring admin state changes
>for the
>underlying devices in the VLAN code?  Ie, if eth0 goes admin down, you won't
>change the eth0.5 vlan to be admin down?

	Ok, I did a bit of testing on this, and I don't believe the
"update vlan carrier state" patch I did changed anything in this regard.

	Without the patch, given a configuration of eth0 with eth0.600
and eth0.700 stacked above, the sequence "ifconfig eth0.700 down;
ifconfig eth0 down; ifconfig eth0 up" results in eth0.700 being up at
the end, and both eth0.600 and eth0.700 being down when eth0 is down (up
and down here referring to IFF_UP, not carrier state).  The VLAN event
handler (vlan_device_event) deliberately sets all VLAN devices to match
the real device when the real device goes up or down.

	I still agree that this is a bit counterintuitive, but the patch
doesn't change the VLAN behavior in this regard.

	With the patch, the same thing happens, but the carrier state
also changes to match (without the patch, the VLAN interfaces remain
carrier up the whole time).

	I'm reluctant to mess with this behavior without knowing why it
works this way; there may be a good reason for it that I'm not aware of.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

  reply	other threads:[~2009-04-27 23:55 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-24 15:44 vlan: update vlan carrier state for admin up/down Patrick McHardy
2009-04-24 16:39 ` Ben Greear
2009-04-25  0:31   ` Jay Vosburgh
2009-04-26 19:58     ` Ben Greear
2009-04-27 23:55       ` Jay Vosburgh [this message]
2009-04-28  1:36         ` David Miller
2009-04-30  7:48           ` Ben Greear
2009-05-05 12:41           ` Patrick McHardy
2009-04-26  1:06 ` David Miller

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=13072.1240876558@death.nxdomain.ibm.com \
    --to=fubar@us.ibm.com \
    --cc=davem@davemloft.net \
    --cc=greearb@candelatech.com \
    --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 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).