| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <56FBC79F.9040104@redhat.com>
Date: Wed, 30 Mar 2016 08:33:35 -0400
From: Cathy Avery <cavery@...hat.com>
To: Vitaly Kuznetsov <vkuznets@...hat.com>,
devel@...uxdriverproject.org
Cc: linux-kernel@...r.kernel.org,
"K. Y. Srinivasan" <kys@...rosoft.com>,
Haiyang Zhang <haiyangz@...rosoft.com>,
Alex Ng <alexng@...rosoft.com>,
Radim Krcmar <rkrcmar@...hat.com>
Subject: Re: [PATCH v2] Drivers: hv: kvp: fix IP Failover
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@...hat.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@...hat.com>
Powered by blists - more mailing lists