| Message ID | 1559532216-12114-1-git-send-email-92siuyang@gmail.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
| Series | ipvlan: Don't propagate IFF_ALLMULTI changes on down interfaces. | expand |
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 >
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 > >
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)
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." Signed-off-by: Young Xiao <92siuyang@gmail.com> --- drivers/net/ipvlan/ipvlan_main.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)