Messages in this thread | | | Subject | Re: [PATCH v2] Drivers: hv: kvp: fix IP Failover | From | Cathy Avery <> | Date | Wed, 30 Mar 2016 08:33:35 -0400 |
| |
Sorry acking wrong email.
Thanks,
Cathy
On 03/30/2016 08:21 AM, Cathy Avery wrote: > > > 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>
| |