Olaf Hering 069948
From: Mohammed Gamal <mgamal@redhat.com>
Olaf Hering 069948
Date: Thu, 5 Apr 2018 21:09:20 +0200
Olaf Hering 069948
Patch-mainline: v4.17-rc1
Olaf Hering 069948
Subject: hv_netvsc: Ensure correct teardown message sequence order
Olaf Hering 069948
Git-commit: a56d99d714665591fed8527b90eef21530ea61e0
Olaf Hering 069948
References: fate#323887
Olaf Hering 069948
Olaf Hering 069948
Prior to commit 0cf737808ae7 ("hv_netvsc: netvsc_teardown_gpadl() split")
Olaf Hering 069948
the call sequence in netvsc_device_remove() was as follows (as
Olaf Hering 069948
implemented in netvsc_destroy_buf()):
Olaf Hering 069948
1- Send NVSP_MSG1_TYPE_REVOKE_RECV_BUF message
Olaf Hering 069948
2- Teardown receive buffer GPADL
Olaf Hering 069948
3- Send NVSP_MSG1_TYPE_REVOKE_SEND_BUF message
Olaf Hering 069948
4- Teardown send buffer GPADL
Olaf Hering 069948
5- Close vmbus
Olaf Hering 069948
Olaf Hering 069948
This didn't work for WS2016 hosts. Commit 0cf737808ae7
Olaf Hering 069948
("hv_netvsc: netvsc_teardown_gpadl() split") rearranged the
Olaf Hering 069948
teardown sequence as follows:
Olaf Hering 069948
1- Send NVSP_MSG1_TYPE_REVOKE_RECV_BUF message
Olaf Hering 069948
2- Send NVSP_MSG1_TYPE_REVOKE_SEND_BUF message
Olaf Hering 069948
3- Close vmbus
Olaf Hering 069948
4- Teardown receive buffer GPADL
Olaf Hering 069948
5- Teardown send buffer GPADL
Olaf Hering 069948
Olaf Hering 069948
That worked well for WS2016 hosts, but it prevented guests on older hosts from
Olaf Hering 069948
shutting down after changing network settings. Commit 0ef58b0a05c1
Olaf Hering 069948
("hv_netvsc: change GPAD teardown order on older versions") ensured the
Olaf Hering 069948
following message sequence for older hosts
Olaf Hering 069948
1- Send NVSP_MSG1_TYPE_REVOKE_RECV_BUF message
Olaf Hering 069948
2- Send NVSP_MSG1_TYPE_REVOKE_SEND_BUF message
Olaf Hering 069948
3- Teardown receive buffer GPADL
Olaf Hering 069948
4- Teardown send buffer GPADL
Olaf Hering 069948
5- Close vmbus
Olaf Hering 069948
Olaf Hering 069948
However, with this sequence calling `ip link set eth0 mtu 1000` hangs and the
Olaf Hering 069948
process becomes uninterruptible. On futher analysis it turns out that on tearing
Olaf Hering 069948
down the receive buffer GPADL the kernel is waiting indefinitely
Olaf Hering 069948
in vmbus_teardown_gpadl() for a completion to be signaled.
Olaf Hering 069948
Olaf Hering 069948
Here is a snippet of where this occurs:
Olaf Hering 069948
int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle)
Olaf Hering 069948
{
Olaf Hering 069948
        struct vmbus_channel_gpadl_teardown *msg;
Olaf Hering 069948
        struct vmbus_channel_msginfo *info;
Olaf Hering 069948
        unsigned long flags;
Olaf Hering 069948
        int ret;
Olaf Hering 069948
Olaf Hering 069948
        info = kmalloc(sizeof(*info) +
Olaf Hering 069948
                       sizeof(struct vmbus_channel_gpadl_teardown), GFP_KERNEL);
Olaf Hering 069948
        if (!info)
Olaf Hering 069948
                return -ENOMEM;
Olaf Hering 069948
Olaf Hering 069948
        init_completion(&info->waitevent);
Olaf Hering 069948
        info->waiting_channel = channel;
Olaf Hering 069948
[....]
Olaf Hering 069948
        ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_gpadl_teardown),
Olaf Hering 069948
                             true);
Olaf Hering 069948
Olaf Hering 069948
        if (ret)
