Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <1559532216-12114-1-git-send-email-92siuyang@gmail.com> <CADvbK_eUYhP=pSLqHdBp8E3-NJP28=jErSSvW5moO9WVK=X8XQ@mail.gmail.com>
In-Reply-To: <CADvbK_eUYhP=pSLqHdBp8E3-NJP28=jErSSvW5moO9WVK=X8XQ@mail.gmail.com>
From:   Yang Xiao <92siuyang@gmail.com>
Date:   Mon, 3 Jun 2019 16:27:41 +0800
Message-ID: <CAKgHYH1_Jcie=qP-TWRLMXV4OTCe8hm6qZzs2bu4Ciooo2hFiQ@mail.gmail.com>
Subject: Re: [PATCH] ipvlan: Don't propagate IFF_ALLMULTI changes on down interfaces.
To:     Xin Long <lucien.xin@gmail.com>
Cc:     davem <davem@davemloft.net>,
        Daniel Borkmann <daniel@iogearbox.net>, petrm@mellanox.com,
        jiri@mellanox.com, idosch@mellanox.com, uehaibing@huawei.com,
        Hangbin Liu <liuhangbin@gmail.com>,
        network dev <netdev@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Based on your explanation, I found that the patch I submitted is useless.
Great thanks and sorry for the trouble.

On Mon, Jun 3, 2019 at 3:54 PM Xin Long <lucien.xin@gmail.com> wrote:
>
> On Mon, Jun 3, 2019 at 11:22 AM Young Xiao <92siuyang@gmail.com> wrote:
> >
> > Clearing the IFF_ALLMULTI flag on a down interface could cause an allmulti
> > overflow on the underlying interface.
> >
> > Attempting the set IFF_ALLMULTI on the underlying interface would cause an
> > error and the log message:
> >
> > "allmulti touches root, set allmulti failed."
> s/root/roof
>
> I guess this patch was inspired by:
>
> commit bbeb0eadcf9fe74fb2b9b1a6fea82cd538b1e556
> Author: Peter Christensen <pch@ordbogen.com>
> Date:   Thu May 8 11:15:37 2014 +0200
>
>     macvlan: Don't propagate IFF_ALLMULTI changes on down interfaces.
>
> I could trigger this error on macvlan prior to this patch with:
>
>   # ip link add mymacvlan1 link eth2 type macvlan mode bridge
>   # ip link set mymacvlan1 up
>   # ip link set mymacvlan1 allmulticast on
>   # ip link set mymacvlan1 down
>   # ip link set mymacvlan1 allmulticast off
>   # ip link set mymacvlan1 allmulticast on
>
> but not on ipvlan, could you?
>
> macvlan had this problem, as lowerdev's allmulticast is cleared when doing
> dev_stop and set when doing dev_open, which doesn't happen on ipvlan.
>
> So I'd think this patch fixes nothing unless you want to add
> dev_set_allmulti(1/-1) in ipvlan_open/stop(), but that's another topic.
>
> did I miss something?
>
> >
> > Signed-off-by: Young Xiao <92siuyang@gmail.com>
> > ---
> >  drivers/net/ipvlan/ipvlan_main.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
> > index bbeb162..523bb83 100644
> > --- a/drivers/net/ipvlan/ipvlan_main.c
> > +++ b/drivers/net/ipvlan/ipvlan_main.c
> > @@ -242,8 +242,10 @@ static void ipvlan_change_rx_flags(struct net_device *dev, int change)
> >         struct ipvl_dev *ipvlan = netdev_priv(dev);
> >         struct net_device *phy_dev = ipvlan->phy_dev;
> >
> > -       if (change & IFF_ALLMULTI)
> > -               dev_set_allmulti(phy_dev, dev->flags & IFF_ALLMULTI? 1 : -1);
> > +       if (dev->flags & IFF_UP) {
> > +               if (change & IFF_ALLMULTI)
> > +                       dev_set_allmulti(phy_dev, dev->flags & IFF_ALLMULTI ? 1 : -1);
> > +       }
> >  }
> >
> >  static void ipvlan_set_multicast_mac_filter(struct net_device *dev)
> > --
> > 2.7.4
> >