From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mahesh Bandewar Subject: Re: [PATCH net-next] macvlan: fix failure during registration v3 Date: Tue, 26 Apr 2016 11:54:08 -0700 Message-ID: References: <1461449012-7401-1-git-send-email-fruggeri@arista.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: linux-netdev , "Eric W. Biederman" , "David S. Miller" To: Francesco Ruggeri Return-path: Received: from mail-yw0-f180.google.com ([209.85.161.180]:33838 "EHLO mail-yw0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752464AbcDZSy3 (ORCPT ); Tue, 26 Apr 2016 14:54:29 -0400 Received: by mail-yw0-f180.google.com with SMTP id j74so27214976ywg.1 for ; Tue, 26 Apr 2016 11:54:29 -0700 (PDT) In-Reply-To: <1461449012-7401-1-git-send-email-fruggeri@arista.com> Sender: netdev-owner@vger.kernel.org List-ID: On Sat, Apr 23, 2016 at 3:03 PM, Francesco Ruggeri wrote: > If macvlan_common_newlink fails in register_netdevice after macvlan_init > then it decrements port->count twice, first in macvlan_uninit (from > register_netdevice or rollback_registered) and then again in > macvlan_common_newlink. > A similar problem may exist in the ipvlan driver. That is correct! The problem exists in ipvlan driver also. I'll cook a patch to fix this in ipvlan driver. > This patch consolidates modifications to port->count into macvlan_init > and macvlan_uninit (thanks to Eric Biederman for suggesting this approach). > > v3: remove macvtap specific bits. > > Signed-off-by: Francesco Ruggeri > --- > drivers/net/macvlan.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c > index 2bcf1f3..cb01023 100644 > --- a/drivers/net/macvlan.c > +++ b/drivers/net/macvlan.c > @@ -795,6 +795,7 @@ static int macvlan_init(struct net_device *dev) > { > struct macvlan_dev *vlan = netdev_priv(dev); > const struct net_device *lowerdev = vlan->lowerdev; > + struct macvlan_port *port = vlan->port; > > dev->state = (dev->state & ~MACVLAN_STATE_MASK) | > (lowerdev->state & MACVLAN_STATE_MASK); > @@ -812,6 +813,8 @@ static int macvlan_init(struct net_device *dev) > if (!vlan->pcpu_stats) > return -ENOMEM; > > + port->count += 1; > + > return 0; > } > > @@ -1312,10 +1315,9 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev, > return err; > } > > - port->count += 1; > err = register_netdevice(dev); > if (err < 0) > - goto destroy_port; > + return err; > > dev->priv_flags |= IFF_MACVLAN; > err = netdev_upper_dev_link(lowerdev, dev); > @@ -1330,10 +1332,6 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev, > > unregister_netdev: > unregister_netdevice(dev); > -destroy_port: > - port->count -= 1; > - if (!port->count) > - macvlan_port_destroy(lowerdev); I think you still need this when it fails netdev_upper_dev_link(). The only thing you should remove is the label. > > return err; > } > -- > 1.8.1.4 >