Messages in this thread | | | Subject | Re: [PATCH v2] Drivers: hv: kvp: fix IP Failover | From | Cathy Avery <> | Date | Wed, 30 Mar 2016 08:21:15 -0400 |
| |
On 03/29/2016 08:30 AM, Vitaly Kuznetsov wrote: > Hyper-V VMs can be replicated to another hosts and there is a feature to > set different IP for replicas, it is called 'Failover TCP/IP'. When > such guest starts Hyper-V host sends it KVP_OP_SET_IP_INFO message as soon > as we finish negotiation procedure. The problem is that it can happen (and > it actually happens) before userspace daemon connects and we reply with > HV_E_FAIL to the message. As there are no repetitions we fail to set the > requested IP. > > Solve the issue by postponing our reply to the negotiation message till > userspace daemon is connected. We can't wait too long as there is a > host-side timeout (cca. 75 seconds) and if we fail to reply in this time > frame the whole KVP service will become inactive. The solution is not > ideal - if it takes userspace daemon more than 60 seconds to connect > IP Failover will still fail but I don't see a solution with our current > separation between kernel and userspace parts. > > Other two modules (VSS and FCOPY) don't require such delay, leave them > untouched. > > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > Changes since v1: > - Cancel handshake timeout work on module unload. > --- > drivers/hv/hv_kvp.c | 31 +++++++++++++++++++++++++++++++ > drivers/hv/hyperv_vmbus.h | 5 +++++ > 2 files changed, 36 insertions(+) > > diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c > index 9b9b370..cb1a916 100644 > --- a/drivers/hv/hv_kvp.c > +++ b/drivers/hv/hv_kvp.c > @@ -78,9 +78,11 @@ static void kvp_send_key(struct work_struct *dummy); > > static void kvp_respond_to_host(struct hv_kvp_msg *msg, int error); > static void kvp_timeout_func(struct work_struct *dummy); > +static void kvp_host_handshake_func(struct work_struct *dummy); > static void kvp_register(int); > > static DECLARE_DELAYED_WORK(kvp_timeout_work, kvp_timeout_func); > +static DECLARE_DELAYED_WORK(kvp_host_handshake_work, kvp_host_handshake_func); > static DECLARE_WORK(kvp_sendkey_work, kvp_send_key); > > static const char kvp_devname[] = "vmbus/hv_kvp"; > @@ -130,6 +132,11 @@ static void kvp_timeout_func(struct work_struct *dummy) > hv_poll_channel(kvp_transaction.recv_channel, kvp_poll_wrapper); > } > > +static void kvp_host_handshake_func(struct work_struct *dummy) > +{ > + hv_poll_channel(kvp_transaction.recv_channel, hv_kvp_onchannelcallback); > +} > + > static int kvp_handle_handshake(struct hv_kvp_msg *msg) > { > switch (msg->kvp_hdr.operation) { > @@ -154,6 +161,12 @@ static int kvp_handle_handshake(struct hv_kvp_msg *msg) > pr_debug("KVP: userspace daemon ver. %d registered\n", > KVP_OP_REGISTER); > kvp_register(dm_reg_value); > + > + /* > + * If we're still negotiating with the host cancel the timeout > + * work to not poll the channel twice. > + */ > + cancel_delayed_work_sync(&kvp_host_handshake_work); > hv_poll_channel(kvp_transaction.recv_channel, kvp_poll_wrapper); > > return 0; > @@ -594,7 +607,22 @@ void hv_kvp_onchannelcallback(void *context) > struct icmsg_negotiate *negop = NULL; > int util_fw_version; > int kvp_srv_version; > + static enum {NEGO_NOT_STARTED, > + NEGO_IN_PROGRESS, > + NEGO_FINISHED} host_negotiatied = NEGO_NOT_STARTED; > > + if (host_negotiatied == NEGO_NOT_STARTED && > + 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, > + HV_UTIL_NEGO_TIMEOUT * HZ); > + return; > + } > if (kvp_transaction.state > HVUTIL_READY) > return; > > @@ -672,6 +700,8 @@ void hv_kvp_onchannelcallback(void *context) > vmbus_sendpacket(channel, recv_buffer, > recvlen, requestid, > VM_PKT_DATA_INBAND, 0); > + > + host_negotiatied = NEGO_FINISHED; > } > > } > @@ -708,6 +738,7 @@ hv_kvp_init(struct hv_util_service *srv) > void hv_kvp_deinit(void) > { > kvp_transaction.state = HVUTIL_DEVICE_DYING; > + cancel_delayed_work_sync(&kvp_host_handshake_work); > cancel_delayed_work_sync(&kvp_timeout_work); > cancel_work_sync(&kvp_sendkey_work); > hvutil_transport_destroy(hvt); > diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h > index 12321b9..8b07f9c 100644 > --- a/drivers/hv/hyperv_vmbus.h > +++ b/drivers/hv/hyperv_vmbus.h > @@ -36,6 +36,11 @@ > #define HV_UTIL_TIMEOUT 30 > > /* > + * Timeout for guest-host handshake for services. > + */ > +#define HV_UTIL_NEGO_TIMEOUT 60 > + > +/* > * The below CPUID leaves are present if VersionAndFeatures.HypervisorPresent > * is set by CPUID(HVCPUID_VERSION_FEATURES). > */ Acked-by: Cathy Avery <cavery@redhat.com>
| |