Olaf Hering 069948
                goto post_msg_err;
Olaf Hering 069948
Olaf Hering 069948
        wait_for_completion(&info->waitevent);
Olaf Hering 069948
[....]
Olaf Hering 069948
}
Olaf Hering 069948
Olaf Hering 069948
The completion is signaled from vmbus_ongpadl_torndown(), which gets called when
Olaf Hering 069948
the corresponding message is received from the host, which apparently never happens
Olaf Hering 069948
in that case.
Olaf Hering 069948
This patch works around the issue by restoring the first mentioned message sequence
Olaf Hering 069948
for older hosts
Olaf Hering 069948
Olaf Hering 069948
Fixes: 0ef58b0a05c1 ("hv_netvsc: change GPAD teardown order on older versions")
Olaf Hering 069948
Signed-off-by: Mohammed Gamal <mgamal@redhat.com>
Olaf Hering 069948
Signed-off-by: David S. Miller <davem@davemloft.net>
Olaf Hering 069948
Acked-by: Olaf Hering <ohering@suse.de>
Olaf Hering 069948
---
Olaf Hering 069948
 drivers/net/hyperv/netvsc.c | 19 +++++++++++++------
Olaf Hering 069948
 1 file changed, 13 insertions(+), 6 deletions(-)
Olaf Hering 069948
Olaf Hering 069948
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
Olaf Hering 069948
--- a/drivers/net/hyperv/netvsc.c
Olaf Hering 069948
+++ b/drivers/net/hyperv/netvsc.c
Olaf Hering 069948
@@ -592,8 +592,17 @@ void netvsc_device_remove(struct hv_device *device)
Olaf Hering 069948
 		= rtnl_dereference(net_device_ctx->nvdev);
Olaf Hering 069948
 	int i;
Olaf Hering 069948
 
Olaf Hering 069948
+	/*
Olaf Hering 069948
+	 * Revoke receive buffer. If host is pre-Win2016 then tear down
Olaf Hering 069948
+	 * receive buffer GPADL. Do the same for send buffer.
Olaf Hering 069948
+	 */
Olaf Hering 069948
 	netvsc_revoke_recv_buf(device, net_device);
Olaf Hering 069948
+	if (vmbus_proto_version < VERSION_WIN10)
Olaf Hering 069948
+		netvsc_teardown_recv_gpadl(device, net_device);
Olaf Hering 069948
+
Olaf Hering 069948
 	netvsc_revoke_send_buf(device, net_device);
Olaf Hering 069948
+	if (vmbus_proto_version < VERSION_WIN10)
Olaf Hering 069948
+		netvsc_teardown_send_gpadl(device, net_device);
Olaf Hering 069948
 
Olaf Hering 069948
 	RCU_INIT_POINTER(net_device_ctx->nvdev, NULL);
Olaf Hering 069948
 
Olaf Hering 069948
@@ -607,15 +616,13 @@ void netvsc_device_remove(struct hv_device *device)
Olaf Hering 069948
 	 */
Olaf Hering 069948
 	netdev_dbg(ndev, "net device safe to remove\n");
Olaf Hering 069948
 
Olaf Hering 069948
-	/* older versions require that buffer be revoked before close */
Olaf Hering 069948
-	if (vmbus_proto_version < VERSION_WIN10) {
Olaf Hering 069948
-		netvsc_teardown_recv_gpadl(device, net_device);
Olaf Hering 069948
-		netvsc_teardown_send_gpadl(device, net_device);
Olaf Hering 069948
-	}
Olaf Hering 069948
-
Olaf Hering 069948
 	/* Now, we can close the channel safely */
Olaf Hering 069948
 	vmbus_close(device->channel);
Olaf Hering 069948
 
Olaf Hering 069948
+	/*
Olaf Hering 069948
+	 * If host is Win2016 or higher then we do the GPADL tear down
Olaf Hering 069948
+	 * here after VMBus is closed.
Olaf Hering 069948
+	*/
Olaf Hering 069948
 	if (vmbus_proto_version >= VERSION_WIN10) {
Olaf Hering 069948
 		netvsc_teardown_recv_gpadl(device, net_device);
Olaf Hering 069948
 		netvsc_teardown_send_gpadl(device, net_device);