[PATCH 4/4] [for linux-4.4.y only] HV: properly delay KVP packets when negotiation is in progress

[PATCH 4/4] [for linux-4.4.y only] HV: properly delay KVP packets when negotiation is in progress

[Dexuan Cui]

The host may send multiple negotiation packets
(due to timeout) before the KVP user-mode daemon
is connected. KVP user-mode daemon is connected.
We need to defer processing those packets
until the daemon is negotiated and connected.
It's okay for guest to respond
to all negotiation packets.

In addition, the host may send multiple staged
KVP requests as soon as negotiation is done.
We need to properly process those packets using one
tasklet for exclusive access to ring buffer.

This patch is based on the work of
Nick Meier <Nick.Meier@microsoft.com>.

Signed-off-by: Long Li <longli@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

The above is the original changelog of
a3ade8cc474d ("HV: properly delay KVP packets when negotiation is in progress"

Here I re-worked the original patch because the mainline version
can't work for the linux-4.4.y branch, on which channel->callback_event
doesn't exist yet. In the mainline, channel->callback_event was added by:
631e63a9f346 ("vmbus: change to per channel tasklet"). Here we don't want
to backport it to v4.4, as it requires extra supporting changes and fixes,
which are unnecessary as to the KVP bug we're trying to resolve.

NOTE: before this patch is used, we should cherry-pick the other related
3 patches from the mainline first:

The background of this backport request is that: recently Wang Jian reported
some KVP issues: https://github.com/LIS/lis-next/issues/593:
e.g. the /var/lib/hyperv/.kvp_pool_* files can not be updated, and sometimes
if the hv_kvp_daemon doesn't timely start, the host may not be able to query
the VM's IP address via KVP.

Reported-by: Wang Jian <jianjian.wang1@gmail.com>
Tested-by: Wang Jian <jianjian.wang1@gmail.com>
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---

This is re-worked by me from the mainline:
a3ade8cc474d ("HV: properly delay KVP packets when negotiation is in progress"

I added my Signed-off-by as I identified and tested the patches.
If this is unnecessary, please feel free to remove it.

 drivers/hv/hv_kvp.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index ff0a426..1771a96 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -612,21 +612,22 @@ void hv_kvp_onchannelcallback(void *context)
 		     NEGO_IN_PROGRESS,
 		     NEGO_FINISHED} host_negotiatied = NEGO_NOT_STARTED;
 
-	if (host_negotiatied == NEGO_NOT_STARTED &&
-	    kvp_transaction.state < HVUTIL_READY) {
+	if (kvp_transaction.state < HVUTIL_READY) {
 		/*
 		 * If userspace daemon is not connected and host is asking
 		 * us to negotiate we need to delay to not lose messages.
 		 * This is important for Failover IP setting.
 		 */
-		host_negotiatied = NEGO_IN_PROGRESS;
-		schedule_delayed_work(&kvp_host_handshake_work,
+		if (host_negotiatied == NEGO_NOT_STARTED) {
+			host_negotiatied = NEGO_IN_PROGRESS;
+			schedule_delayed_work(&kvp_host_handshake_work,
 				      HV_UTIL_NEGO_TIMEOUT * HZ);
+		}
 		return;
 	}
 	if (kvp_transaction.state > HVUTIL_READY)
 		return;
-
+recheck:
 	vmbus_recvpacket(channel, recv_buffer, PAGE_SIZE * 4, &recvlen,
 			 &requestid);
 
@@ -703,6 +704,8 @@ void hv_kvp_onchannelcallback(void *context)
 				       VM_PKT_DATA_INBAND, 0);
 
 		host_negotiatied = NEGO_FINISHED;
+
+		goto recheck;
 	}
 
 }
-- 
2.7.4

Re: [PATCH 4/4] [for linux-4.4.y only] HV: properly delay KVP packets when negotiation is in progress

['gregkh@linuxfoundation.org']

On Wed, Oct 17, 2018 at 01:23:26AM +0000, Dexuan Cui wrote:
> 
> The host may send multiple negotiation packets
> (due to timeout) before the KVP user-mode daemon
> is connected. KVP user-mode daemon is connected.
> We need to defer processing those packets
> until the daemon is negotiated and connected.
> It's okay for guest to respond
> to all negotiation packets.
> 
> In addition, the host may send multiple staged
> KVP requests as soon as negotiation is done.
> We need to properly process those packets using one
> tasklet for exclusive access to ring buffer.
> 
> This patch is based on the work of
> Nick Meier <Nick.Meier@microsoft.com>.
> 
> Signed-off-by: Long Li <longli@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> The above is the original changelog of
> a3ade8cc474d ("HV: properly delay KVP packets when negotiation is in progress"
> 
> Here I re-worked the original patch because the mainline version
> can't work for the linux-4.4.y branch, on which channel->callback_event
> doesn't exist yet. In the mainline, channel->callback_event was added by:
> 631e63a9f346 ("vmbus: change to per channel tasklet"). Here we don't want
> to backport it to v4.4, as it requires extra supporting changes and fixes,
> which are unnecessary as to the KVP bug we're trying to resolve.
> 
> NOTE: before this patch is used, we should cherry-pick the other related
> 3 patches from the mainline first:
> 
> The background of this backport request is that: recently Wang Jian reported
> some KVP issues: https://github.com/LIS/lis-next/issues/593:
> e.g. the /var/lib/hyperv/.kvp_pool_* files can not be updated, and sometimes
> if the hv_kvp_daemon doesn't timely start, the host may not be able to query
> the VM's IP address via KVP.
> 
> Reported-by: Wang Jian <jianjian.wang1@gmail.com>
> Tested-by: Wang Jian <jianjian.wang1@gmail.com>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>


This should also be added to 4.9.y, right?

thanks,

greg k-h

RE: [PATCH 4/4] [for linux-4.4.y only] HV: properly delay KVP packets when negotiation is in progress

[Dexuan Cui]

> From: 'gregkh@linuxfoundation.org' <gregkh@linuxfoundation.org>
> Sent: Thursday, October 18, 2018 10:08
> 
> This should also be added to 4.9.y, right?
> 
> greg k-h

According to my test against the latest 4.9.y (4.9.134), yes, 4.9.y does
need this patch (and doesn't need the other 3 patches).

Thanks for noticing this!

-- Dexuan