Re: [PATCH net-next 3/3] hv_sock: implements Hyper-V transport for Virtual Sockets (AF_VSOCK)

2017-08-24 Thread Stefan Hajnoczi
On Tue, Aug 22, 2017 at 09:40:01PM +, Dexuan Cui wrote:
> > From: Stefan Hajnoczi [mailto:stefa...@redhat.com]
> > On Fri, Aug 18, 2017 at 10:23:54PM +, Dexuan Cui wrote:
> > > > > +static bool hvs_stream_allow(u32 cid, u32 port)
> > > > > +{
> > > > > + static const u32 valid_cids[] = {
> > > > > + VMADDR_CID_ANY,
> > > >
> > > > Is this for loopback?
> > >
> > > No, we don't support lookback in Linux VM, at least for now.
> > > In our Linux implementation, Linux VM can only connect to the host, and
> > > here when Linux VM calls connect(), I treat  VMADDR_CID_ANY
> > > the same as VMADDR_CID_HOST.
> > 
> > VMCI and virtio-vsock do not treat connect(VMADDR_CID_ANY) the same as
> > connect(VMADDR_CID_HOST).  It is an error to connect to VMADDR_CID_ANY.
> 
> Ok. Then I'll only allow VMADDR_CID_HOST as the destination CID, since 
> we don't support loopback mode.
> 
> > > > > + /* The host's port range [MIN_HOST_EPHEMERAL_PORT, 0x)
> > > > is
> > > > > +  * reserved as ephemeral ports, which are used as the host's 
> > > > > ports
> > > > > +  * when the host initiates connections.
> > > > > +  */
> > > > > + if (port > MAX_HOST_LISTEN_PORT)
> > > > > + return false;
> > > >
> > > > Without this if statement the guest will attempt to connect.  I guess
> > > > there will be no listen sockets above MAX_HOST_LISTEN_PORT, so the
> > > > connection attempt will fail.
> > >
> > > You're correct.
> > > To use the vsock common infrastructure, we have to map Hyper-V's
> > > GUID  to int , and hence we must limit
> > > the port range we can listen() on to [0, MAX_LISTEN_PORT], i.e.
> > > we can only use half of the whole 32-bit port space for listen().
> > > This is detailed in the long comments starting at about Line 100.
> > >
> > > > ...but hardcode this knowledge into the guest driver?
> > > I'd like the guest's connect() to fail immediately here.
> > > IMO this is better than a connect timeout. :-)
> > 
> > Thanks for explaining.  Perhaps the comment could be updated:
> > 
> >  /* The host's port range [MIN_HOST_EPHEMERAL_PORT, 0x) is
> >   * reserved as ephemeral ports, which are used as the host's ports when
> >   * the host initiates connections.
> >   *
> >   * Perform this check in the guest so an immediate error is produced
> >   * instead of a timeout.
> >   */
> > 
> > Stefan
> 
> Thank you, Stefan! 
> Please see the below for the updated version of the function:
> 
> static bool hvs_stream_allow(u32 cid, u32 port)
> {
> /* The host's port range [MIN_HOST_EPHEMERAL_PORT, 0x) is
>  * reserved as ephemeral ports, which are used as the host's ports
>  * when the host initiates connections.
>  *
>  * Perform this check in the guest so an immediate error is produced
>  * instead of a timeout.
>  */
> if (port > MAX_HOST_LISTEN_PORT)
> return false;
> 
> if (cid == VMADDR_CID_HOST)
> return true;
> 
> return false;
> }
> 
> I'll send a v2 of the patch later today.

Great, thanks!
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH net-next 3/3] hv_sock: implements Hyper-V transport for Virtual Sockets (AF_VSOCK)

