Subject: [RFC 1/2] hv_netvsc: make sure device is idle before changes

From: Stephen Hemminger <[email-masked]>
Date: Fri, 26 Jan 2018 16:18:13 -0800

Make sure that device is in detached state before doing ring
and mtu changes. When doing these changes, wait for all outstanding
send completions and ring buffer events.

Signed-off-by: Stephen Hemminger <[email-masked]>
---
 drivers/net/hyperv/hyperv_net.h   |  1 -
 drivers/net/hyperv/netvsc.c       |  6 +----
 drivers/net/hyperv/netvsc_drv.c   | 29 +++++++++++++----------
 drivers/net/hyperv/rndis_filter.c | 48 ++++++++++++++++++---------------------
 4 files changed, 40 insertions(+), 44 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 0db3bd1ea06f..a846a9c50ddb 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -211,7 +211,6 @@ void netvsc_channel_cb(void *context);
 int netvsc_poll(struct napi_struct *napi, int budget);
 
 void rndis_set_subchannel(struct work_struct *w);
-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);
 struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 17e529af79dc..619a04f98321 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -849,7 +849,7 @@ int netvsc_send(struct net_device *ndev,
 	bool try_batch, xmit_more;
 
 	/* If device is rescinded, return error and packet will get dropped. */
-	if (unlikely(!net_device || net_device->destroy))
+	if (unlikely(!net_device))
 		return -ENODEV;
 
 	/* We may race with netvsc_connect_vsp()/netvsc_init_buf() and get
@@ -996,10 +996,6 @@ static int send_recv_completions(struct net_device *ndev,
 			mrc->first = 0;
 	}
 
-	/* receive completion ring has been emptied */
-	if (unlikely(nvdev->destroy))
-		wake_up(&nvdev->wait_drain);
-
 	return 0;
 }
 
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index c5584c2d440e..ef395e379a83 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -820,7 +820,7 @@ static int netvsc_set_channels(struct net_device *net,
 	    channels->rx_count || channels->tx_count || channels->other_count)
 		return -EINVAL;
 
-	if (!nvdev || nvdev->destroy)
+	if (!nvdev)
 		return -ENODEV;
 
 	if (nvdev->nvsp_version < NVSP_PROTOCOL_VERSION_5)
