From patchwork Wed Nov 20 07:21:27 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Priebe - Profihost AG X-Patchwork-Id: 292664 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 5CD882C00C3 for ; Wed, 20 Nov 2013 18:21:40 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751799Ab3KTHVf (ORCPT ); Wed, 20 Nov 2013 02:21:35 -0500 Received: from mail-ph.de-nserver.de ([85.158.179.214]:57733 "EHLO mail-ph.de-nserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750792Ab3KTHVe (ORCPT ); Wed, 20 Nov 2013 02:21:34 -0500 Received: (qmail 23829 invoked from network); 20 Nov 2013 08:21:32 +0100 X-Fcrdns: No Received: from fw-office.allied-internet.ag (HELO [192.168.1.148]) (85.158.179.66) (smtp-auth username hostmaster@profihost.com, mechanism plain) by mail-ph.de-nserver.de (qpsmtpd/0.92) with (CAMELLIA256-SHA encrypted) ESMTPSA; Wed, 20 Nov 2013 08:21:32 +0100 Message-ID: <528C62F7.2000100@profihost.ag> Date: Wed, 20 Nov 2013 08:21:27 +0100 From: Stefan Priebe - Profihost AG User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0 MIME-Version: 1.0 To: Vlad Yasevich , netdev@vger.kernel.org CC: vfalico@redhat.com Subject: Re: [PATCHv3] net: core: Always propagate flag changes to interfaces References: <1384912035-17800-1-git-send-email-vyasevic@redhat.com> In-Reply-To: <1384912035-17800-1-git-send-email-vyasevic@redhat.com> X-Enigmail-Version: 1.5.2 X-User-Auth: Auth by hostmaster@profihost.com through 85.158.179.66 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hi, Am 20.11.2013 02:47, schrieb Vlad Yasevich: > 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 > Suggested-by: Veaceslav Falico > Signed-off-by: Vlad Yasevich > --- > v2->v3: Removed a strange chunk that modified comments. Not sure where it > came from. > > net/core/dev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/dev.c b/net/core/dev.c > index 974143d..da9c5e1 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); > } thanks for this patch - in one of the first posts you send this one: diff --git a/net/core/dev.c b/net/core/dev.c index fc913f4..016857b 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -4525,7 +4525,9 @@ 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 (((dev->flags & IFF_UP) || + (dev->flags & (IFF_MASTER | IFF_SLAVE))) + && ops->ndo_change_rx_flags) ops->ndo_change_rx_flags(dev, flags); }