[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <528C132C.9070700@redhat.com>
Date: Tue, 19 Nov 2013 20:41:00 -0500
From: Vlad Yasevich <vyasevic@...hat.com>
To: netdev@...r.kernel.org
Subject: Re: [PATCHv2] net: core: Always propagate flag changes to interfaces
On 11/19/2013 07:37 PM, Vlad Yasevich wrote:
> The following commit:
> b6c40d68ff6498b7f63ddf97cf0aa818d748dee7
> net: only invoke dev->change_rx_flags when device is UP
>
> tried to fix a problem with VLAN devices and promiscuouse flag setting.
> The issue was that VLAN device was setting a flag on an interface that
> was down, thus resulting in bad promiscuity count.
> This commit blocked flag propagation to any device that is currently
> down.
>
> A later commit:
> deede2fabe24e00bd7e246eb81cd5767dc6fcfc7
> vlan: Don't propagate flag changes on down interfaces
>
> fixed VLAN code to only propagate flags when the VLAN interface is up,
> thus fixing the same issue as above, only localized to VLAN.
>
> The problem we have now is that if we have create a complex stack
> involving multiple software devices like bridges, bonds, and vlans,
> then it is possible that the flags would not propagate properly to
> the physical devices. A simple examle of the scenario is the
> following:
>
> eth0----> bond0 ----> bridge0 ---> vlan50
>
> If bond0 or eth0 happen to be down at the time bond0 is added to
> the bridge, then eth0 will never have promisc mode set which is
> currently required for operation as part of the bridge. As a
> result, packets with vlan50 will be dropped by the interface.
>
> The only 2 devices that implement the special flag handling are
> VLAN and DSA and they both have required code to prevent incorrect
> flag propagation. As a result we can remove the generic solution
> introduced in b6c40d68ff6498b7f63ddf97cf0aa818d748dee7 and leave
> it to the individual devices to decide whether they will block
> flag propagation or not.
>
> Reported-by: Stefan Priebe <s.priebe@...fihost.ag>
> Suggested-by: Veaceslav Falico <vfalico@...hat.com>
> Signed-off-by: Vlad Yasevich <vyasevic@...hat.com>
> ---
> v1-v2: rebased on the proper net tree.
>
> net/core/dev.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 974143d..d856e34 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4991,7 +4991,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);
> }
>
> @@ -6885,10 +6885,6 @@ static struct pernet_operations __net_initdata default_device_ops = {
> .exit = default_device_exit,
> .exit_batch = default_device_exit_batch,
> };
> -
> -/*
> - * Initialize the DEV module. At boot time this walks the device list and
> - * unhooks any devices that fail to initialise (normally hardware not
> * present) and leaves us with a valid list of present and active devices.
> *
> */
>
Hm.. not sure where this last hunk came from... taking a look. sorry
about this.
-vlad
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists