[PATCH net-next 0/8] netvsc: lockdep and related fixes

[PATCH net-next 0/8] netvsc: lockdep and related fixes

[Stephen Hemminger]

These fix sparse and lockdep warnings from netvsc driver.
Targeting these at net-next since no actual related failures
have been observed in non-debug kernels.

Stephen Hemminger (8):
  netvsc: force link update after MTU change
  netvsc: add some rtnl_dereference annotations
  netvsc: change order of steps in setting queues
  netvsc: change logic for change mtu and set_queues
  netvsc: use ERR_PTR to avoid dereference issues
  netvsc: need rcu_derefence when accessing internal device info
  netvsc: save pointer to parent netvsc_device in channel table
  netvsc: add rtnl annotations in rndis

 drivers/net/hyperv/hyperv_net.h   | 26 +++++-------
 drivers/net/hyperv/netvsc.c       | 33 ++++++++++------
 drivers/net/hyperv/netvsc_drv.c   | 83 +++++++++++++++++++++------------------
 drivers/net/hyperv/rndis_filter.c | 62 ++++++++++++++++-------------
 4 files changed, 107 insertions(+), 97 deletions(-)

-- 
2.11.0

[PATCH net-next 1/8] netvsc: force link update after MTU change

[Stephen Hemminger]

If two MTU changes are in less than update interval (2 seconds),
then the netvsc network device may get stuck with no carrier.

