From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753445Ab2HMQxW (ORCPT ); Mon, 13 Aug 2012 12:53:22 -0400 Received: from p3plsmtps2ded02.prod.phx3.secureserver.net ([208.109.80.59]:60837 "HELO p3plsmtps2ded02-01.prod.phx3.secureserver.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1752989Ab2HMQxR (ORCPT ); Mon, 13 Aug 2012 12:53:17 -0400 From: "K. Y. Srinivasan" To: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, devel@linuxdriverproject.org, virtualization@lists.osdl.org, olaf@aepfle.de, apw@canonical.com, ben@decadent.org.uk Cc: "K. Y. Srinivasan" Subject: [PATCH V2 03/18] Drivers: hv: kvp: Cleanup error handling in KVP Date: Mon, 13 Aug 2012 10:06:52 -0700 Message-Id: <1344877627-21779-3-git-send-email-kys@microsoft.com> X-Mailer: git-send-email 1.7.4.1 In-Reply-To: <1344877627-21779-1-git-send-email-kys@microsoft.com> References: <1344877584-21738-1-git-send-email-kys@microsoft.com> <1344877627-21779-1-git-send-email-kys@microsoft.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org In preparation to implementing IP injection, cleanup the way we propagate and handle errors both in the driver as well as in the user level daemon. Signed-off-by: K. Y. Srinivasan Reviewed-by: Haiyang Zhang Reviewed-by: Olaf Hering Reviewed-by: Ben Hutchings --- drivers/hv/hv_kvp.c | 114 ++++++++++++++++++++++++++++++++++++--------- include/linux/hyperv.h | 17 ++++-- tools/hv/hv_kvp_daemon.c | 66 +++++++++++++------------- 3 files changed, 135 insertions(+), 62 deletions(-) diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c index 0012eed..eb4d073 100644 --- a/drivers/hv/hv_kvp.c +++ b/drivers/hv/hv_kvp.c @@ -48,13 +48,24 @@ static struct { void *kvp_context; /* for the channel callback */ } kvp_transaction; +/* + * Before we can accept KVP messages from the host, we need + * to handshake with the user level daemon. This state tracks + * if we are in the handshake phase. + */ +static bool in_hand_shake = true; + +/* + * This state maintains the version number registered by the daemon. + */ +static int dm_reg_value; + static void kvp_send_key(struct work_struct *dummy); -#define TIMEOUT_FIRED 1 static void kvp_respond_to_host(char *key, char *value, int error); static void kvp_work_func(struct work_struct *dummy); -static void kvp_register(void); +static void kvp_register(int); static DECLARE_DELAYED_WORK(kvp_work, kvp_work_func); static DECLARE_WORK(kvp_sendkey_work, kvp_send_key); @@ -68,7 +79,7 @@ static u8 *recv_buffer; */ static void -kvp_register(void) +kvp_register(int reg_value) { struct cn_msg *msg; @@ -83,7 +94,7 @@ kvp_register(void) msg->id.idx = CN_KVP_IDX; msg->id.val = CN_KVP_VAL; - kvp_msg->kvp_hdr.operation = KVP_OP_REGISTER; + kvp_msg->kvp_hdr.operation = reg_value; strcpy(version, HV_DRV_VERSION); msg->len = sizeof(struct hv_kvp_msg); cn_netlink_send(msg, 0, GFP_ATOMIC); @@ -97,9 +108,43 @@ kvp_work_func(struct work_struct *dummy) * If the timer fires, the user-mode component has not responded; * process the pending transaction. */ - kvp_respond_to_host("Unknown key", "Guest timed out", TIMEOUT_FIRED); + kvp_respond_to_host("Unknown key", "Guest timed out", HV_E_FAIL); +} + +static int kvp_handle_handshake(struct hv_kvp_msg *msg) +{ + int ret = 1; + + switch (msg->kvp_hdr.operation) { + case KVP_OP_REGISTER: + dm_reg_value = KVP_OP_REGISTER; + pr_info("KVP: IP injection functionality not available\n"); + pr_info("KVP: Upgrade the KVP daemon\n"); + break; + case KVP_OP_REGISTER1: + dm_reg_value = KVP_OP_REGISTER1; + break; + default: + pr_info("KVP: incompatible daemon\n"); + pr_info("KVP: KVP version: %d, Daemon version: %d\n", + KVP_OP_REGISTER1, msg->kvp_hdr.operation); + ret = 0; + } + + if (ret) { + /* + * We have a compatible daemon; complete the handshake. + */ + pr_info("KVP: user-mode registering done.\n"); + kvp_register(dm_reg_value); + kvp_transaction.active = false; + if (kvp_transaction.kvp_context) + hv_kvp_onchannelcallback(kvp_transaction.kvp_context); + } + return ret; } + /* * Callback when data is received from user mode. */ @@ -109,27 +154,52 @@ kvp_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp) { struct hv_kvp_msg *message; struct hv_kvp_msg_enumerate *data; + int error = 0; message = (struct hv_kvp_msg *)msg->data; - switch (message->kvp_hdr.operation) { + + /* + * If we are negotiating the version information + * with the daemon; handle that first. + */ + + if (in_hand_shake) { + if (kvp_handle_handshake(message)) + in_hand_shake = false; + return; + } + + /* + * Based on the version of the daemon, we propagate errors from the + * daemon differently. + */ + + data = &message->body.kvp_enum_data; + + switch (dm_reg_value) { case KVP_OP_REGISTER: - pr_info("KVP: user-mode registering done.\n"); - kvp_register(); - kvp_transaction.active = false; - hv_kvp_onchannelcallback(kvp_transaction.kvp_context); + /* + * Null string is used to pass back error condition. + */ + if (data->data.key[0] == 0) + error = HV_S_CONT; break; - default: - data = &message->body.kvp_enum_data; + case KVP_OP_REGISTER1: /* - * Complete the transaction by forwarding the key value - * to the host. But first, cancel the timeout. + * We use the message header information from + * the user level daemon to transmit errors. */ - if (cancel_delayed_work_sync(&kvp_work)) - kvp_respond_to_host(data->data.key, - data->data.value, - !strlen(data->data.key)); + error = message->error; + break; } + + /* + * Complete the transaction by forwarding the key value + * to the host. But first, cancel the timeout. + */ + if (cancel_delayed_work_sync(&kvp_work)) + kvp_respond_to_host(data->data.key, data->data.value, error); } static void @@ -287,6 +357,7 @@ kvp_respond_to_host(char *key, char *value, int error) */ return; + icmsghdrp->status = error; /* * If the error parameter is set, terminate the host's enumeration @@ -294,15 +365,12 @@ kvp_respond_to_host(char *key, char *value, int error) */ if (error) { /* - * Something failed or the we have timedout; - * terminate the current host-side iteration. + * Something failed or we have timedout; + * terminate the current host-side iteration. */ - icmsghdrp->status = HV_S_CONT; goto response_done; } - icmsghdrp->status = HV_S_OK; - kvp_msg = (struct hv_kvp_msg *) &recv_buffer[sizeof(struct vmbuspipe_hdr) + sizeof(struct icmsg_hdr)]; diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index 11afc4e..b587c44 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -181,6 +181,17 @@ enum hv_kvp_exchg_pool { KVP_POOL_COUNT /* Number of pools, must be last. */ }; +/* + * Some Hyper-V status codes. + */ + +#define HV_S_OK 0x00000000 +#define HV_E_FAIL 0x80004005 +#define HV_S_CONT 0x80070103 +#define HV_ERROR_NOT_SUPPORTED 0x80070032 +#define HV_ERROR_MACHINE_LOCKED 0x800704F7 +#define HV_ERROR_DEVICE_NOT_CONNECTED 0x8007048F + #define ADDR_FAMILY_NONE 0x00 #define ADDR_FAMILY_IPV4 0x01 #define ADDR_FAMILY_IPV6 0x02 @@ -1048,12 +1059,6 @@ void vmbus_driver_unregister(struct hv_driver *hv_driver); #define ICMSGHDRFLAG_REQUEST 2 #define ICMSGHDRFLAG_RESPONSE 4 -#define HV_S_OK 0x00000000 -#define HV_E_FAIL 0x80004005 -#define HV_S_CONT 0x80070103 -#define HV_ERROR_NOT_SUPPORTED 0x80070032 -#define HV_ERROR_MACHINE_LOCKED 0x800704F7 -#define HV_ERROR_DEVICE_NOT_CONNECTED 0x8007048F /* * While we want to handle util services as regular devices, diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c index 8fbcf7b..069e2b3 100644 --- a/tools/hv/hv_kvp_daemon.c +++ b/tools/hv/hv_kvp_daemon.c @@ -71,13 +71,14 @@ enum key_index { static char kvp_send_buffer[4096]; static char kvp_recv_buffer[4096 * 2]; static struct sockaddr_nl addr; +static int in_hand_shake = 1; static char *os_name = ""; static char *os_major = ""; static char *os_minor = ""; static char *processor_arch; static char *os_build; -static char *lic_version; +static char *lic_version = "Unknown version"; static struct utsname uts_buf; @@ -394,7 +395,7 @@ static int kvp_get_value(int pool, __u8 *key, int key_size, __u8 *value, return 1; } -static void kvp_pool_enumerate(int pool, int index, __u8 *key, int key_size, +static int kvp_pool_enumerate(int pool, int index, __u8 *key, int key_size, __u8 *value, int value_size) { struct kvp_record *record; @@ -406,16 +407,12 @@ static void kvp_pool_enumerate(int pool, int index, __u8 *key, int key_size, record = kvp_file_info[pool].records; if (index >= kvp_file_info[pool].num_records) { - /* - * This is an invalid index; terminate enumeration; - * - a NULL value will do the trick. - */ - strcpy(value, ""); - return; + return 1; } memcpy(key, record[index].key, key_size); memcpy(value, record[index].value, value_size); + return 0; } @@ -646,6 +643,8 @@ int main(void) char *p; char *key_value; char *key_name; + int op; + int pool; daemon(1, 0); openlog("KVP", 0, LOG_USER); @@ -687,7 +686,7 @@ int main(void) message->id.val = CN_KVP_VAL; hv_msg = (struct hv_kvp_msg *)message->data; - hv_msg->kvp_hdr.operation = KVP_OP_REGISTER; + hv_msg->kvp_hdr.operation = KVP_OP_REGISTER1; message->ack = 0; message->len = sizeof(struct hv_kvp_msg); @@ -721,12 +720,21 @@ int main(void) incoming_cn_msg = (struct cn_msg *)NLMSG_DATA(incoming_msg); hv_msg = (struct hv_kvp_msg *)incoming_cn_msg->data; - switch (hv_msg->kvp_hdr.operation) { - case KVP_OP_REGISTER: + /* + * We will use the KVP header information to pass back + * the error from this daemon. So, first copy the state + * and set the error code to success. + */ + op = hv_msg->kvp_hdr.operation; + pool = hv_msg->kvp_hdr.pool; + hv_msg->error = HV_S_OK; + + if ((in_hand_shake) && (op == KVP_OP_REGISTER1)) { /* * Driver is registering with us; stash away the version * information. */ + in_hand_shake = 0; p = (char *)hv_msg->body.kvp_register.version; lic_version = malloc(strlen(p) + 1); if (lic_version) { @@ -737,44 +745,39 @@ int main(void) syslog(LOG_ERR, "malloc failed"); } continue; + } - /* - * The current protocol with the kernel component uses a - * NULL key name to pass an error condition. - * For the SET, GET and DELETE operations, - * use the existing protocol to pass back error. - */ - + switch (op) { case KVP_OP_SET: - if (kvp_key_add_or_modify(hv_msg->kvp_hdr.pool, + if (kvp_key_add_or_modify(pool, hv_msg->body.kvp_set.data.key, hv_msg->body.kvp_set.data.key_size, hv_msg->body.kvp_set.data.value, hv_msg->body.kvp_set.data.value_size)) - strcpy(hv_msg->body.kvp_set.data.key, ""); + hv_msg->error = HV_S_CONT; break; case KVP_OP_GET: - if (kvp_get_value(hv_msg->kvp_hdr.pool, + if (kvp_get_value(pool, hv_msg->body.kvp_set.data.key, hv_msg->body.kvp_set.data.key_size, hv_msg->body.kvp_set.data.value, hv_msg->body.kvp_set.data.value_size)) - strcpy(hv_msg->body.kvp_set.data.key, ""); + hv_msg->error = HV_S_CONT; break; case KVP_OP_DELETE: - if (kvp_key_delete(hv_msg->kvp_hdr.pool, + if (kvp_key_delete(pool, hv_msg->body.kvp_delete.key, hv_msg->body.kvp_delete.key_size)) - strcpy(hv_msg->body.kvp_delete.key, ""); + hv_msg->error = HV_S_CONT; break; default: break; } - if (hv_msg->kvp_hdr.operation != KVP_OP_ENUMERATE) + if (op != KVP_OP_ENUMERATE) goto kvp_done; /* @@ -782,13 +785,14 @@ int main(void) * both the key and the value; if not read from the * appropriate pool. */ - if (hv_msg->kvp_hdr.pool != KVP_POOL_AUTO) { - kvp_pool_enumerate(hv_msg->kvp_hdr.pool, + if (pool != KVP_POOL_AUTO) { + if (kvp_pool_enumerate(pool, hv_msg->body.kvp_enum_data.index, hv_msg->body.kvp_enum_data.data.key, HV_KVP_EXCHANGE_MAX_KEY_SIZE, hv_msg->body.kvp_enum_data.data.value, - HV_KVP_EXCHANGE_MAX_VALUE_SIZE); + HV_KVP_EXCHANGE_MAX_VALUE_SIZE)) + hv_msg->error = HV_S_CONT; goto kvp_done; } @@ -841,11 +845,7 @@ int main(void) strcpy(key_name, "ProcessorArchitecture"); break; default: - strcpy(key_value, "Unknown Key"); - /* - * We use a null key name to terminate enumeration. - */ - strcpy(key_name, ""); + hv_msg->error = HV_S_CONT; break; } /* -- 1.7.4.1