2017-08-22 Thread Dexuan Cui
> From: Stefan Hajnoczi [mailto:stefa...@redhat.com]
> On Fri, Aug 18, 2017 at 10:23:54PM +, Dexuan Cui wrote:
> > > > +static bool hvs_stream_allow(u32 cid, u32 port)
> > > > +{
> > > > +   static const u32 valid_cids[] = {
> > > > +   VMADDR_CID_ANY,
> > >
> > > Is this for loopback?
> >
> > No, we don't support lookback in Linux VM, at least for now.
> > In our Linux implementation, Linux VM can only connect to the host, and
> > here when Linux VM calls connect(), I treat  VMADDR_CID_ANY
> > the same as VMADDR_CID_HOST.
> 
> VMCI and virtio-vsock do not treat connect(VMADDR_CID_ANY) the same as
> connect(VMADDR_CID_HOST).  It is an error to connect to VMADDR_CID_ANY.

Ok. Then I'll only allow VMADDR_CID_HOST as the destination CID, since 
we don't support loopback mode.

> > > > +   /* The host's port range [MIN_HOST_EPHEMERAL_PORT, 0x)
> > > is
> > > > +* reserved as ephemeral ports, which are used as the host's 
> > > > ports
> > > > +* when the host initiates connections.
> > > > +*/
> > > > +   if (port > MAX_HOST_LISTEN_PORT)
> > > > +   return false;
> > >
> > > Without this if statement the guest will attempt to connect.  I guess
> > > there will be no listen sockets above MAX_HOST_LISTEN_PORT, so the
> > > connection attempt will fail.
> >
> > You're correct.
> > To use the vsock common infrastructure, we have to map Hyper-V's
> > GUID  to int , and hence we must limit
> > the port range we can listen() on to [0, MAX_LISTEN_PORT], i.e.
> > we can only use half of the whole 32-bit port space for listen().
> > This is detailed in the long comments starting at about Line 100.
> >
> > > ...but hardcode this knowledge into the guest driver?
> > I'd like the guest's connect() to fail immediately here.
> > IMO this is better than a connect timeout. :-)
> 
> Thanks for explaining.  Perhaps the comment could be updated:
> 
>  /* The host's port range [MIN_HOST_EPHEMERAL_PORT, 0x) is
>   * reserved as ephemeral ports, which are used as the host's ports when
>   * the host initiates connections.
>   *
>   * Perform this check in the guest so an immediate error is produced
>   * instead of a timeout.
>   */
> 
> Stefan

Thank you, Stefan! 
Please see the below for the updated version of the function:

static bool hvs_stream_allow(u32 cid, u32 port)
{
/* The host's port range [MIN_HOST_EPHEMERAL_PORT, 0x) is
 * reserved as ephemeral ports, which are used as the host's ports
 * when the host initiates connections.
 *
 * Perform this check in the guest so an immediate error is produced
 * instead of a timeout.
 */
if (port > MAX_HOST_LISTEN_PORT)
return false;

if (cid == VMADDR_CID_HOST)
return true;

return false;
}

I'll send a v2 of the patch later today.

-- Dexuan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH net-next 3/3] hv_sock: implements Hyper-V transport for Virtual Sockets (AF_VSOCK)

2017-08-22 Thread Stefan Hajnoczi
On Fri, Aug 18, 2017 at 10:23:54PM +, Dexuan Cui wrote:
> > From: Stefan Hajnoczi [mailto:stefa...@redhat.com]
> > Sent: Thursday, August 17, 2017 07:56
> > To: Dexuan Cui 
> > On Tue, Aug 15, 2017 at 10:18:41PM +, Dexuan Cui wrote:
> > > +static u32 hvs_get_local_cid(void)
> > > +{
> > > + return VMADDR_CID_ANY;
> > > +}
> > 
> > Interesting concept: the guest never knows its CID.  This is nice from a
> > live migration perspective.  Currently VMCI and virtio adjust listen
> > socket local CIDs after migration.
> > 
> > > +static bool hvs_stream_allow(u32 cid, u32 port)
> > > +{
> > > + static const u32 valid_cids[] = {
> > > + VMADDR_CID_ANY,
> > 
> > Is this for loopback?
> 
> No, we don't support lookback in Linux VM, at least for now.
> In our Linux implementation, Linux VM can only connect to the host, and
> here when Linux VM calls connect(), I treat  VMADDR_CID_ANY 
> the same as VMADDR_CID_HOST.

VMCI and virtio-vsock do not treat connect(VMADDR_CID_ANY) the same as
connect(VMADDR_CID_HOST).  It is an error to connect to VMADDR_CID_ANY.

> > > + VMADDR_CID_HOST,
> > > + };
> > > + int i;
> > > +
> > > + /* The host's port range [MIN_HOST_EPHEMERAL_PORT, 0x)
> > is
> > > +  * reserved as ephemeral ports, which are used as the host's ports
> > > +  * when the host initiates connections.
> > > +  */
> > > + if (port > MAX_HOST_LISTEN_PORT)
> > > + return false;
> > 
> > Without this if statement the guest will attempt to connect.  I guess
> > there will be no listen sockets above MAX_HOST_LISTEN_PORT, so the
> > connection attempt will fail.
> 
> You're correct.
> To use the vsock common infrastructure, we have to map Hyper-V's
> GUID  to int , and hence we must limit
> the port range we can listen() on to [0, MAX_LISTEN_PORT], i.e.
> we can only use half of the whole 32-bit port space for listen().
> This is detailed in the long comments starting at about Line 100.
>  
> > ...but hardcode this knowledge into the guest driver?
> I'd like the guest's connect() to fail immediately here.
> IMO this is better than a connect timeout. :-)

