Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1436842 > unrolled thread

Re: [PATCH v14 net-next 1/1] hv_sock: introduce Hyper-V Sockets

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v14 net-next 1/1] hv_sock: introduce Hyper-V Sockets Joe Perches <joe@perches.com> - 2016-07-05 10:30 +0200
    Re: [PATCH v14 net-next 1/1] hv_sock: introduce Hyper-V Sockets Joe Perches <joe@perches.com> - 2016-07-05 11:50 +0200
      RE: [PATCH v14 net-next 1/1] hv_sock: introduce Hyper-V Sockets Dexuan Cui <decui@microsoft.com> - 2016-07-05 12:50 +0200
    RE: [PATCH v14 net-next 1/1] hv_sock: introduce Hyper-V Sockets Dexuan Cui <decui@microsoft.com> - 2016-07-05 12:10 +0200

#1436842 — Re: [PATCH v14 net-next 1/1] hv_sock: introduce Hyper-V Sockets

On Thu, 2016-06-30 at 15:58 +0000, Dexuan Cui wrote:
> 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.

trivia:

> diff --git a/include/net/af_hvsock.h b/include/net/af_hvsock.h
[]
> @@ -0,0 +1,59 @@
[]
> +#define sk_to_hvsock(__sk)   ((struct hvsock_sock *)(__sk))
> +#define hvsock_to_sk(__hvsk) ((struct sock *)(__hvsk))

Might as well be static inlines

> +/* We send at most 4KB payload per VMBus packet. */
> +struct hvsock_send_buf {
> +	struct vmpipe_proto_header hdr;
> +	u8 buf[PAGE_SIZE];

PAGE_SIZE might not be the right define here if
the comment is to be believed.

> diff --git a/include/uapi/linux/hyperv.h b/include/uapi/linux/hyperv.h
[]
> @@ -396,4 +397,27 @@ struct hv_kvp_ip_msg {
>  	struct hv_kvp_ipaddr_value      kvp_ip_val;
>  } __attribute__((packed));
>  
> +/* This is the address fromat of Hyper-V Sockets.

format

> diff --git a/net/hv_sock/af_hvsock.c b/net/hv_sock/af_hvsock.c
[]
> @@ -0,0 +1,1519 @@
> +/*
> + * Hyper-V Sockets -- a socket-based communication channel between the
> + * Hyper-V host and the virtual machines running on it.
> + *
> + * Copyright(c) 2016, Microsoft Corporation. All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + * 3. The name of the author may not be used to endorse or promote
> + *    products derived from this software without specific prior written
> + *    permission.
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
> + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT,
> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
> + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
> + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING
> + * IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */

Is this license GPL compatible?

> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static struct proto hvsock_proto = {
> +	.name = "HV_SOCK",
> +	.owner = THIS_MODULE,
> +	.obj_size = sizeof(struct hvsock_sock),
> +};
	
const?

> +static int hvsock_recvmsg_wait(struct sock *sk, struct msghdr *msg,
> +			       size_t len, int flags)
> +{
[]
> +				if (ret != 0 || payload_len >
> +						sizeof(hvsk->recv->buf)) {

This could look nicer as

				if (ret != 0 ||
				    payload_len > sizeof(hvsk->recv->buf)) {

[toc] | [next] | [standalone]

#1436871

On Tue, 2016-07-05 at 09:31 +0000, Dexuan Cui wrote:

> > > +/* This is the address fromat of Hyper-V Sockets.
> > format
> I suppose you meant I should change 
> /* This is ...
> to 
> /*
>   * This is ...
> I'll fix this.

No, I just meant fromat should  be format

[toc] | [prev] | [next] | [standalone]

#1436915

> From: Joe Perches [mailto:joe@perches.com]
> Sent: Tuesday, July 5, 2016 17:39
> To: Dexuan Cui <decui@microsoft.com>; davem@davemloft.net;
> gregkh@linuxfoundation.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com; Vitaly Kuznetsov
> <vkuznets@redhat.com>; Cathy Avery <cavery@redhat.com>; KY Srinivasan
> <kys@microsoft.com>
> Cc: Haiyang Zhang <haiyangz@microsoft.com>; Rolf Neugebauer
> <rolf.neugebauer@docker.com>
> Subject: Re: [PATCH v14 net-next 1/1] hv_sock: introduce Hyper-V Sockets
> 
> On Tue, 2016-07-05 at 09:31 +0000, Dexuan Cui wrote:
> 
> > > > +/* This is the address fromat of Hyper-V Sockets.
> > > format
> > I suppose you meant I should change
> > /* This is ...
> > to
> > /*
> >   * This is ...
> > I'll fix this.
> 
> No, I just meant fromat should  be format

Oh... Got it.  Thanks!
I'll fix the typo.

-- Dexuan

[toc] | [prev] | [next] | [standalone]

#1436882

> From: Joe Perches [mailto:joe@perches.com]
> 
> > +#define sk_to_hvsock(__sk)   ((struct hvsock_sock *)(__sk))
> > +#define hvsock_to_sk(__hvsk) ((struct sock *)(__hvsk))
> 
> Might as well be static inlines
Hi Joe,
Thank you for the suggestions (again)! :-)

I'll change them to static inlines.

> > +/* We send at most 4KB payload per VMBus packet. */
> > +struct hvsock_send_buf {
> > +	struct vmpipe_proto_header hdr;
> > +	u8 buf[PAGE_SIZE];
> 
> PAGE_SIZE might not be the right define here if
> the comment is to be believed.

I'll change to something like this:

+#define HVSOCK_MAX_SND_SIZE_BY_VM (1024 * 4)
 struct hvsock_send_buf {
        struct vmpipe_proto_header hdr;
-       u8 buf[PAGE_SIZE];
+       u8 buf[HVSOCK_MAX_SND_SIZE_BY_VM];
 };

 > > diff --git a/include/uapi/linux/hyperv.h b/include/uapi/linux/hyperv.h
> []
> > @@ -396,4 +397,27 @@ struct hv_kvp_ip_msg {
> >  	struct hv_kvp_ipaddr_value      kvp_ip_val;
> >  } __attribute__((packed));
> >
> > +/* This is the address fromat of Hyper-V Sockets.
> 
> format
I suppose you meant I should change 
/* This is ...
to 
/*
  * This is ...
I'll fix this.

> > diff --git a/net/hv_sock/af_hvsock.c b/net/hv_sock/af_hvsock.c
> []
> > @@ -0,0 +1,1519 @@
> > +/*
> > + * Hyper-V Sockets -- a socket-based communication channel between the
> > + * Hyper-V host and the virtual machines running on it.
> > + *
> > + * Copyright(c) 2016, Microsoft Corporation. All rights reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions
> > + * are met
> .....
> Is this license GPL compatible?
Yes. At the end of the file, there is a line
+MODULE_LICENSE("Dual BSD/GPL");

> > +static struct proto hvsock_proto = {
> > +	.name = "HV_SOCK",
> > +	.owner = THIS_MODULE,
> > +	.obj_size = sizeof(struct hvsock_sock),
> > +};
> 
> const?
No. In hvsock_create(), hvsock_proto is passed to sk_alloc(), which requires
a non-const argument. 

> > +static int hvsock_recvmsg_wait(struct sock *sk, struct msghdr *msg,
> > +			       size_t len, int flags)
> > +{
> []
> > +				if (ret != 0 || payload_len >
> > +						sizeof(hvsk->recv->buf)) {
> 
> This could look nicer as
> 
> 				if (ret != 0 ||
> 				    payload_len > sizeof(hvsk->recv->buf)) {
I'll fix this.

Thanks,
-- Dexuan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web