The netvsc driver debounces link status events which is fine
for unsolicited updates, but blocks getting the update after
down/up from MTU reinitialization.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/hyperv/netvsc_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 63c98bbbc596..09b07ca9e69a 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -783,6 +783,7 @@ static int netvsc_set_channels(struct net_device *net,
 		ret = netvsc_open(net);
 
 	/* We may have missed link change notifications */
+	net_device_ctx->last_reconfig = 0;
 	schedule_delayed_work(&net_device_ctx->dwork, 0);
 
 	return ret;
-- 
2.11.0

[PATCH net-next 2/8] netvsc: add some rtnl_dereference annotations

[Stephen Hemminger]

In a couple places RTNL is held, and the netvsc_device pointer
is acquired without annotation.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/hyperv/netvsc.c     | 5 +++--
 drivers/net/hyperv/netvsc_drv.c | 7 ++++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 0a9167dd72fb..e202ec5d6f63 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -41,7 +41,7 @@ void netvsc_switch_datapath(struct net_device *ndev, bool vf)
 {
 	struct net_device_context *net_device_ctx = netdev_priv(ndev);
 	struct hv_device *dev = net_device_ctx->device_ctx;
-	struct netvsc_device *nv_dev = net_device_ctx->nvdev;
+	struct netvsc_device *nv_dev = rtnl_dereference(net_device_ctx->nvdev);
 	struct nvsp_message *init_pkt = &nv_dev->channel_init_pkt;
 
 	memset(init_pkt, 0, sizeof(struct nvsp_message));
@@ -549,7 +549,8 @@ void netvsc_device_remove(struct hv_device *device)
 {
 	struct net_device *ndev = hv_get_drvdata(device);
 	struct net_device_context *net_device_ctx = netdev_priv(ndev);
-	struct netvsc_device *net_device = net_device_ctx->nvdev;
+	struct netvsc_device *net_device
+		= rtnl_dereference(net_device_ctx->nvdev);
 	int i;
 
 	netvsc_disconnect_vsp(device);
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 09b07ca9e69a..e8e82a6a4b1a 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -69,7 +69,7 @@ static void netvsc_set_multicast_list(struct net_device *net)
 static int netvsc_open(struct net_device *net)
 {
 	struct net_device_context *ndev_ctx = netdev_priv(net);
-	struct netvsc_device *nvdev = ndev_ctx->nvdev;
+	struct netvsc_device *nvdev = rtnl_dereference(ndev_ctx->nvdev);
 	struct rndis_device *rdev;
 	int ret = 0;
 
@@ -1364,7 +1364,7 @@ static struct net_device *get_netvsc_byref(struct net_device *vf_netdev)
 			continue;	/* not a netvsc device */
 
 		net_device_ctx = netdev_priv(dev);
-		if (net_device_ctx->nvdev == NULL)
+		if (!rtnl_dereference(net_device_ctx->nvdev))
 			continue;	/* device is removed */
 
 		if (rtnl_dereference(net_device_ctx->vf_netdev) == vf_netdev)
@@ -1589,7 +1589,8 @@ static int netvsc_remove(struct hv_device *dev)
 	 * removed. Also blocks mtu and channel changes.
 	 */
 	rtnl_lock();
-	rndis_filter_device_remove(dev, ndev_ctx->nvdev);
+	rndis_filter_device_remove(dev,
+				   rtnl_dereference(ndev_ctx->nvdev));
 	rtnl_unlock();
 
 	unregister_netdev(net);
-- 
2.11.0

[PATCH net-next 3/8] netvsc: change order of steps in setting queues

[Stephen Hemminger]

This fixes the error unwind logic for incorrect number of queues.
If netif_set_real_num_XX_queues failed then rndis_filter_device_add
would have been called twice. Since input arguments are already
ranged checked this is a hypothetical only problem, not possible
in actual code.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/hyperv/netvsc_drv.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index e8e82a6a4b1a..91637336d1fb 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -724,17 +724,15 @@ static int netvsc_set_queues(struct net_device *net, struct hv_device *dev,
 	device_info.ring_size = ring_size;
 	device_info.max_num_vrss_chns = num_chn;
 
-	ret = rndis_filter_device_add(dev, &device_info);
-	if (ret)
-		return ret;
-
 	ret = netif_set_real_num_tx_queues(net, num_chn);
 	if (ret)
 		return ret;
 
 	ret = netif_set_real_num_rx_queues(net, num_chn);
+	if (ret)
+		return ret;
 
-	return ret;
+	return rndis_filter_device_add(dev, &device_info);
 }
 
 static int netvsc_set_channels(struct net_device *net,
-- 
2.11.0

[PATCH net-next 4/8] netvsc: change logic for change mtu and set_queues

[Stephen Hemminger]

Use device detach/attach to ensure that no packets are handed
to device during state changes. Call rndis_filter_open/close
directly as part of later VF related changes.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h   |  1 +
 drivers/net/hyperv/netvsc_drv.c   | 38 ++++++++++++++++++--------------------
 drivers/net/hyperv/rndis_filter.c |  5 +++++
 3 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index d6c25580f8dd..5d541a1462c2 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -200,6 +200,7 @@ int netvsc_recv_callback(struct net_device *net,
 			 const struct ndis_pkt_8021q_info *vlan);
 void netvsc_channel_cb(void *context);
 int netvsc_poll(struct napi_struct *napi, int budget);
+bool rndis_filter_opened(const struct netvsc_device *nvdev);
 int rndis_filter_open(struct netvsc_device *nvdev);
 int rndis_filter_close(struct netvsc_device *nvdev);
 int rndis_filter_device_add(struct hv_device *dev,
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 91637336d1fb..82e41c056e53 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -742,7 +742,7 @@ static int netvsc_set_channels(struct net_device *net,
 	struct hv_device *dev = net_device_ctx->device_ctx;
 	struct netvsc_device *nvdev = rtnl_dereference(net_device_ctx->nvdev);
 	unsigned int count = channels->combined_count;
-	bool was_running;
+	bool was_opened;
 	int ret;
 
 	/* We do not support separate count for rx, tx, or other */
@@ -762,12 +762,9 @@ static int netvsc_set_channels(struct net_device *net,
 	if (count > nvdev->max_chn)
 		return -EINVAL;
 
-	was_running = netif_running(net);
-	if (was_running) {
-		ret = netvsc_close(net);
-		if (ret)
-			return ret;
-	}
+	was_opened = rndis_filter_opened(nvdev);
+	if (was_opened)
+		rndis_filter_close(nvdev);
 
 	rndis_filter_device_remove(dev, nvdev);
 
@@ -777,8 +774,9 @@ static int netvsc_set_channels(struct net_device *net,
 	else
 		netvsc_set_queues(net, dev, nvdev->num_chn);
 
-	if (was_running)
-		ret = netvsc_open(net);
+	nvdev = rtnl_dereference(net_device_ctx->nvdev);
+	if (was_opened)
+		rndis_filter_open(nvdev);
 
 	/* We may have missed link change notifications */
 	net_device_ctx->last_reconfig = 0;
@@ -848,18 +846,15 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 	struct netvsc_device *nvdev = rtnl_dereference(ndevctx->nvdev);
 	struct hv_device *hdev = ndevctx->device_ctx;
 	struct netvsc_device_info device_info;
-	bool was_running;
-	int ret = 0;
+	bool was_opened;
 
 	if (!nvdev || nvdev->destroy)
 		return -ENODEV;
 
-	was_running = netif_running(ndev);
-	if (was_running) {
-		ret = netvsc_close(ndev);
-		if (ret)
-			return ret;
-	}
+	netif_device_detach(ndev);
+	was_opened = rndis_filter_opened(nvdev);
+	if (was_opened)
+		rndis_filter_close(nvdev);
 
 	memset(&device_info, 0, sizeof(device_info));
 	device_info.ring_size = ring_size;
@@ -877,14 +872,17 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 	ndev->mtu = mtu;
 
 	rndis_filter_device_add(hdev, &device_info);
+	nvdev = rtnl_dereference(ndevctx->nvdev);
 
-	if (was_running)
-		ret = netvsc_open(ndev);
+	if (was_opened)
+		rndis_filter_open(nvdev);
+
+	netif_device_attach(ndev);
 
 	/* We may have missed link change notifications */
 	schedule_delayed_work(&ndevctx->dwork, 0);
 
-	return ret;
+	return 0;
 }
 
 static void netvsc_get_stats64(struct net_device *net,
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 85c00e1c52b6..313c6d00d7d9 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -1302,3 +1302,8 @@ int rndis_filter_close(struct netvsc_device *nvdev)
 
 	return rndis_filter_close_device(nvdev->extension);
 }
+
+bool rndis_filter_opened(const struct netvsc_device *nvdev)
+{
+	return atomic_read(&nvdev->open_cnt) > 0;
+}
-- 
2.11.0

[PATCH net-next 5/8] netvsc: use ERR_PTR to avoid dereference issues

[Stephen Hemminger]

The rndis_filter_device_add function is called both in
probe context and RTNL context,and creates the netvsc_device
inner structure. It is easier to get the RTNL lock annotation
correct if it returns the object directly, rather than implicitly
by updating network device private data.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h   |  8 ++++----
 drivers/net/hyperv/netvsc.c       | 13 ++++++------
 drivers/net/hyperv/netvsc_drv.c   | 34 ++++++++++++++++++-------------
 drivers/net/hyperv/rndis_filter.c | 43 ++++++++++++++++-----------------------
 4 files changed, 49 insertions(+), 49 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 5d541a1462c2..e620374727c8 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -183,8 +183,8 @@ struct rndis_device {
 /* Interface */
 struct rndis_message;
 struct netvsc_device;
-int netvsc_device_add(struct hv_device *device,
-		      const struct netvsc_device_info *info);
+struct netvsc_device *netvsc_device_add(struct hv_device *device,
+					const struct netvsc_device_info *info);
 void netvsc_device_remove(struct hv_device *device);
 int netvsc_send(struct hv_device *device,
 		struct hv_netvsc_packet *packet,
@@ -203,8 +203,8 @@ int netvsc_poll(struct napi_struct *napi, int budget);
 bool rndis_filter_opened(const struct netvsc_device *nvdev);
 int rndis_filter_open(struct netvsc_device *nvdev);
 int rndis_filter_close(struct netvsc_device *nvdev);
-int rndis_filter_device_add(struct hv_device *dev,
-			    struct netvsc_device_info *info);
+struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
+					      struct netvsc_device_info *info);
 void rndis_filter_update(struct netvsc_device *nvdev);
 void rndis_filter_device_remove(struct hv_device *dev,
 				struct netvsc_device *nvdev);
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index e202ec5d6f63..4a2550559442 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -29,6 +29,8 @@
 #include <linux/netdevice.h>
 #include <linux/if_ether.h>
 #include <linux/vmalloc.h>
+#include <linux/rtnetlink.h>
+
 #include <asm/sync_bitops.h>
 
 #include "hyperv_net.h"
@@ -1272,8 +1274,8 @@ void netvsc_channel_cb(void *context)
  * netvsc_device_add - Callback when the device belonging to this
  * driver is added
  */
-int netvsc_device_add(struct hv_device *device,
-		      const struct netvsc_device_info *device_info)
+struct netvsc_device *netvsc_device_add(struct hv_device *device,
+				const struct netvsc_device_info *device_info)
 {
 	int i, ret = 0;
 	int ring_size = device_info->ring_size;
@@ -1283,7 +1285,7 @@ int netvsc_device_add(struct hv_device *device,
 
 	net_device = alloc_net_device();
 	if (!net_device)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	net_device->ring_size = ring_size;
 
@@ -1339,7 +1341,7 @@ int netvsc_device_add(struct hv_device *device,
 		goto close;
 	}
 
-	return ret;
+	return net_device;
 
 close:
 	netif_napi_del(&net_device->chan_table[0].napi);
@@ -1350,6 +1352,5 @@ int netvsc_device_add(struct hv_device *device,
 cleanup:
 	free_netvsc_device(&net_device->rcu);
 
-	return ret;
-
+	return ERR_PTR(ret);
 }
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 82e41c056e53..0ca8c74143b4 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -717,6 +717,7 @@ static int netvsc_set_queues(struct net_device *net, struct hv_device *dev,
 			     u32 num_chn)
 {
 	struct netvsc_device_info device_info;
+	struct netvsc_device *net_device;
 	int ret;
 
 	memset(&device_info, 0, sizeof(device_info));
@@ -732,7 +733,8 @@ static int netvsc_set_queues(struct net_device *net, struct hv_device *dev,
 	if (ret)
 		return ret;
 
-	return rndis_filter_device_add(dev, &device_info);
+	net_device = rndis_filter_device_add(dev, &device_info);
+	return IS_ERR(net_device) ? PTR_ERR(net_device) : 0;
 }
 
 static int netvsc_set_channels(struct net_device *net,
@@ -845,8 +847,10 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 	struct net_device_context *ndevctx = netdev_priv(ndev);
 	struct netvsc_device *nvdev = rtnl_dereference(ndevctx->nvdev);
 	struct hv_device *hdev = ndevctx->device_ctx;
+	int orig_mtu = ndev->mtu;
 	struct netvsc_device_info device_info;
 	bool was_opened;
+	int ret = 0;
 
 	if (!nvdev || nvdev->destroy)
 		return -ENODEV;
@@ -863,16 +867,16 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 
 	rndis_filter_device_remove(hdev, nvdev);
 
-	/* 'nvdev' has been freed in rndis_filter_device_remove() ->
-	 * netvsc_device_remove () -> free_netvsc_device().
-	 * We mustn't access it before it's re-created in
-	 * rndis_filter_device_add() -> netvsc_device_add().
-	 */
-
 	ndev->mtu = mtu;
 
-	rndis_filter_device_add(hdev, &device_info);
-	nvdev = rtnl_dereference(ndevctx->nvdev);
+	nvdev = rndis_filter_device_add(hdev, &device_info);
+	if (IS_ERR(nvdev)) {
+		ret = PTR_ERR(nvdev);
+
+		/* Attempt rollback to original MTU */
+		ndev->mtu = orig_mtu;
+		rndis_filter_device_add(hdev, &device_info);
+	}
 
 	if (was_opened)
 		rndis_filter_open(nvdev);
@@ -882,7 +886,7 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 	/* We may have missed link change notifications */
 	schedule_delayed_work(&ndevctx->dwork, 0);
 
-	return 0;
+	return ret;
 }
 
 static void netvsc_get_stats64(struct net_device *net,
@@ -1525,8 +1529,10 @@ static int netvsc_probe(struct hv_device *dev,
 	memset(&device_info, 0, sizeof(device_info));
 	device_info.ring_size = ring_size;
 	device_info.num_chn = VRSS_CHANNEL_DEFAULT;
-	ret = rndis_filter_device_add(dev, &device_info);
-	if (ret != 0) {
+
+	nvdev = rndis_filter_device_add(dev, &device_info);
+	if (IS_ERR(nvdev)) {
+		ret = PTR_ERR(nvdev);
 		netdev_err(net, "unable to add netvsc device (ret %d)\n", ret);
 		free_netdev(net);
 		hv_set_drvdata(dev, NULL);
@@ -1540,11 +1546,11 @@ static int netvsc_probe(struct hv_device *dev,
 		NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX;
 	net->vlan_features = net->features;
 
-	/* RCU not necessary here, device not registered */
-	nvdev = net_device_ctx->nvdev;
 	netif_set_real_num_tx_queues(net, nvdev->num_chn);
 	netif_set_real_num_rx_queues(net, nvdev->num_chn);
 
+	netdev_lockdep_set_classes(net);
+
 	/* MTU range: 68 - 1500 or 65521 */
 	net->min_mtu = NETVSC_MTU_MIN;
 	if (nvdev->nvsp_version >= NVSP_PROTOCOL_VERSION_2)
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 313c6d00d7d9..cacf1e5536f7 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -658,9 +658,9 @@ int rndis_filter_set_device_mac(struct net_device *ndev, char *mac)
 
 static int
 rndis_filter_set_offload_params(struct net_device *ndev,
+				struct netvsc_device *nvdev,
 				struct ndis_offload_params *req_offloads)
 {
-	struct netvsc_device *nvdev = net_device_to_netvsc_device(ndev);
 	struct rndis_device *rdev = nvdev->extension;
 	struct rndis_request *request;
 	struct rndis_set_request *set;
@@ -1052,8 +1052,8 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc)
 		complete(&nvscdev->channel_init_wait);
 }
 
-int rndis_filter_device_add(struct hv_device *dev,
-			    struct netvsc_device_info *device_info)
+struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
+				      struct netvsc_device_info *device_info)
 {
 	struct net_device *net = hv_get_drvdata(dev);
 	struct net_device_context *net_device_ctx = netdev_priv(net);
@@ -1072,21 +1072,20 @@ int rndis_filter_device_add(struct hv_device *dev,
 
 	rndis_device = get_rndis_device();
 	if (!rndis_device)
-		return -ENODEV;
+		return ERR_PTR(-ENODEV);
 
 	/*
 	 * Let the inner driver handle this first to create the netvsc channel
 	 * NOTE! Once the channel is created, we may get a receive callback
 	 * (RndisFilterOnReceive()) before this call is completed
 	 */
-	ret = netvsc_device_add(dev, device_info);
-	if (ret != 0) {
+	net_device = netvsc_device_add(dev, device_info);
+	if (IS_ERR(net_device)) {
 		kfree(rndis_device);
-		return ret;
+		return net_device;
 	}
 
 	/* Initialize the rndis device */
-	net_device = net_device_ctx->nvdev;
 	net_device->max_chn = 1;
 	net_device->num_chn = 1;
 
@@ -1097,10 +1096,8 @@ int rndis_filter_device_add(struct hv_device *dev,
 
 	/* Send the rndis initialization message */
 	ret = rndis_filter_init_device(rndis_device);
-	if (ret != 0) {
-		rndis_filter_device_remove(dev, net_device);
-		return ret;
-	}
+	if (ret != 0)
+		goto err_dev_remv;
 
 	/* Get the MTU from the host */
 	size = sizeof(u32);
@@ -1112,19 +1109,15 @@ int rndis_filter_device_add(struct hv_device *dev,
 
 	/* Get the mac address */
 	ret = rndis_filter_query_device_mac(rndis_device);
-	if (ret != 0) {
-		rndis_filter_device_remove(dev, net_device);
-		return ret;
-	}
+	if (ret != 0)
+		goto err_dev_remv;
 
 	memcpy(device_info->mac_adr, rndis_device->hw_mac_adr, ETH_ALEN);
 
 	/* Find HW offload capabilities */
 	ret = rndis_query_hwcaps(rndis_device, &hwcaps);
-	if (ret != 0) {
-		rndis_filter_device_remove(dev, net_device);
-		return ret;
-	}
+	if (ret != 0)
+		goto err_dev_remv;
 
 	/* A value of zero means "no change"; now turn on what we want. */
 	memset(&offloads, 0, sizeof(struct ndis_offload_params));
@@ -1179,7 +1172,7 @@ int rndis_filter_device_add(struct hv_device *dev,
 
 	netif_set_gso_max_size(net, gso_max_size);
 
-	ret = rndis_filter_set_offload_params(net, &offloads);
+	ret = rndis_filter_set_offload_params(net, net_device, &offloads);
 	if (ret)
 		goto err_dev_remv;
 
@@ -1190,7 +1183,7 @@ int rndis_filter_device_add(struct hv_device *dev,
 		   rndis_device->link_state ? "down" : "up");
 
 	if (net_device->nvsp_version < NVSP_PROTOCOL_VERSION_5)
-		return 0;
+		return net_device;
 
 	rndis_filter_query_link_speed(rndis_device);
 
@@ -1223,7 +1216,7 @@ int rndis_filter_device_add(struct hv_device *dev,
 
 	num_rss_qs = net_device->num_chn - 1;
 	if (num_rss_qs == 0)
-		return 0;
+		return net_device;
 
 	refcount_set(&net_device->sc_offered, num_rss_qs);
 	vmbus_set_sc_create_callback(dev->channel, netvsc_sc_open);
@@ -1260,11 +1253,11 @@ int rndis_filter_device_add(struct hv_device *dev,
 		net_device->num_chn = 1;
 	}
 
-	return 0; /* return 0 because primary channel can be used alone */
+	return net_device;
 
 err_dev_remv:
 	rndis_filter_device_remove(dev, net_device);
-	return ret;
+	return ERR_PTR(ret);
 }
 
 void rndis_filter_device_remove(struct hv_device *dev,
-- 
2.11.0

[PATCH net-next 6/8] netvsc: need rcu_derefence when accessing internal device info

[Stephen Hemminger]

The netvsc_device structure should be accessed by rcu_dereference
in the send path.  Change arguments to netvsc_send() to make
this easier to do correctly.

Remove no longer needed hv_device_to_netvsc_device.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h   | 10 +++-------
 drivers/net/hyperv/netvsc.c       |  8 +++++---
 drivers/net/hyperv/netvsc_drv.c   |  4 ++--
 drivers/net/hyperv/rndis_filter.c |  2 +-
 4 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index e620374727c8..0054b6929f6e 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -183,10 +183,12 @@ struct rndis_device {
 /* Interface */
 struct rndis_message;
 struct netvsc_device;
+struct net_device_context;
+
 struct netvsc_device *netvsc_device_add(struct hv_device *device,
 					const struct netvsc_device_info *info);
 void netvsc_device_remove(struct hv_device *device);
-int netvsc_send(struct hv_device *device,
+int netvsc_send(struct net_device_context *ndc,
 		struct hv_netvsc_packet *packet,
 		struct rndis_message *rndis_msg,
 		struct hv_page_buffer **page_buffer,
@@ -790,12 +792,6 @@ net_device_to_netvsc_device(struct net_device *ndev)
 	return ((struct net_device_context *)netdev_priv(ndev))->nvdev;
 }
 
-static inline struct netvsc_device *
-hv_device_to_netvsc_device(struct hv_device *device)
-{
-	return net_device_to_netvsc_device(hv_get_drvdata(device));
-}
-
 /* NdisInitialize message */
 struct rndis_initialize_request {
 	u32 req_id;
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 4a2550559442..3c6f3ae520d9 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -822,13 +822,15 @@ static inline void move_pkt_msd(struct hv_netvsc_packet **msd_send,
 	msdp->count = 0;
 }
 
-int netvsc_send(struct hv_device *device,
+/* RCU already held by caller */
+int netvsc_send(struct net_device_context *ndev_ctx,
 		struct hv_netvsc_packet *packet,
 		struct rndis_message *rndis_msg,
 		struct hv_page_buffer **pb,
 		struct sk_buff *skb)
 {
-	struct netvsc_device *net_device = hv_device_to_netvsc_device(device);
+	struct netvsc_device *net_device = rcu_dereference(ndev_ctx->nvdev);
+	struct hv_device *device = ndev_ctx->device_ctx;
 	int ret = 0;
 	struct netvsc_channel *nvchan;
 	u32 pktlen = packet->total_data_buflen, msd_len = 0;
@@ -840,7 +842,7 @@ int netvsc_send(struct hv_device *device,
 	bool xmit_more = (skb != NULL) ? skb->xmit_more : false;
 
 	/* If device is rescinded, return error and packet will get dropped. */
-	if (unlikely(net_device->destroy))
+	if (unlikely(!net_device || net_device->destroy))
 		return -ENODEV;
 
 	/* We may race with netvsc_connect_vsp()/netvsc_init_buf() and get
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 0ca8c74143b4..1238600d717e 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -505,8 +505,8 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 
 	/* timestamp packet in software */
 	skb_tx_timestamp(skb);
-	ret = netvsc_send(net_device_ctx->device_ctx, packet,
-			  rndis_msg, &pb, skb);
+
+	ret = netvsc_send(net_device_ctx, packet, rndis_msg, &pb, skb);
 	if (likely(ret == 0))
 		return NETDEV_TX_OK;
 
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index cacf1e5536f7..9ab67c8309ff 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -243,7 +243,7 @@ static int rndis_filter_send_request(struct rndis_device *dev,
 			pb[0].len;
 	}
 
-	ret = netvsc_send(net_device_ctx->device_ctx, packet, NULL, &pb, NULL);
+	ret = netvsc_send(net_device_ctx, packet, NULL, &pb, NULL);
 	return ret;
 }
 
-- 
2.11.0

[PATCH net-next 7/8] netvsc: save pointer to parent netvsc_device in channel table

[Stephen Hemminger]

Keep back pointer in the per-channel data structure to
avoid any possible RCU related issues when napi poll is
called but netvsc_device is in RCU limbo.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h | 1 +
 drivers/net/hyperv/netvsc.c     | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 0054b6929f6e..d13572879e7e 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -727,6 +727,7 @@ struct net_device_context {
 /* Per channel data */
 struct netvsc_channel {
 	struct vmbus_channel *channel;
+	struct netvsc_device *net_device;
 	const struct vmpacket_descriptor *desc;
 	struct napi_struct napi;
 	struct multi_send_data msd;
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 3c6f3ae520d9..c15640c6fd83 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -1224,11 +1224,11 @@ int netvsc_poll(struct napi_struct *napi, int budget)
 {
 	struct netvsc_channel *nvchan
 		= container_of(napi, struct netvsc_channel, napi);
+	struct netvsc_device *net_device = nvchan->net_device;
 	struct vmbus_channel *channel = nvchan->channel;
 	struct hv_device *device = netvsc_channel_to_device(channel);
 	u16 q_idx = channel->offermsg.offer.sub_channel_index;
 	struct net_device *ndev = hv_get_drvdata(device);
-	struct netvsc_device *net_device = net_device_to_netvsc_device(ndev);
 	int work_done = 0;
 
 	/* If starting a new interval */
@@ -1307,6 +1307,7 @@ struct netvsc_device *netvsc_device_add(struct hv_device *device,
 		struct netvsc_channel *nvchan = &net_device->chan_table[i];
 
 		nvchan->channel = device->channel;
+		nvchan->net_device = net_device;
 	}
 
 	/* Enable NAPI handler before init callbacks */
-- 
2.11.0

[PATCH net-next 8/8] netvsc: add rtnl annotations in rndis

[Stephen Hemminger]

The rndis functions are used when changing device state.
Therefore the references from network device to internal state
are protected by RTNL mutex.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h   |  6 ------
 drivers/net/hyperv/netvsc.c       |  6 ++++--
 drivers/net/hyperv/netvsc_drv.c   |  1 +
 drivers/net/hyperv/rndis_filter.c | 12 ++++++++++--
 4 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index d13572879e7e..afb65f753574 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -787,12 +787,6 @@ struct netvsc_device {
 	struct rcu_head rcu;
 };
 
-static inline struct netvsc_device *
-net_device_to_netvsc_device(struct net_device *ndev)
-{
-	return ((struct net_device_context *)netdev_priv(ndev))->nvdev;
-}
-
 /* NdisInitialize message */
 struct rndis_initialize_request {
 	u32 req_id;
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index c15640c6fd83..0a9d9feedc3f 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -105,7 +105,8 @@ static void netvsc_destroy_buf(struct hv_device *device)
 {
 	struct nvsp_message *revoke_packet;
 	struct net_device *ndev = hv_get_drvdata(device);
-	struct netvsc_device *net_device = net_device_to_netvsc_device(ndev);
+	struct net_device_context *ndc = netdev_priv(ndev);
+	struct netvsc_device *net_device = rtnl_dereference(ndc->nvdev);
 	int ret;
 
 	/*
@@ -829,7 +830,8 @@ int netvsc_send(struct net_device_context *ndev_ctx,
 		struct hv_page_buffer **pb,
 		struct sk_buff *skb)
 {
-	struct netvsc_device *net_device = rcu_dereference(ndev_ctx->nvdev);
+	struct netvsc_device *net_device
+		= rcu_dereference_rtnl(ndev_ctx->nvdev);
 	struct hv_device *device = ndev_ctx->device_ctx;
 	int ret = 0;
 	struct netvsc_channel *nvchan;
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 1238600d717e..a164981c15f7 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1548,6 +1548,7 @@ static int netvsc_probe(struct hv_device *dev,
 
 	netif_set_real_num_tx_queues(net, nvdev->num_chn);
 	netif_set_real_num_rx_queues(net, nvdev->num_chn);
+	rtnl_unlock();
 
 	netdev_lockdep_set_classes(net);
 
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 9ab67c8309ff..e439886f72c1 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -84,6 +84,14 @@ static struct rndis_device *get_rndis_device(void)
 	return device;
 }
 
+static struct netvsc_device *
+net_device_to_netvsc_device(struct net_device *ndev)
+{
+	struct net_device_context *net_device_ctx = netdev_priv(ndev);
+
+	return rtnl_dereference(net_device_ctx->nvdev);
+}
+
 static struct rndis_request *get_rndis_request(struct rndis_device *dev,
 					     u32 msg_type,
 					     u32 msg_len)
@@ -472,7 +480,7 @@ static int rndis_filter_query_device(struct rndis_device *dev, u32 oid,
 
 	if (oid == OID_TCP_OFFLOAD_HARDWARE_CAPABILITIES) {
 		struct net_device_context *ndevctx = netdev_priv(dev->ndev);
-		struct netvsc_device *nvdev = ndevctx->nvdev;
+		struct netvsc_device *nvdev = rtnl_dereference(ndevctx->nvdev);
 		struct ndis_offload *hwcaps;
 		u32 nvsp_version = nvdev->nvsp_version;
 		u8 ndis_rev;
@@ -944,7 +952,7 @@ static void rndis_filter_halt_device(struct rndis_device *dev)
 	struct rndis_request *request;
 	struct rndis_halt_request *halt;
 	struct net_device_context *net_device_ctx = netdev_priv(dev->ndev);
-	struct netvsc_device *nvdev = net_device_ctx->nvdev;
+	struct netvsc_device *nvdev = rtnl_dereference(net_device_ctx->nvdev);
 
 	/* Attempt to do a rndis device halt */
 	request = get_rndis_request(dev, RNDIS_MSG_HALT,
-- 
2.11.0

RE: [PATCH net-next 3/8] netvsc: change order of steps in setting queues

[Haiyang Zhang]

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, July 19, 2017 2:53 PM
> To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; Stephen Hemminger <sthemmin@microsoft.com>
> Cc: devel@linuxdriverproject.org; netdev@vger.kernel.org
> Subject: [PATCH net-next 3/8] netvsc: change order of steps in setting
> queues
> 
> This fixes the error unwind logic for incorrect number of queues.
> If netif_set_real_num_XX_queues failed then rndis_filter_device_add
> would have been called twice. Since input arguments are already
> ranged checked this is a hypothetical only problem, not possible
> in actual code.
> 
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
>  drivers/net/hyperv/netvsc_drv.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/hyperv/netvsc_drv.c
> b/drivers/net/hyperv/netvsc_drv.c
> index e8e82a6a4b1a..91637336d1fb 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -724,17 +724,15 @@ static int netvsc_set_queues(struct net_device
> *net, struct hv_device *dev,
>  	device_info.ring_size = ring_size;
>  	device_info.max_num_vrss_chns = num_chn;
> 
> -	ret = rndis_filter_device_add(dev, &device_info);
> -	if (ret)
> -		return ret;
> -
>  	ret = netif_set_real_num_tx_queues(net, num_chn);
>  	if (ret)
>  		return ret;
> 
>  	ret = netif_set_real_num_rx_queues(net, num_chn);
> +	if (ret)
> +		return ret;
> 
> -	return ret;
> +	return rndis_filter_device_add(dev, &device_info);
>  }

The existing code has a bug here. After rndis_filter_device_add(),
the number of queues granted by the host may change:
        net_device->num_chn = 1 +
                init_packet->msg.v5_msg.subchn_comp.num_subchannels;
So we should call rndis_filter_device_add() first, then
assign net_device->num_chn to "real number tx queues".

Thanks,
- Haiyang

Re: [PATCH net-next 0/8] netvsc: lockdep and related fixes

[David Miller]

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Wed, 19 Jul 2017 11:53:11 -0700

> These fix sparse and lockdep warnings from netvsc driver.
> Targeting these at net-next since no actual related failures
> have been observed in non-debug kernels.

Series applied, thanks Stephen.

Re: [PATCH net-next 2/8] netvsc: add some rtnl_dereference annotations

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2644 bytes --]

Hi Stephen,

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Stephen-Hemminger/netvsc-lockdep-and-related-fixes/20170720-191938
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: the linux-review/Stephen-Hemminger/netvsc-lockdep-and-related-fixes/20170720-191938 HEAD a5e48342eae178e9ed020d3634cdf92f1f05cb70 builds fine.
      It only hurts bisectibility.

All error/warnings (new ones prefixed by >>):

   drivers/net/hyperv/netvsc.c: In function 'netvsc_switch_datapath':
>> drivers/net/hyperv/netvsc.c:44:33: error: implicit declaration of function 'rtnl_dereference' [-Werror=implicit-function-declaration]
     struct netvsc_device *nv_dev = rtnl_dereference(net_device_ctx->nvdev);
                                    ^~~~~~~~~~~~~~~~
>> drivers/net/hyperv/netvsc.c:44:33: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
   drivers/net/hyperv/netvsc.c: In function 'netvsc_device_remove':
   drivers/net/hyperv/netvsc.c:553:5: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
      = rtnl_dereference(net_device_ctx->nvdev);
        ^~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/rtnl_dereference +44 drivers/net/hyperv/netvsc.c

    35	
    36	/*
    37	 * Switch the data path from the synthetic interface to the VF
    38	 * interface.
    39	 */
    40	void netvsc_switch_datapath(struct net_device *ndev, bool vf)
    41	{
    42		struct net_device_context *net_device_ctx = netdev_priv(ndev);
    43		struct hv_device *dev = net_device_ctx->device_ctx;
  > 44		struct netvsc_device *nv_dev = rtnl_dereference(net_device_ctx->nvdev);
    45		struct nvsp_message *init_pkt = &nv_dev->channel_init_pkt;
    46	
    47		memset(init_pkt, 0, sizeof(struct nvsp_message));
    48		init_pkt->hdr.msg_type = NVSP_MSG4_TYPE_SWITCH_DATA_PATH;
    49		if (vf)
    50			init_pkt->msg.v4_msg.active_dp.active_datapath =
    51				NVSP_DATAPATH_VF;
    52		else
    53			init_pkt->msg.v4_msg.active_dp.active_datapath =
    54				NVSP_DATAPATH_SYNTHETIC;
    55	
    56		vmbus_sendpacket(dev->channel, init_pkt,
    57				       sizeof(struct nvsp_message),
    58				       (unsigned long)init_pkt,
    59				       VM_PKT_DATA_INBAND, 0);
    60	
    61		net_device_ctx->datapath = vf;
    62	}
    63	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39495 bytes --]