Thanks for explaining.  Perhaps the comment could be updated:

 /* The host's port range [MIN_HOST_EPHEMERAL_PORT, 0x) is
  * reserved as ephemeral ports, which are used as the host's ports when
  * the host initiates connections.
  *
  * Perform this check in the guest so an immediate error is produced
  * instead of a timeout.
  */

Stefan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH net-next 3/3] hv_sock: implements Hyper-V transport for Virtual Sockets (AF_VSOCK)

2017-08-18 Thread Dexuan Cui
> From: Stefan Hajnoczi [mailto:stefa...@redhat.com]
> Sent: Thursday, August 17, 2017 07:56
> To: Dexuan Cui 
> On Tue, Aug 15, 2017 at 10:18:41PM +, Dexuan Cui wrote:
> > +static u32 hvs_get_local_cid(void)
> > +{
> > +   return VMADDR_CID_ANY;
> > +}
> 
> Interesting concept: the guest never knows its CID.  This is nice from a
> live migration perspective.  Currently VMCI and virtio adjust listen
> socket local CIDs after migration.
> 
> > +static bool hvs_stream_allow(u32 cid, u32 port)
> > +{
> > +   static const u32 valid_cids[] = {
> > +   VMADDR_CID_ANY,
> 
> Is this for loopback?

No, we don't support lookback in Linux VM, at least for now.
In our Linux implementation, Linux VM can only connect to the host, and
here when Linux VM calls connect(), I treat  VMADDR_CID_ANY 
the same as VMADDR_CID_HOST.

> > +   VMADDR_CID_HOST,
> > +   };
> > +   int i;
> > +
> > +   /* The host's port range [MIN_HOST_EPHEMERAL_PORT, 0x)
> is
> > +* reserved as ephemeral ports, which are used as the host's ports
> > +* when the host initiates connections.
> > +*/
> > +   if (port > MAX_HOST_LISTEN_PORT)
> > +   return false;
> 
> Without this if statement the guest will attempt to connect.  I guess
> there will be no listen sockets above MAX_HOST_LISTEN_PORT, so the
> connection attempt will fail.

You're correct.
To use the vsock common infrastructure, we have to map Hyper-V's
GUID  to int , and hence we must limit
the port range we can listen() on to [0, MAX_LISTEN_PORT], i.e.
we can only use half of the whole 32-bit port space for listen().
This is detailed in the long comments starting at about Line 100.
 
> ...but hardcode this knowledge into the guest driver?
I'd like the guest's connect() to fail immediately here.
IMO this is better than a connect timeout. :-)

Thanks,
-- Dexuan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH net-next 3/3] hv_sock: implements Hyper-V transport for Virtual Sockets (AF_VSOCK)

2017-08-18 Thread Stefan Hajnoczi
On Tue, Aug 15, 2017 at 10:18:41PM +, Dexuan Cui wrote:
> +static u32 hvs_get_local_cid(void)
> +{
> + return VMADDR_CID_ANY;
> +}

Interesting concept: the guest never knows its CID.  This is nice from a
live migration perspective.  Currently VMCI and virtio adjust listen
socket local CIDs after migration.

> +static bool hvs_stream_allow(u32 cid, u32 port)
> +{
> + static const u32 valid_cids[] = {
> + VMADDR_CID_ANY,

Is this for loopback?

> + VMADDR_CID_HOST,
> + };
> + int i;
> +
> + /* The host's port range [MIN_HOST_EPHEMERAL_PORT, 0x) is
> +  * reserved as ephemeral ports, which are used as the host's ports
> +  * when the host initiates connections.
> +  */
> + if (port > MAX_HOST_LISTEN_PORT)
> + return false;

Without this if statement the guest will attempt to connect.  I guess
there will be no listen sockets above MAX_HOST_LISTEN_PORT, so the
connection attempt will fail.

...but hardcode this knowledge into the guest driver?


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH net-next 3/3] hv_sock: implements Hyper-V transport for Virtual Sockets (AF_VSOCK)

2017-08-15 Thread Dexuan Cui

Hyper-V Sockets (hv_sock) supplies a byte-stream based communication
mechanism between the host and the guest. It uses VMBus ringbuffer as the
transportation layer.

With hv_sock, applications between the host (Windows 10, Windows Server
2016 or newer) and the guest can talk with each other using the traditional
socket APIs.

More info about Hyper-V Sockets is available here:

"Make your own integration services":
https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/user-guide/make-integration-service

The patch implements the necessary support in Linux guest by introducing a new
vsock transport for AF_VSOCK.

