[PATCH v16 net-next 0/1] introduce Hyper-V VM Sockets(hv_sock)

* [PATCH v16 net-next 0/1] introduce Hyper-V VM Sockets(hv_sock)
@ 2016-07-11 14:54 Dexuan Cui
  2016-07-13  7:30 ` Michal Kubecek
  0 siblings, 1 reply; 3+ messages in thread
From: Dexuan Cui @ 2016-07-11 14:54 UTC (permalink / raw)
  To: davem, gregkh, netdev, linux-kernel, devel, olaf, apw, jasowang,
	Vitaly Kuznetsov, Cathy Avery, KY Srinivasan, joe,
	Rolf Neugebauer
  Cc: Haiyang Zhang, Dave Scott

[-- Attachment #1: Type: text/plain, Size: 8878 bytes --]

FYI: you can also get the patch by (commit 5dde7975):
https://github.com/dcui/linux/tree/decui/hv_sock/net-next/20160711_v16

The delta changes between v15 and v16 are in the attached patch
delta_v15_vs_v16.patch.

------------------------------------------

Hyper-V Sockets (hv_sock) supplies a byte-stream based communication
mechanism between the host and the guest. It's somewhat like TCP over
VMBus, but the transportation layer (VMBus) is much simpler than IP.

With Hyper-V Sockets, applications between the host and the guest can talk
to each other directly by the traditional BSD-style socket APIs.

Hyper-V Sockets is only available on new Windows hosts, like Windows Server
2016. More info is in this article "Make your own integration services":
https://msdn.microsoft.com/en-us/virtualization/hyperv_on_windows/develop/make_mgmt_service

The patch implements the necessary support in the guest side by
introducing a new socket address family AF_HYPERV.

Note: the VMBus driver side's supporting patches have been in the mainline
tree.

I know the kernel has already had a VM Sockets driver (AF_VSOCK) based
on VMware VMCI (net/vmw_vsock/, drivers/misc/vmw_vmci), and KVM is
proposing AF_VSOCK of virtio version:
http://marc.info/?l=linux-netdev&m=145952064004765&w=2

However, though Hyper-V Sockets may seem conceptually similar to
AF_VOSCK, there are differences in the transportation layer, and IMO these
make the direct code reusing impractical:

1. In AF_VSOCK, the endpoint type is: <u32 ContextID, u32 Port>, but in
AF_HYPERV, the endpoint type is: <GUID VM_ID, GUID ServiceID>. Here GUID
is 128-bit.

2. AF_VSOCK supports SOCK_DGRAM, while AF_HYPERV doesn't.

3. AF_VSOCK supports some special sock opts, like SO_VM_SOCKETS_BUFFER_SIZE,
SO_VM_SOCKETS_BUFFER_MIN/MAX_SIZE and SO_VM_SOCKETS_CONNECT_TIMEOUT.
These are meaningless to AF_HYPERV.

4. Some AF_VSOCK's VMCI transportation ops are meanless to AF_HYPERV/VMBus,
like .notify_recv_init
.notify_recv_pre_block
.notify_recv_pre_dequeue
.notify_recv_post_dequeue
.notify_send_init
.notify_send_pre_block
.notify_send_pre_enqueue
.notify_send_post_enqueue
etc.

So I think we'd better introduce a new address family: AF_HYPERV.

Please review the patch.

Looking forward to your comments, especially comments from David. :-)

Changes since v1:
- updated "[PATCH 6/7] hvsock: introduce Hyper-V VM Sockets feature"
- added __init and __exit for the module init/exit functions
- net/hv_sock/Kconfig: "default m" -> "default m if HYPERV"
- MODULE_LICENSE: "Dual MIT/GPL" -> "Dual BSD/GPL"

Changes since v2:
- fixed various coding issue pointed out by David Miller
- fixed indentation issues
- removed pr_debug in net/hv_sock/af_hvsock.c
- used reverse-Chrismas-tree style for local variables.
- EXPORT_SYMBOL -> EXPORT_SYMBOL_GPL

Changes since v3:
- fixed a few coding issue pointed by Vitaly Kuznetsov and Dan Carpenter
- fixed the ret value in vmbus_recvpacket_hvsock on error
- fixed the style of multi-line comment: vmbus_get_hvsock_rw_status()

Changes since v4 (https://lkml.org/lkml/2015/7/28/404):
- addressed all the comments about V4.
- treat the hvsock offers/channels as special VMBus devices
- add a mechanism to pass hvsock events to the hvsock driver
- fixed some corner cases with proper locking when a connection is closed
- rebased to the latest Greg's tree

Changes since v5 (https://lkml.org/lkml/2015/12/24/103):
- addressed the coding style issues (Vitaly Kuznetsov & David Miller, thanks!)
- used a better coding for the per-channel rescind callback (Thank Vitaly!)
- avoided the introduction of new VMBUS driver APIs vmbus_sendpacket_hvsock()
and vmbus_recvpacket_hvsock() and used vmbus_sendpacket()/vmbus_recvpacket()
in the higher level (i.e., the vmsock driver). Thank Vitaly!

Changes since v6 (http://lkml.iu.edu/hypermail/linux/kernel/1601.3/01813.html)
- only a few minor changes of coding style and comments

Changes since v7
- a few minor changes of coding style: thanks, Joe Perches!
- added some lines of comments about GUID/UUID before the struct sockaddr_hv.

Changes since v8
- removed the unnecessary __packed for some definitions: thanks, David!
- hvsock_open_connection:  use offer.u.pipe.user_def[0] to know the connection
and reorganized the function
direction 
- reorganized the code according to suggestions from Cathy Avery: split big
functions into small ones, set .setsockopt and getsockopt to
sock_no_setsockopt/sock_no_getsockopt
- inline'd some small list helper functions

Changes since v9
- minimized struct hvsock_sock by making the send/recv buffers pointers.
   the buffers are allocated by kmalloc() in __hvsock_create() now.
- minimized the sizes of the send/recv buffers and the vmbus ringbuffers.

Changes since v10

1) add module params: send_ring_page, recv_ring_page. They can be used to
enlarge the ringbuffer size to get better performance, e.g.,
# modprobe hv_sock  recv_ring_page=16 send_ring_page=16
By default, recv_ring_page is 3 and send_ring_page is 2.

2) add module param max_socket_number (the default is 1024).
A user can enlarge the number to create more than 1024 hv_sock sockets.
By default, 1024 sockets take about 1024 * (3+2+1+1) * 4KB = 28M bytes.
(Here 1+1 means 1 page for send/recv buffers per connection, respectively.)

3) implement the TODO in hvsock_shutdown().

4) fix a bug in hvsock_close_connection():
   I remove "sk->sk_socket->state = SS_UNCONNECTED;" -- actually this line
is not really useful. For a connection triggered by a host app's connect(),
sk->sk_socket remains NULL before the connection is accepted by the server
app (in Linux VM): see hvsock_accept() -> hvsock_accept_wait() ->
sock_graft(connected, newsock). If the host app exits before the server
app's accept() returns, the host can send a rescind-message to close the
connection and later in the Linux VM's message handler 
i.e. vmbus_onoffer_rescind()), Linux will get a NULL de-referencing crash. 

5) fix a bug in hvsock_open_connection()
  I move the vmbus_set_chn_rescind_callback() to a later place, because
when vmbus_open() fails, hvsock_close_connection() can do nothing and we
count on vmbus_onoffer_rescind() -> vmbus_device_unregister() to clean up
the device.

6) some stylistic modificiation.


Changes since v11:
1) remove the module params as David suggested.

2) use 5 exact pages for VMBus send/recv rings, respectively.
The host side's design of the feature requires 5 exact pages for recv/send
rings respectively -- this is suboptimal considering memory consumption,
however unluckily we have to live with it, before the host comes up with
a new design in the future. :-(

3) remove the per-connection static send/recv buffers
Instead, we allocate and free the buffers dynamically only when we recv/send
data. This means: when a connection is idle, no memory is consumed as
recv/send buffers at all.
Dexuan Cui (1):
  hv_sock: introduce Hyper-V Sockets

Changes since v12:
return ENOMEM on buffer alllocation failure

   Actually "man read/write" says "Other errors may occur, depending on the
object connected to fd". "man send/recv" indeed lists ENOMEM.
   Considering AF_HYPERV is a new socket type, ENOMEM seems OK here.
   In the long run, I think we should add a new API in the VMBus driver,
allowing data copy from VMBus ringbuffer into user mode buffer directly.
This way, we can even eliminate this temporary buffer.

Changes since v13:
fix some coding style issues pointed out by David.

Changes since v14:
Just some stylistic changes addressing comments from Joe Perches and
Olaf Hering -- thank you!
- add a GPL blurb.
- define a new macro PAGE_SIZE_4K and use it to replace PAGE_SIZE
- change sk_to_hvsock/hvsock_to_sk() from macros to inline functions
- remove a not-very-useful pr_err()
- fix some typos in comment and coding style issues.

Changes since v15:
Made stylistic changes addressing comments from Vitaly Kuznetsov.
Thank you very much for the detailed comments, Vitaly! 

Dexuan Cui (1):
  hv_sock: introduce Hyper-V Sockets

 MAINTAINERS                 |    2 +
 include/linux/hyperv.h      |   13 +
 include/linux/socket.h      |    4 +-
 include/net/af_hvsock.h     |   78 +++
 include/uapi/linux/hyperv.h |   23 +
 net/Kconfig                 |    1 +
 net/Makefile                |    1 +
 net/hv_sock/Kconfig         |   10 +
 net/hv_sock/Makefile        |    3 +
 net/hv_sock/af_hvsock.c     | 1509 +++++++++++++++++++++++++++++++++++++++++++
 10 files changed, 1643 insertions(+), 1 deletion(-)
 create mode 100644 include/net/af_hvsock.h
 create mode 100644 net/hv_sock/Kconfig
 create mode 100644 net/hv_sock/Makefile
 create mode 100644 net/hv_sock/af_hvsock.c

-- 
2.1.0

[-- Attachment #2: delta_v15_vs_v16.patch --]
[-- Type: application/octet-stream, Size: 7849 bytes --]

diff --git a/include/uapi/linux/hyperv.h b/include/uapi/linux/hyperv.h
index d942996..eb3e44b 100644
--- a/include/uapi/linux/hyperv.h
+++ b/include/uapi/linux/hyperv.h
@@ -405,12 +405,11 @@ struct hv_kvp_ip_msg {
  * the first 3 fields. Refer to:
  * https://en.wikipedia.org/wiki/Globally_unique_identifier
  */
-#define guid_t uuid_le
 struct sockaddr_hv {
 	__kernel_sa_family_t	shv_family;  /* Address family		*/
 	u16		reserved;	     /* Must be Zero		*/
-	guid_t		shv_vm_id;	     /* VM ID			*/
-	guid_t		shv_service_id;	     /* Service ID		*/
+	uuid_le		shv_vm_guid;	     /* VM ID			*/
+	uuid_le		shv_service_guid;    /* Service ID		*/
 };
 
 #define SHV_VMID_GUEST	NULL_UUID_LE
diff --git a/net/hv_sock/Kconfig b/net/hv_sock/Kconfig
index 1f41848..ff84875 100644
--- a/net/hv_sock/Kconfig
+++ b/net/hv_sock/Kconfig
@@ -3,8 +3,8 @@ config HYPERV_SOCK
 	depends on HYPERV
 	default m if HYPERV
 	help
-	  Hyper-V Sockets is somewhat like TCP over VMBus, allowing
-	  communication between Linux guest and Hyper-V host without TCP/IP.
+	  Hyper-V Sockets is a socket interface for high speed
+	  communication between Linux guest and Hyper-V host over VMBus.
 
 	  To compile this driver as a module, choose M here: the module
 	  will be called hv_sock.
diff --git a/net/hv_sock/af_hvsock.c b/net/hv_sock/af_hvsock.c
index f339f38..d86b879 100644
--- a/net/hv_sock/af_hvsock.c
+++ b/net/hv_sock/af_hvsock.c
@@ -50,22 +50,22 @@ static struct proto hvsock_proto = {
 
 #define SS_LISTEN 255
 
+#define HVSOCK_CONNECT_TIMEOUT (30 * HZ)
+
+/* This is an artificial limit */
+#define HVSOCK_MAX_BACKLOG	128
+
 static LIST_HEAD(hvsock_bound_list);
 static LIST_HEAD(hvsock_connected_list);
 static DEFINE_MUTEX(hvsock_mutex);
 
-static bool uuid_equals(uuid_le u1, uuid_le u2)
-{
-	return !uuid_le_cmp(u1, u2);
-}
-
 static struct sock *hvsock_find_bound_socket(const struct sockaddr_hv *addr)
 {
 	struct hvsock_sock *hvsk;
 
 	list_for_each_entry(hvsk, &hvsock_bound_list, bound_list) {
-		if (uuid_equals(addr->shv_service_id,
-				hvsk->local_addr.shv_service_id))
+		if (!uuid_le_cmp(addr->shv_service_guid,
+				 hvsk->local_addr.shv_service_guid))
 			return hvsock_to_sk(hvsk);
 	}
 	return NULL;
@@ -147,7 +147,7 @@ static void hvsock_addr_init(struct sockaddr_hv *addr, uuid_le service_id)
 {
 	memset(addr, 0, sizeof(*addr));
 	addr->shv_family = AF_HYPERV;
-	addr->shv_service_id = service_id;
+	addr->shv_service_guid = service_id;
 }
 
 static int hvsock_addr_validate(const struct sockaddr_hv *addr)
@@ -166,7 +166,7 @@ static int hvsock_addr_validate(const struct sockaddr_hv *addr)
 
 static bool hvsock_addr_bound(const struct sockaddr_hv *addr)
 {
-	return !uuid_equals(addr->shv_service_id, SHV_SERVICE_ID_ANY);
+	return !!uuid_le_cmp(addr->shv_service_guid, SHV_SERVICE_ID_ANY);
 }
 
 static int hvsock_addr_cast(const struct sockaddr *addr, size_t len,
@@ -185,13 +185,13 @@ static int __hvsock_do_bind(struct hvsock_sock *hvsk,
 	struct sockaddr_hv hv_addr;
 	int ret = 0;
 
-	hvsock_addr_init(&hv_addr, addr->shv_service_id);
+	hvsock_addr_init(&hv_addr, addr->shv_service_guid);
 
 	mutex_lock(&hvsock_mutex);
 
-	if (uuid_equals(addr->shv_service_id, SHV_SERVICE_ID_ANY)) {
+	if (!uuid_le_cmp(addr->shv_service_guid, SHV_SERVICE_ID_ANY)) {
 		do {
-			uuid_le_gen(&hv_addr.shv_service_id);
+			uuid_le_gen(&hv_addr.shv_service_guid);
 		} while (hvsock_find_bound_socket(&hv_addr));
 	} else {
 		if (hvsock_find_bound_socket(&hv_addr)) {
@@ -200,7 +200,7 @@ static int __hvsock_do_bind(struct hvsock_sock *hvsk,
 		}
 	}
 
-	hvsock_addr_init(&hvsk->local_addr, hv_addr.shv_service_id);
+	hvsock_addr_init(&hvsk->local_addr, hv_addr.shv_service_guid);
 
 	sock_hold(&hvsk->sk);
 	list_add(&hvsk->bound_list, &hvsock_bound_list);
@@ -366,7 +366,7 @@ static int hvsock_bind(struct socket *sock, struct sockaddr *addr,
 	if (hvsock_addr_cast(addr, addr_len, &hv_addr) != 0)
 		return -EINVAL;
 
-	if (!uuid_equals(hv_addr->shv_vm_id, NULL_UUID_LE))
+	if (uuid_le_cmp(hv_addr->shv_vm_guid, NULL_UUID_LE))
 		return -EINVAL;
 
 	lock_sock(sk);
@@ -609,7 +609,6 @@ static unsigned int hvsock_poll(struct file *file, struct socket *sock,
 		if (!(sk->sk_shutdown & RCV_SHUTDOWN) && can_read)
 			mask |= POLLIN | POLLRDNORM;
 	} else {
-		can_read = false;
 		can_write = false;
 	}
 
@@ -654,10 +653,7 @@ static void hvsock_on_channel_cb(void *ctx)
 
 	hvsk = sk_to_hvsock(sk);
 	channel = hvsk->channel;
-	if (!channel) {
-		WARN_ONCE(1, "NULL channel! There is a programming bug.\n");
-		return;
-	}
+	BUG_ON(!channel);
 
 	get_ringbuffer_rw_status(channel, &can_read, &can_write);
 
@@ -720,7 +716,7 @@ static int hvsock_open_connection(struct vmbus_channel *channel)
 
 	if (conn_from_host) {
 		if (sk->sk_ack_backlog >= sk->sk_max_ack_backlog) {
-			ret = -EMFILE;
+			ret = -ECONNREFUSED;
 			goto out;
 		}
 
@@ -811,7 +807,7 @@ static int hvsock_connect_wait(struct socket *sock,
 	long timeout;
 
 	hvsk = sk_to_hvsock(sk);
-	timeout = 30 * HZ;
+	timeout = HVSOCK_CONNECT_TIMEOUT;
 	prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
 
 	while (sk->sk_state != SS_CONNECTED && sk->sk_err == 0) {
@@ -905,8 +901,8 @@ static int hvsock_connect(struct socket *sock, struct sockaddr *addr,
 		sk->sk_state = SS_CONNECTING;
 
 		ret = vmbus_send_tl_connect_request(
-					&hvsk->local_addr.shv_service_id,
-					&hvsk->remote_addr.shv_service_id);
+					&hvsk->local_addr.shv_service_guid,
+					&hvsk->remote_addr.shv_service_guid);
 		if (ret < 0)
 			goto out;
 
@@ -964,15 +960,12 @@ static int hvsock_accept_wait(struct sock *listener,
 		lock_sock(connected);
 		hvconnected = sk_to_hvsock(connected);
 
-		if (ret) {
-			release_sock(connected);
-			sock_put(connected);
-		} else {
+		if (!ret) {
 			newsock->state = SS_CONNECTED;
 			sock_graft(connected, newsock);
-			release_sock(connected);
-			sock_put(connected);
 		}
+		release_sock(connected);
+		sock_put(connected);
 	}
 
 out_wait:
@@ -1029,9 +1022,8 @@ static int hvsock_listen(struct socket *sock, int backlog)
 		ret = -EINVAL;
 		goto out;
 	}
-	/* This is an artificial limit */
-	if (backlog > 128)
-		backlog = 128;
+	if (backlog > HVSOCK_MAX_BACKLOG)
+		backlog = HVSOCK_MAX_BACKLOG;
 
 	hvsk = sk_to_hvsock(sk);
 	if (!hvsock_addr_bound(&hvsk->local_addr)) {
@@ -1057,7 +1049,7 @@ static int hvsock_sendmsg_wait(struct sock *sk, struct msghdr *msg,
 	DEFINE_WAIT(wait);
 	bool can_write;
 	long timeout;
-	int ret = 0;
+	int ret;
 
 	timeout = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
 	prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
@@ -1162,11 +1154,8 @@ static int hvsock_sendmsg(struct socket *sock, struct msghdr *msg,
 	if (len == 0)
 		return -EINVAL;
 
-	if (msg->msg_flags & ~MSG_DONTWAIT) {
-		pr_err("%s: unsupported flags=0x%x\n", __func__,
-		       msg->msg_flags);
+	if (msg->msg_flags & ~MSG_DONTWAIT)
 		return -EOPNOTSUPP;
-	}
 
 	sk = sock->sk;
 	hvsk = sk_to_hvsock(sk);
@@ -1201,11 +1190,10 @@ static int hvsock_sendmsg(struct socket *sock, struct msghdr *msg,
 out:
 	release_sock(sk);
 
-	/* ret is a bigger-than-0 total_written or a negative err code. */
-	if (ret == 0) {
-		WARN(1, "unexpected return value of 0\n");
-		ret = -EIO;
-	}
+	/* ret should be a bigger-than-0 total_written or a negative err
+	 * code.
+	 */
+	BUG_ON(ret == 0);
 
 	return ret;
 }
@@ -1358,7 +1346,6 @@ static int hvsock_recvmsg(struct socket *sock, struct msghdr *msg,
 
 	/* We ignore msg->addr_name/len. */
 	if (flags & ~MSG_DONTWAIT) {
-		pr_err("%s: unsupported flags=0x%x\n", __func__, flags);
 		ret = -EOPNOTSUPP;
 		goto out;
 	}
@@ -1478,8 +1465,7 @@ static int __init hvsock_init(void)
 {
 	int ret;
 
-	/* Hyper-V Sockets requires at least VMBus 4.0 */
-	if ((vmbus_proto_version >> 16) < 4)
+	if (vmbus_proto_version < VERSION_WIN10)
 		return -ENODEV;
 
 	ret = vmbus_driver_register(&hvsock_drv);

^ permalink raw reply related	[flat|nested] 3+ messages in thread