From: Stefan Priebe - Profihost AG <s.priebe@profihost•ag>
To: Vlad Yasevich <vyasevich@gmail•com>,
Veaceslav Falico <vfalico@redhat•com>,
Vlad Yasevich <vyasevic@redhat•com>
Cc: Linux Netdev List <netdev@vger•kernel.org>
Subject: Re: how to mix bridges and bonding inc. vlans correctly on Kernel > 3.10
Date: Thu, 14 Nov 2013 15:29:57 +0100 [thread overview]
Message-ID: <5284DE65.40105@profihost.ag> (raw)
In-Reply-To: <5284DDCE.4090901@gmail.com>
Am 14.11.2013 15:27, schrieb Vlad Yasevich:
> On 11/14/2013 06:54 AM, Veaceslav Falico wrote:
>> On Wed, Nov 13, 2013 at 10:09:30PM -0500, Vlad Yasevich wrote:
>>> Can you try out attached patch please. I ran it in my environment and
>>> always get promisc link when attaching devices to the bridge.
>>
>> It's interesting, actually, why do we need to check for IFF_UP at all
>> when
>> changing flags.
>>
>> The addition of IFF_UP goes up to:
>>
>> commit b6c40d68ff6498b7f63ddf97cf0aa818d748dee7
>> Author: Patrick McHardy <kaber@trash•net>
>> Date: Tue Oct 7 15:26:48 2008 -0700
>>
>> net: only invoke dev->change_rx_flags when device is UP
>>
>> Jesper Dangaard Brouer <hawk@comx•dk> reported a bug when setting a
>> VLAN
>> device down that is in promiscous mode:
>>
>> When the VLAN device is set down, the promiscous count on the real
>> device is decremented by one by vlan_dev_stop(). When removing the
>> promiscous flag from the VLAN device afterwards, the promiscous
>> count on the real device is decremented a second time by the
>> vlan_change_rx_flags() callback.
>>
>> ... snip ...
>>
>
> This was applied in 2.6.27 timeframe.
>
>> However, I'm not sure that this is still needed, cause:
>>
>> commit deede2fabe24e00bd7e246eb81cd5767dc6fcfc7
>> Author: Matthijs Kooijman <matthijs@stdin•nl>
>> Date: Mon Oct 31 04:53:13 2011 +0000
>>
>> vlan: Don't propagate flag changes on down interfaces.
>>
>> When (de)configuring a vlan interface, the IFF_ALLMULTI ans
>> IFF_PROMISC
>> flags are cleared or set on the underlying interface. So, if these
>> flags
>> are changed on a vlan interface that is not up, the flags underlying
>> interface might be set or cleared twice.
>>
>> Only propagating flag changes when a device is up makes sure this
>> does
>> not happen. It also makes sure that an underlying device is not
>> set to
>> promiscuous or allmulti mode for a vlan device that is down.
>>
>> ... snip ...
>>
>
> And this in 3.2. So how did this work for Stefan in 3.9? Unless there
> is something in Ubuntu that changed how the stacked interfaces are
> configured and is keeping interfaces down longer then it used to.
Stop here ;-) in 3.9 it was the other way round.
This one worked:
> eth2
> \
> -- bond1 -- vmbr1
> / \
> eth3 \ vmbr1.3000
> \ ---- tap114i1
this one did not:
> eth2
> \
> -- bond1 -- vmbr1
> / \
> eth3 ----- bond1.3000 --- vmbr1v3000
> \ ---- tap114i1
Now with the patch - the 2nd one works the first one does not.
Stefan
>> which fixed completely the initial issue with vlans.
>>
>> Maybe we should just remove the IFF_UP check in dev_change_rx_flags(),
>> and
>> let the drivers using it handle its own logic, as vlan does?
>>
>
> This should work, but this might regress a few drivers. Looking through
> the callers of dev_set_promiscuity at least DSA driver suffers
> the same fate as VLAN.
>
> -vlad
>
>> Something like this:
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 8ffc52e..9615cd7 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -4995,7 +4995,7 @@ static void dev_change_rx_flags(struct net_device
>> *dev, int flags)
>> {
>> const struct net_device_ops *ops = dev->netdev_ops;
>>
>> - if ((dev->flags & IFF_UP) && ops->ndo_change_rx_flags)
>> + if (ops->ndo_change_rx_flags)
>> ops->ndo_change_rx_flags(dev, flags);
>> }
>>
>>
>>>
>>> Thanks
>>> -vlad
>
next prev parent reply other threads:[~2013-11-14 14:30 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-13 13:58 how to mix bridges and bonding inc. vlans correctly on Kernel > 3.10 Stefan Priebe - Profihost AG
2013-11-13 14:12 ` Veaceslav Falico
2013-11-13 14:20 ` Stefan Priebe - Profihost AG
2013-11-13 14:34 ` Veaceslav Falico
2013-11-13 14:43 ` Stefan Priebe - Profihost AG
2013-11-13 15:05 ` Vlad Yasevich
2013-11-13 15:17 ` Stefan Priebe - Profihost AG
2013-11-13 16:21 ` Veaceslav Falico
2013-11-13 16:43 ` Stefan Priebe - Profihost AG
2013-11-13 17:21 ` Vlad Yasevich
2013-11-13 20:09 ` Stefan Priebe
2013-11-14 3:09 ` Vlad Yasevich
2013-11-14 7:47 ` Stefan Priebe - Profihost AG
2013-11-14 12:29 ` Veaceslav Falico
2013-11-14 21:13 ` Vlad Yasevich
2013-11-16 21:02 ` Stefan Priebe
2013-11-17 3:41 ` Vladislav Yasevich
2013-11-18 7:37 ` Stefan Priebe - Profihost AG
2013-11-14 11:54 ` Veaceslav Falico
2013-11-14 14:27 ` Vlad Yasevich
2013-11-14 14:29 ` Stefan Priebe - Profihost AG [this message]
2013-11-14 14:41 ` Vlad Yasevich
2013-11-16 21:00 ` Stefan Priebe
2013-11-13 16:44 ` Vlad Yasevich
2013-11-13 17:22 ` Stefan Priebe - Profihost AG
2013-11-13 17:37 ` Vlad Yasevich
2013-11-13 17:46 ` Stefan Priebe - Profihost AG
2013-11-13 17:49 ` Vlad Yasevich
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=5284DE65.40105@profihost.ag \
--to=s.priebe@profihost$(echo .)ag \
--cc=netdev@vger$(echo .)kernel.org \
--cc=vfalico@redhat$(echo .)com \
--cc=vyasevic@redhat$(echo .)com \
--cc=vyasevich@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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