@@ -830,9 +830,6 @@ static int netvsc_set_channels(struct net_device *net,
 		return -EINVAL;
 
 	orig = nvdev->num_chn;
-	was_opened = rndis_filter_opened(nvdev);
-	if (was_opened)
-		rndis_filter_close(nvdev);
 
 	memset(&device_info, 0, sizeof(device_info));
 	device_info.num_chn = count;
@@ -841,6 +838,11 @@ static int netvsc_set_channels(struct net_device *net,
 	device_info.recv_sections = nvdev->recv_section_cnt;
 	device_info.recv_section_size = nvdev->recv_section_size;
 
+	was_opened = netif_running(net);
+	netif_device_detach(net);
+	if (was_opened)
+		rndis_filter_close(nvdev);
+
 	rndis_filter_device_remove(dev, nvdev);
 
 	nvdev = rndis_filter_device_add(dev, &device_info);
@@ -859,6 +861,8 @@ static int netvsc_set_channels(struct net_device *net,
 	if (was_opened)
 		rndis_filter_open(nvdev);
 
+	netif_device_attach(net);
+
 	/* We may have missed link change notifications */
 	net_device_ctx->last_reconfig = 0;
 	schedule_delayed_work(&net_device_ctx->dwork, 0);
@@ -934,7 +938,7 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 	bool was_opened;
 	int ret = 0;
 
-	if (!nvdev || nvdev->destroy)
+	if (!nvdev)
 		return -ENODEV;
 
 	/* Change MTU of underlying VF netdev first. */
@@ -944,11 +948,6 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 			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.num_chn = nvdev->num_chn;
 	device_info.send_sections = nvdev->send_section_cnt;
@@ -956,6 +955,11 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 	device_info.recv_sections = nvdev->recv_section_cnt;
 	device_info.recv_section_size = nvdev->recv_section_size;
 
+	was_opened = netif_running(ndev);
+	netif_device_detach(ndev);
+	if (was_opened)
+		rndis_filter_close(nvdev);
+
 	rndis_filter_device_remove(hdev, nvdev);
 
 	ndev->mtu = mtu;
@@ -1497,7 +1501,7 @@ static int netvsc_set_ringparam(struct net_device *ndev,
 	bool was_opened;
 	int ret = 0;
 
-	if (!nvdev || nvdev->destroy)
+	if (!nvdev)
 		return -ENODEV;
 
 	memset(&orig, 0, sizeof(orig));
@@ -1519,8 +1523,8 @@ static int netvsc_set_ringparam(struct net_device *ndev,
 	device_info.recv_sections = new_rx;
 	device_info.recv_section_size = nvdev->recv_section_size;
 
+	was_opened = netif_running(ndev);
 	netif_device_detach(ndev);
-	was_opened = rndis_filter_opened(nvdev);
 	if (was_opened)
 		rndis_filter_close(nvdev);
 
@@ -1542,6 +1546,7 @@ static int netvsc_set_ringparam(struct net_device *ndev,
 
 	if (was_opened)
 		rndis_filter_open(nvdev);
+
 	netif_device_attach(ndev);
 
 	/* We may have missed link change notifications */
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index c3ca191fea7f..a6c9c9a2973d 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -923,6 +923,7 @@ static int rndis_filter_init_device(struct rndis_device *dev,
 	return ret;
 }
 
+/* Are there any outstanding sends */
 static bool netvsc_device_idle(const struct netvsc_device *nvdev)
 {
 	int i;
@@ -930,8 +931,7 @@ static bool netvsc_device_idle(const struct netvsc_device *nvdev)
 	for (i = 0; i < nvdev->num_chn; i++) {
 		const struct netvsc_channel *nvchan = &nvdev->chan_table[i];
 
-		if (nvchan->mrc.first != nvchan->mrc.next)
-			return false;
+		napi_synchronize(&nvchan->napi);
 
 		if (atomic_read(&nvchan->queue_sends) > 0)
 			return false;
@@ -944,14 +944,12 @@ 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 = rtnl_dereference(net_device_ctx->nvdev);
 
 	/* Attempt to do a rndis device halt */
 	request = get_rndis_request(dev, RNDIS_MSG_HALT,
 				RNDIS_MESSAGE_SIZE(struct rndis_halt_request));
 	if (!request)
-		goto cleanup;
+		return;
 
 	/* Setup the rndis set */
 	halt = &request->request_msg.msg.halt_req;
@@ -962,17 +960,7 @@ static void rndis_filter_halt_device(struct rndis_device *dev)
 
 	dev->state = RNDIS_DEV_UNINITIALIZED;
 
-cleanup:
-	nvdev->destroy = true;
-
-	/* Force flag to be ordered before waiting */
-	wmb();
-
-	/* Wait for all send completions */
-	wait_event(nvdev->wait_drain, netvsc_device_idle(nvdev));
-
-	if (request)
-		put_rndis_request(dev, request);
+	put_rndis_request(dev, request);
 }
 
 static int rndis_filter_open_device(struct rndis_device *dev)
@@ -994,9 +982,11 @@ static int rndis_filter_open_device(struct rndis_device *dev)
 
 static int rndis_filter_close_device(struct rndis_device *dev)
 {
+	struct net_device_context *net_device_ctx = netdev_priv(dev->ndev);
+	struct netvsc_device *nvdev = rtnl_dereference(net_device_ctx->nvdev);
 	int ret;
 
-	if (dev->state != RNDIS_DEV_DATAINITIALIZED)
+	if (!nvdev || dev->state != RNDIS_DEV_DATAINITIALIZED)
 		return 0;
 
 	/* Make sure rndis_set_multicast doesn't re-enable filter! */
@@ -1004,10 +994,23 @@ static int rndis_filter_close_device(struct rndis_device *dev)
 
 	ret = rndis_filter_set_packet_filter(dev, 0);
 	if (ret == -ENODEV)
-		ret = 0;
+		ret = 0;	/* rescinded is ok */
+
+	if (ret == 0) {
+		/* Indicate that wakeup on send done is desired */
+		nvdev->destroy = true;
+
+		/* Force flag to be ordered before waiting */
+		wmb();
+
+		/* Wait for all completions */
+		wait_event(nvdev->wait_drain, netvsc_device_idle(nvdev));
+
+		/* No more wakeups please */
+		nvdev->destroy = false;
 
-	if (ret == 0)
 		dev->state = RNDIS_DEV_INITIALIZED;
+	}
 
 	return ret;
 }
@@ -1364,10 +1367,3 @@ int rndis_filter_close(struct netvsc_device *nvdev)
 
 	return rndis_filter_close_device(nvdev->extension);
 }
-
-bool rndis_filter_opened(const struct netvsc_device *nvdev)
-{
-	const struct rndis_device *dev = nvdev->extension;
-
-	return dev->state == RNDIS_DEV_DATAINITIALIZED;
-}
-- 
2.15.1