Netdev List
 help / color / mirror / Atom feed
From: Francesco Ruggeri <fruggeri@arista.com>
To: <netdev@vger.kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Mahesh Bandewar <maheshb@google.com>,
	Francesco Ruggeri <fruggeri@arista.com>
Subject: [PATCH net-next] macvlan: fix failure during registration
Date: Mon, 18 Apr 2016 08:20:11 -0700	[thread overview]
Message-ID: <1460992811-46992-1-git-send-email-fruggeri@arista.com> (raw)

Resending, did not include netdev the first time ...

If a macvlan/macvtap creation fails in register_netdevice in
call_netdevice_notifiers(NETDEV_REGISTER) then while cleaning things up in
rollback_registered_many it invokes macvlan_uninit. This results in
port->count being decremented twice (in macvlan_uninit and in
macvlan_common_newlink).
A similar problem may exist in the ipvlan driver.
This patch adds priv_flags to struct macvlan_dev and a flag that
macvlan_uninit can use to determine if it is invoked in the context of a
failed newlink.
In macvtap_device_event(NETDEV_UNREGISTER) it also avoids cleaning up
/dev/tapNN in case creation of the char device had previously failed.
Tested in 3.18.
The failure can be reproduced by running the script below a few times. The
script creates macvtap interfaces in different namespaces causing along the
way sysfs_warn_dup failures in macvtap_device_event(NETDEV_REGISTER)
because of conflicting ifindexes, and finally a panic or
"unregister_netdevice: waiting for ddev to become free".

for ((ns=1; ns<3; ns++))
do
  ip netns add ns${ns}
  ip netns exec ns${ns} ip link add dev ddev type dummy
done

for ((ns=1; ns<3; ns++))
do
  for ((mv=0; mv<2; mv++))
  do
    ret=1
    while [ ${ret} != 0 ]
    do
      ip netns exec ns${ns} ip link add link ddev name \
                        macvtap${ns}${mv} type macvtap
      ret=$?
    done &
  done
done

sleep 5
ls /dev/tap*

for ((ns=1; ns<3; ns++))
do
  ip netns exec ns${ns} ip link del ddev &
done

sleep 2
ls /dev/tap*

for ((ns=1; ns<3; ns++))
do
  ip netns del ns${ns}
done

Signed-off-by: Francesco Ruggeri <fruggeri@arista.com>
---
 drivers/net/macvlan.c      | 11 ++++++++---
 drivers/net/macvtap.c      |  2 ++
 include/linux/if_macvlan.h |  3 +++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 2bcf1f3..11065af 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -823,9 +823,12 @@ static void macvlan_uninit(struct net_device *dev)
 	free_percpu(vlan->pcpu_stats);
 
 	macvlan_flush_sources(port, vlan);
-	port->count -= 1;
-	if (!port->count)
-		macvlan_port_destroy(port->dev);
+
+	if (!(vlan->priv_flags & MACVLAN_PRIV_FLAG_REGISTERING)) {
+		port->count -= 1;
+		if (!port->count)
+			macvlan_port_destroy(port->dev);
+	}
 }
 
 static struct rtnl_link_stats64 *macvlan_dev_get_stats64(struct net_device *dev,
@@ -1313,7 +1316,9 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
 	}
 
 	port->count += 1;
+	vlan->priv_flags |= MACVLAN_PRIV_FLAG_REGISTERING;
 	err = register_netdevice(dev);
+	vlan->priv_flags &= ~MACVLAN_PRIV_FLAG_REGISTERING;
 	if (err < 0)
 		goto destroy_port;
 
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 95394ed..e770221 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -1303,6 +1303,8 @@ static int macvtap_device_event(struct notifier_block *unused,
 		}
 		break;
 	case NETDEV_UNREGISTER:
+		if (vlan->minor == 0)
+			break;
 		devt = MKDEV(MAJOR(macvtap_major), vlan->minor);
 		device_destroy(macvtap_class, devt);
 		macvtap_free_minor(vlan);
diff --git a/include/linux/if_macvlan.h b/include/linux/if_macvlan.h
index a4ccc31..7cf82d8 100644
--- a/include/linux/if_macvlan.h
+++ b/include/linux/if_macvlan.h
@@ -48,6 +48,7 @@ struct macvlan_dev {
 	netdev_features_t	set_features;
 	enum macvlan_mode	mode;
 	u16			flags;
+	u16			priv_flags;
 	/* This array tracks active taps. */
 	struct macvtap_queue	__rcu *taps[MAX_MACVTAP_QUEUES];
 	/* This list tracks all taps (both enabled and disabled) */
@@ -63,6 +64,8 @@ struct macvlan_dev {
 	unsigned int		macaddr_count;
 };
 
+#define MACVLAN_PRIV_FLAG_REGISTERING	1
+
 static inline void macvlan_count_rx(const struct macvlan_dev *vlan,
 				    unsigned int len, bool success,
 				    bool multicast)
-- 
1.8.1.4

             reply	other threads:[~2016-04-18 15:20 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-18 15:20 Francesco Ruggeri [this message]
2016-04-18 18:48 ` [PATCH net-next] macvlan: fix failure during registration Eric W. Biederman
2016-04-18 20:10   ` Francesco Ruggeri
2016-04-18 21:33     ` Eric W. Biederman
2016-04-20  1:13       ` Francesco Ruggeri

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1460992811-46992-1-git-send-email-fruggeri@arista.com \
    --to=fruggeri@arista.com \
    --cc=davem@davemloft.net \
    --cc=ebiederm@xmission.com \
    --cc=maheshb@google.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox