[PATCH 0/2] registration fixes for vbus venet macvlan driver

[PATCH 0/2] registration fixes for vbus venet macvlan driver

[Patrick Mullaney]

Applies to alacrityvm.git/master:4ac12ced9)

These patches fix some netdev registration problems discovered with
the venet macvlan driver. The newline patch is trivial. The other
patch addresses several problems with registration. Worth noting
are the problems registering a netdevice that has previously been
unregistered. This doesn't seem work and I was wondering if this
is expected behavior in net/core(see patch 2/2 for a description).

---

Patrick Mullaney (2):
      venet: Fixes for shutdown and re-registration of venet devices.
      venet-macvlan: correctly remove newline from lower device name


 kernel/vbus/devices/venet/device.c      |   11 ++++--
 kernel/vbus/devices/venet/macvlan.c     |   61 +++++++++++++++++++++++++------
 kernel/vbus/devices/venet/venetdevice.h |    7 +++-
 3 files changed, 63 insertions(+), 16 deletions(-)

-- 

[PATCH 1/2] venet-macvlan: correctly remove newline from lower device name

[Patrick Mullaney]

Signed-off-by: Patrick Mullaney <pmullaney@novell.com>
---

 kernel/vbus/devices/venet/macvlan.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/kernel/vbus/devices/venet/macvlan.c b/kernel/vbus/devices/venet/macvlan.c
index 8724e26..432ff5d 100644
--- a/kernel/vbus/devices/venet/macvlan.c
+++ b/kernel/vbus/devices/venet/macvlan.c
@@ -350,7 +350,11 @@ ll_ifname_store(struct vbus_device *dev, struct vbus_device_attribute *attr,
 	if (priv->dev.vbus.opened)
 		return -EINVAL;
 
-	strncpy(priv->ll_ifname, buf, count-1);
+	memcpy(priv->ll_ifname, buf, count);
+
+	/* remove trailing newline if present */
+	if (priv->ll_ifname[count-1] == '\n')
+		priv->ll_ifname[count-1] = '\0';
 
 	if (priv->mdev.lowerdev) {
 		dev_put(priv->mdev.lowerdev);

[PATCH 2/2] venet: Fixes for shutdown and re-registration of venet devices.

[Patrick Mullaney]

Fixed a problem with simple re-registration of the venet netdevice. The
netdev core infrastructure appears to have a problem with re-registering
a previously registered netdevice(reg_state and kobj.state_initialized
need to be reset to their default values). I now do this prior to calling
register_netdev but this probably should be moved to net/core. Fixed an
issue with the removal of the lower level device of a venet-macvlan. I'm
planning a cleaner solution in the future. Fixed a lock issue with
venet-macvlan stop method that was causing a hang.

Signed-off-by: Patrick Mullaney <pmullaney@novell.com>
---

 kernel/vbus/devices/venet/device.c      |   11 +++++-
 kernel/vbus/devices/venet/macvlan.c     |   55 +++++++++++++++++++++++++------
 kernel/vbus/devices/venet/venetdevice.h |    7 +++-
 3 files changed, 58 insertions(+), 15 deletions(-)

diff --git a/kernel/vbus/devices/venet/device.c b/kernel/vbus/devices/venet/device.c
index 01d6f1c..b4cf397 100644
--- a/kernel/vbus/devices/venet/device.c
+++ b/kernel/vbus/devices/venet/device.c
@@ -2300,7 +2300,7 @@ client_mac_show(struct vbus_device *dev, struct vbus_device_attribute *attr,
 struct vbus_device_attribute attr_cmac =
 	__ATTR(client_mac, S_IRUGO | S_IWUSR, client_mac_show, cmac_store);
 
-static ssize_t
+ssize_t
 enabled_show(struct vbus_device *dev, struct vbus_device_attribute *attr,
 	 char *buf)
 {
@@ -2309,7 +2309,7 @@ enabled_show(struct vbus_device *dev, struct vbus_device_attribute *attr,
 	return snprintf(buf, PAGE_SIZE, "%d\n", priv->netif.enabled);
 }
 
-static ssize_t
+ssize_t
 enabled_store(struct vbus_device *dev, struct vbus_device_attribute *attr,
 	      const char *buf, size_t count)
 {
@@ -2323,8 +2323,13 @@ enabled_store(struct vbus_device *dev, struct vbus_device_attribute *attr,
 	if (enabled != 0 && enabled != 1)
 		return -EINVAL;
 
-	if (enabled && !priv->netif.enabled)
+	if (enabled && !priv->netif.enabled) {
+		/* need to un-initialize certain fields of the
+		   net_device so that it may be re-registered */
+		priv->netif.dev->reg_state = NETREG_UNINITIALIZED;
+		priv->netif.dev->dev.kobj.state_initialized = 0;
 		ret = register_netdev(priv->netif.dev);
+	}
 
 	if (!enabled && priv->netif.enabled)
 		venetdev_netdev_unregister(priv);
diff --git a/kernel/vbus/devices/venet/macvlan.c b/kernel/vbus/devices/venet/macvlan.c
index 432ff5d..60479a6 100644
--- a/kernel/vbus/devices/venet/macvlan.c
+++ b/kernel/vbus/devices/venet/macvlan.c
@@ -137,7 +137,7 @@ venetmacv_vlink_up(struct venetdev *vdev)
 		ret = macv->macvlan_netdev_ops->ndo_open(vdev->netif.dev);
 		rtnl_unlock();
 		if (ret)
-			printk(KERN_ERR "macvlanopen failed %d!\n", ret);
+			printk(KERN_ERR "macvlan_open failed %d!\n", ret);
 	}
 }
 
@@ -320,8 +320,10 @@ venetmacv_device_release(struct vbus_device *dev)
 {
 	struct venetmacv *macv = vbusdev_to_macv(dev);
 
-	if (macv->mdev.lowerdev)
+	if (macv->mdev.lowerdev) {
 		dev_put(macv->mdev.lowerdev);
+		macv->mdev.lowerdev = NULL;
+	}
 
 	venetdev_netdev_unregister(&macv->dev);
 	free_netdev(macv->mdev.dev);
@@ -397,12 +399,31 @@ clientmac_store(struct vbus_device *dev, struct vbus_device_attribute *attr,
 	return ret;
 }
 
-struct vbus_device_attribute attr_clientmac =
+static struct vbus_device_attribute attr_clientmac =
 	__ATTR(client_mac, S_IRUGO | S_IWUSR, client_mac_show, clientmac_store);
 
+
+static ssize_t
+macv_enabled_store(struct vbus_device *dev, struct vbus_device_attribute *attr,
+		   const char *buf, size_t count)
+{
+	struct venetmacv *macv = vbusdev_to_macv(dev);
+	int ret;
+
+	if (!macv->mdev.lowerdev)
+		return -ENXIO;
+
+	ret = attr_enabled.store(dev, attr, buf, count);
+
+	return ret;
+}
+
+static struct vbus_device_attribute attr_macv_enabled =
+	__ATTR(enabled, S_IRUGO | S_IWUSR, enabled_show, macv_enabled_store);
+
 static struct attribute *attrs[] = {
 	&attr_clientmac.attr,
-	&attr_enabled.attr,
+	&attr_macv_enabled.attr,
 	&attr_burstthresh.attr,
 	&attr_txmitigation.attr,
 	&attr_ifname.attr,
@@ -423,9 +444,7 @@ venetmacv_netdev_open(struct net_device *dev)
 	venetdev_open(&priv->dev);
 
 	if (priv->dev.vbus.link) {
-		rtnl_lock();
 		ret = priv->macvlan_netdev_ops->ndo_open(priv->mdev.dev);
-		rtnl_unlock();
 	}
 
 	return ret;
@@ -443,15 +462,26 @@ venetmacv_netdev_stop(struct net_device *dev)
 
 	venetdev_stop(&priv->dev);
 
-	if (priv->dev.vbus.link && needs_stop) {
-		rtnl_lock();
+	if (priv->dev.vbus.link && needs_stop)
 		ret = priv->macvlan_netdev_ops->ndo_stop(dev);
-		rtnl_unlock();
-	}
 
 	return ret;
 }
 
+static void
+venetmacv_netdev_uninit(struct net_device *dev)
+{
+	struct venetmacv *macv = netdev_priv(dev);
+
+	if (macv->mdev.lowerdev) {
+		dev_put(macv->mdev.lowerdev);
+		macv->mdev.lowerdev = NULL;
+		memset(macv->ll_ifname, '\0', IFNAMSIZ);
+	}
+
+	macv->dev.netif.enabled = 0;
+}
+
 /*
  * out routine for macvlan
  */
@@ -527,9 +557,9 @@ static struct net_device_ops venetmacv_netdev_ops = {
 	.ndo_start_xmit         = venetmacv_netdev_tx,
 	.ndo_do_ioctl           = venetdev_netdev_ioctl,
 	.ndo_get_stats          = venetmacv_netdev_stats,
+	.ndo_uninit             = venetmacv_netdev_uninit,
 };
 
-
 /*
  * This is called whenever the admin instantiates our devclass via
  * "mkdir /config/vbus/devices/$(inst)/venet-tap"
@@ -545,6 +575,9 @@ venetmacv_device_create(struct vbus_devclass *dc,
 	dev = alloc_netdev(sizeof(struct venetmacv), "macvenet%d",
 					   macvlan_setup);
 
+
+	dev->destructor = NULL;
+
 	if (!dev)
 		return -ENOMEM;
 
diff --git a/kernel/vbus/devices/venet/venetdevice.h b/kernel/vbus/devices/venet/venetdevice.h
index 1a74723..edee570 100644
--- a/kernel/vbus/devices/venet/venetdevice.h
+++ b/kernel/vbus/devices/venet/venetdevice.h
@@ -150,7 +150,7 @@ struct net_device_stats *venetdev_get_stats(struct venetdev *dev);
 static inline void venetdev_netdev_unregister(struct venetdev *priv)
 {
 	if (priv->netif.enabled) {
-		venetdev_netdev_stop(priv->netif.dev);
+		venetdev_stop(priv);
 		unregister_netdev(priv->netif.dev);
 	}
 }
@@ -173,6 +173,11 @@ extern struct vbus_device_attribute attr_ifname;
 extern struct vbus_device_attribute attr_txmitigation;
 extern struct vbus_device_attribute attr_zcthresh;
 
+ssize_t enabled_store(struct vbus_device *dev,
+		struct vbus_device_attribute *attr,
+		 const char *buf, size_t count);
+ssize_t enabled_show(struct vbus_device *dev,
+			struct vbus_device_attribute *attr, char *buf);
 ssize_t cmac_store(struct vbus_device *dev,
 		struct vbus_device_attribute *attr,
 		 const char *buf, size_t count);