Signed-off-by: Dexuan Cui 
Cc: K. Y. Srinivasan 
Cc: Haiyang Zhang 
Cc: Stephen Hemminger 
Cc: Andy King 
Cc: Dmitry Torokhov 
Cc: George Zhang 
Cc: Jorgen Hansen 
Cc: Reilly Grant 
Cc: Asias He 
Cc: Stefan Hajnoczi 
Cc: Vitaly Kuznetsov 
Cc: Cathy Avery 
Cc: Rolf Neugebauer 
Cc: Marcelo Cerri 
---
 MAINTAINERS  |   1 +
 net/vmw_vsock/Kconfig|  12 +
 net/vmw_vsock/Makefile   |   3 +
 net/vmw_vsock/hyperv_transport.c | 890 +++
 4 files changed, 906 insertions(+)
 create mode 100644 net/vmw_vsock/hyperv_transport.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 2db0f8c..dae0573 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6279,6 +6279,7 @@ F:drivers/net/hyperv/
 F: drivers/scsi/storvsc_drv.c
 F: drivers/uio/uio_hv_generic.c
 F: drivers/video/fbdev/hyperv_fb.c
+F: net/vmw_vsock/hyperv_transport.c
 F: include/linux/hyperv.h
 F: tools/hv/
 F: Documentation/ABI/stable/sysfs-bus-vmbus
diff --git a/net/vmw_vsock/Kconfig b/net/vmw_vsock/Kconfig
index 8831e7c..a24369d 100644
--- a/net/vmw_vsock/Kconfig
+++ b/net/vmw_vsock/Kconfig
@@ -46,3 +46,15 @@ config VIRTIO_VSOCKETS_COMMON
  This option is selected by any driver which needs to access
  the virtio_vsock.  The module will be called
  vmw_vsock_virtio_transport_common.
+
+config HYPERV_VSOCKETS
+   tristate "Hyper-V transport for Virtual Sockets"
+   depends on VSOCKETS && HYPERV
+   help
+ This module implements a Hyper-V transport for Virtual Sockets.
+
+ Enable this transport if your Virtual Machine host supports Virtual
+ Sockets over Hyper-V VMBus.
+
+ To compile this driver as a module, choose M here: the module will be
+ called hv_sock. If unsure, say N.
diff --git a/net/vmw_vsock/Makefile b/net/vmw_vsock/Makefile
index 09fc2eb..e63d574 100644
--- a/net/vmw_vsock/Makefile
+++ b/net/vmw_vsock/Makefile
@@ -2,6 +2,7 @@ obj-$(CONFIG_VSOCKETS) += vsock.o
 obj-$(CONFIG_VMWARE_VMCI_VSOCKETS) += vmw_vsock_vmci_transport.o
 obj-$(CONFIG_VIRTIO_VSOCKETS) += vmw_vsock_virtio_transport.o
 obj-$(CONFIG_VIRTIO_VSOCKETS_COMMON) += vmw_vsock_virtio_transport_common.o
+obj-$(CONFIG_HYPERV_VSOCKETS) += hv_sock.o
 
 vsock-y += af_vsock.o af_vsock_tap.o vsock_addr.o
 
@@ -11,3 +12,5 @@ vmw_vsock_vmci_transport-y += vmci_transport.o 
vmci_transport_notify.o \
 vmw_vsock_virtio_transport-y += virtio_transport.o
 
 vmw_vsock_virtio_transport_common-y += virtio_transport_common.o
+
+hv_sock-y += hyperv_transport.o
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
new file mode 100644
index 000..1913b38
--- /dev/null
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -0,0 +1,890 @@
+/*
+ * Hyper-V transport for vsock
+ *
+ * Hyper-V Sockets supplies a byte-stream based communication mechanism
+ * between the host and the VM. This driver implements the necessary
+ * support in the VM by introducing the new vsock transport.
+ *
+ * Copyright (c) 2017, Microsoft Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* The host side's design of the feature requires 6 exact 4KB 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 better design in the future.
+ */
+#define PAGE_SIZE_4K   4096
+#define RINGBUFFER_HVS_RCV_SIZE (PAGE_SIZE_4K * 6)
+#define RINGBUFFER_HVS_SND_SIZE (PAGE_SIZE_4K * 6)
+
+/* The MTU is 16KB per the host side's design */
+#define HVS_MTU_SIZE   (1024 * 16)
+
+struct vmpipe_proto_header {
+   u32 pkt_type;
+   u32 data_size;
+};
+
+/* For recv, we use the VMBus in-place packet iterator APIs to directly copy
+ * data from the ringbuffer into the userspace buffer.
+ */
+struct hvs_recv_buf {
+   /* The hea