Drivers: hv: kvp: Cleanup error handling in KVP
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 <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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 @@
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 void
-kvp_register(void)
+kvp_register(int reg_value)
{
struct cn_msg *msg;
@@ -83,7 +94,7 @@
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 @@
* 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 @@
{
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 @@
*/
return;
+ icmsghdrp->status = error;
/*
* If the error parameter is set, terminate the host's enumeration
@@ -294,15 +365,12 @@
*/
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 @@
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 @@
#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..069e2b38 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -71,13 +71,14 @@
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 @@
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 @@
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 @@
char *p;
char *key_value;
char *key_name;
+ int op;
+ int pool;
daemon(1, 0);
openlog("KVP", 0, LOG_USER);
@@ -687,7 +686,7 @@
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 @@
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 @@
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 @@
* 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 @@
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;
}
/*