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

Groups > linux.kernel > #1395983 > unrolled thread

Re: [PATCH v9 net-next 1/2] 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 v9 net-next 1/2] hv_sock: introduce Hyper-V Sockets David Miller <davem@davemloft.net> - 2016-05-06 19:10 +0200
    RE: [PATCH v9 net-next 1/2] hv_sock: introduce Hyper-V Sockets Dexuan Cui <decui@microsoft.com> - 2016-05-07 12:50 +0200
      Re: [PATCH v9 net-next 1/2] hv_sock: introduce Hyper-V Sockets David Miller <davem@davemloft.net> - 2016-05-07 19:50 +0200
        RE: [PATCH v9 net-next 1/2] hv_sock: introduce Hyper-V Sockets Dexuan Cui <decui@microsoft.com> - 2016-05-08 08:20 +0200
          Re: [PATCH v9 net-next 1/2] hv_sock: introduce Hyper-V Sockets David Miller <davem@davemloft.net> - 2016-05-08 19:50 +0200
            RE: [PATCH v9 net-next 1/2] hv_sock: introduce Hyper-V Sockets Dexuan Cui <decui@microsoft.com> - 2016-05-09 17:30 +0200

#1395983 — Re: [PATCH v9 net-next 1/2] hv_sock: introduce Hyper-V Sockets

From: Dexuan Cui <decui@microsoft.com>
Date: Wed,  4 May 2016 09:56:57 -0700

> +#define VMBUS_RINGBUFFER_SIZE_HVSOCK_RECV (5 * PAGE_SIZE)
> +#define VMBUS_RINGBUFFER_SIZE_HVSOCK_SEND (5 * PAGE_SIZE)
> +
> +#define HVSOCK_RCV_BUF_SZ	VMBUS_RINGBUFFER_SIZE_HVSOCK_RECV
 ...
> +struct hvsock_sock {
 ...
> +	/* The 'hdr' and 'buf' in the below 'send' and 'recv' definitions must
> +	 * be consecutive: see hvsock_send_data() and hvsock_recv_data().
> +	 */
> +	struct {
> +		struct vmpipe_proto_header hdr;
> +		u8 buf[HVSOCK_SND_BUF_SZ];
> +	} send;
> +
> +	struct {
> +		struct vmpipe_proto_header hdr;
> +		u8 buf[HVSOCK_RCV_BUF_SZ];
> +
> +		unsigned int data_len;
> +		unsigned int data_offset;
> +	} recv;

I don't think allocating 5 pages of unswappable memory for every Hyper-V socket
created is reasonable.

[toc] | [next] | [standalone]

#1396294

> From: David Miller [mailto:davem@davemloft.net]
> Sent: Saturday, May 7, 2016 1:04
> To: Dexuan Cui <decui@microsoft.com>
> Cc: gregkh@linuxfoundation.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com; cavery@redhat.com; KY
> Srinivasan <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>;
> joe@perches.com; vkuznets@redhat.com
> Subject: Re: [PATCH v9 net-next 1/2] hv_sock: introduce Hyper-V Sockets
> 
> From: Dexuan Cui <decui@microsoft.com>
> Date: Wed,  4 May 2016 09:56:57 -0700
> 
> > +#define VMBUS_RINGBUFFER_SIZE_HVSOCK_RECV (5 * PAGE_SIZE)
> > +#define VMBUS_RINGBUFFER_SIZE_HVSOCK_SEND (5 * PAGE_SIZE)
> > +
> > +#define HVSOCK_RCV_BUF_SZ
> 	VMBUS_RINGBUFFER_SIZE_HVSOCK_RECV
>  ...
> > +struct hvsock_sock {
>  ...
> > +	/* The 'hdr' and 'buf' in the below 'send' and 'recv' definitions must
> > +	 * be consecutive: see hvsock_send_data() and hvsock_recv_data().
> > +	 */
> > +	struct {
> > +		struct vmpipe_proto_header hdr;
> > +		u8 buf[HVSOCK_SND_BUF_SZ];
> > +	} send;
> > +
> > +	struct {
> > +		struct vmpipe_proto_header hdr;
> > +		u8 buf[HVSOCK_RCV_BUF_SZ];
> > +
> > +		unsigned int data_len;
> > +		unsigned int data_offset;
> > +	} recv;
> 
> I don't think allocating 5 pages of unswappable memory for every Hyper-V
> socket
> created is reasonable.

Thanks for the comment, David!

I should be able to make 'send', 'recv' here to pointers and use vmalloc()
to allocate the memory for them.  I will do this.

Thanks,
-- Dexuan

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

#1396338

From: Dexuan Cui <decui@microsoft.com>
Date: Sat, 7 May 2016 10:49:25 +0000

> I should be able to make 'send', 'recv' here to pointers and use vmalloc()
> to allocate the memory for them.  I will do this.

That's still unswappable kernel memory.

People can open N sockets, where N is something on the order of the FD
limit the process has, per process.  This allows someone to quickly
eat up a lot of memory and hold onto it nearly indefinitely.

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

#1396383

> From: David Miller [mailto:davem@davemloft.net]
> Sent: Sunday, May 8, 2016 1:41
> To: Dexuan Cui <decui@microsoft.com>
> Cc: gregkh@linuxfoundation.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com; cavery@redhat.com; KY
> Srinivasan <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>;
> joe@perches.com; vkuznets@redhat.com
> Subject: Re: [PATCH v9 net-next 1/2] hv_sock: introduce Hyper-V Sockets
>
> From: Dexuan Cui <decui@microsoft.com>
> Date: Sat, 7 May 2016 10:49:25 +0000
>
> > I should be able to make 'send', 'recv' here to pointers and use vmalloc()
> > to allocate the memory for them.  I will do this.
>
> That's still unswappable kernel memory.
Hi David,
My understanding is: kernel pages are not swappable in Linux, so it looks I
can't avoid unswappable kernel memory here?

> People can open N sockets, where N is something on the order of the FD
> limit the process has, per process.  This allows someone to quickly
> eat up a lot of memory and hold onto it nearly indefinitely.

Thanks for pointing this out!
I understand, so I think I should add a module parameter, e.g.,
"hv_sock.max_socket_number" with a default value, say, 1024?

1 established hv_sock connection takes less than 20 pages, including 10
pages for VMBus ringbuffer, 6 pages for send/recv buffers(I'll use
vmalloc() for this), etc.
Here the recv buf needs a size of 5 pages because potentially the host
can send the guest a VMBus packet with an up-to-5-page payload, i..e,
the VMBus inbound ringbuffer size.

1024 hv_sock connections take less than 20*4KB * 1K = 80MB memory.

A user who needs more connections can change the module parameter
without reboot.

hv_sock connection is designed  to work only between the host and the
guest. I think 1024 connections seem pretty enough.

BTW, a user can't create hv_sock connections without enough privilege.
Please see

+static int hvsock_create(struct net *net, struct socket *sock,
+                        int protocol, int kern)
+{
+       if (!capable(CAP_SYS_ADMIN) && !capable(CAP_NET_ADMIN))
+               return -EPERM;

David, does this make sense to you?

Thanks,
-- Dexuan

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

#1396444

From: Dexuan Cui <decui@microsoft.com>
Date: Sun, 8 May 2016 06:11:04 +0000

> Thanks for pointing this out!
> I understand, so I think I should add a module parameter, e.g.,
> "hv_sock.max_socket_number" with a default value, say, 1024?

No, you should get rid of the huge multi-page buffers.

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

#1397143

> From: David Miller [mailto:davem@davemloft.net]
> Sent: Monday, May 9, 2016 1:45
> To: Dexuan Cui <decui@microsoft.com>
> Cc: gregkh@linuxfoundation.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com; cavery@redhat.com; KY
> Srinivasan <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>;
> joe@perches.com; vkuznets@redhat.com
> Subject: Re: [PATCH v9 net-next 1/2] hv_sock: introduce Hyper-V Sockets
>
> From: Dexuan Cui <decui@microsoft.com>
> Date: Sun, 8 May 2016 06:11:04 +0000
>
> > Thanks for pointing this out!
> > I understand, so I think I should add a module parameter, e.g.,
> > "hv_sock.max_socket_number" with a default value, say, 1024?
>
> No, you should get rid of the huge multi-page buffers.

Hi David,
Ok, how do you like the below proof-of-concept patch snippet?

I use 1 page for the recv buf and another page for send buf. They should be
allocated by kmalloc(sizeof(struct hvsock_send/recv_buf), GFP_KERNEL).

And, by default, I use 2 pages for VMBUS send/recv ringbuffers respectively.
(Note: 2 is the minimal ringbuffer size because actually 1 page of the two is used
as the shared read/write index etc, rather than data)
A module parameter will be added to allow the user to use a big ringbuffer
size, if the user cares too much about the performance.

Another parameter will be added to limit how many hvsock sockets can be
created at most. The default value can be 1024, meaning at most
1024 * (2+2+1+1) * 4KB = 24MB memory is used.

-#define VMBUS_RINGBUFFER_SIZE_HVSOCK_RECV (5 * PAGE_SIZE)
-#define VMBUS_RINGBUFFER_SIZE_HVSOCK_SEND (5 * PAGE_SIZE)
+#define VMBUS_RINGBUFFER_SIZE_HVSOCK_RECV (2 * PAGE_SIZE)
+#define VMBUS_RINGBUFFER_SIZE_HVSOCK_SEND (2 * PAGE_SIZE)

-#define HVSOCK_RCV_BUF_SZ      VMBUS_RINGBUFFER_SIZE_HVSOCK_RECV
+#define HVSOCK_RCV_BUF_SZ      PAGE_SIZE
 #define HVSOCK_SND_BUF_SZ      PAGE_SIZE

+struct hvsock_send_buf {
+       struct vmpipe_proto_header hdr;
+       u8 buf[HVSOCK_SND_BUF_SZ];
+};
+
+struct hvsock_recv_buf {
+       struct vmpipe_proto_header hdr;
+       u8 buf[HVSOCK_RCV_BUF_SZ];
+
+       unsigned int data_len;
+       unsigned int data_offset;
+};
+
@@ -35,21 +48,8 @@ struct hvsock_sock {

        struct vmbus_channel *channel;

-       struct {
-               struct vmpipe_proto_header hdr;
-               u8 buf[HVSOCK_SND_BUF_SZ];
-       } send;
-
-       struct {
-               struct vmpipe_proto_header hdr;
-               u8 buf[HVSOCK_RCV_BUF_SZ];
-
-               unsigned int data_len;
-               unsigned int data_offset;
-       } recv;
+       struct hvsock_send_buf *send_buf;
+       struct hvsock_recv_buf *recv_buf;
 };

Thanks,
-- Dexuan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web