Re: [PATCH v2 8/8] vsock/virtio: make the RX buffer size tunable

2019-05-14 Thread Stefano Garzarella 
On Mon, May 13, 2019 at 08:46:19PM +0800, Jason Wang wrote:
> 
> On 2019/5/13 下午6:05, Jason Wang wrote:
> > 
> > On 2019/5/10 下午8:58, Stefano Garzarella wrote:
> > > The RX buffer size determines the memory consumption of the
> > > vsock/virtio guest driver, so we make it tunable through
> > > a module parameter.
> > > 
> > > The size allowed are between 4 KB and 64 KB in order to be
> > > compatible with old host drivers.
> > > 
> > > Suggested-by: Stefan Hajnoczi 
> > > Signed-off-by: Stefano Garzarella 
> > 
> > 
> > I don't see much value of doing this through kernel command line. We
> > should deal with them automatically like what virtio-net did. Or even a
> > module parameter is better.
> > 
> > Thanks
> 
> 
> Sorry, I misread the patch. But even module parameter is something not
> flexible enough. We should deal with them transparently.
> 

Okay, I'll try to understand how we can automatically adapt the RX
buffer size. Since the flow is stream based, the receiver doesn't know the
original packet size.

Maybe I can reuse the EWMA approach to understand if the buffers are
entirely filled or not.
In that case I can increase (e.g. double) or decrease the size.

I'll try to do it!

Thanks,
Stefano
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 8/8] vsock/virtio: make the RX buffer size tunable

2019-05-13 Thread Jason Wang 


On 2019/5/13 下午6:05, Jason Wang wrote:


On 2019/5/10 下午8:58, Stefano Garzarella wrote:

The RX buffer size determines the memory consumption of the
vsock/virtio guest driver, so we make it tunable through
a module parameter.

The size allowed are between 4 KB and 64 KB in order to be
compatible with old host drivers.

Suggested-by: Stefan Hajnoczi 
Signed-off-by: Stefano Garzarella 



I don't see much value of doing this through kernel command line. We 
should deal with them automatically like what virtio-net did. Or even 
a module parameter is better.


Thanks



Sorry, I misread the patch. But even module parameter is something not 
flexible enough. We should deal with them transparently.


Thanks






---
  include/linux/virtio_vsock.h |  1 +
  net/vmw_vsock/virtio_transport.c | 27 ++-
  2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index 5a9d25be72df..b9f8c3d91f80 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -13,6 +13,7 @@
  #define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE    (1024 * 64)
  #define VIRTIO_VSOCK_MAX_BUF_SIZE    0xUL
  #define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE    (1024 * 64)
+#define VIRTIO_VSOCK_MIN_PKT_BUF_SIZE    (1024 * 4)
    enum {
  VSOCK_VQ_RX = 0, /* for host to guest data */
diff --git a/net/vmw_vsock/virtio_transport.c 
b/net/vmw_vsock/virtio_transport.c

index af1d2ce12f54..732398b4e28f 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -66,6 +66,31 @@ struct virtio_vsock {
  u32 guest_cid;
  };
  +static unsigned int rx_buf_size = VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE;
+
+static int param_set_rx_buf_size(const char *val, const struct 
kernel_param *kp)

+{
+    unsigned int size;
+    int ret;
+
+    ret = kstrtouint(val, 0, );
+    if (ret)
+    return ret;
+
+    if (size < VIRTIO_VSOCK_MIN_PKT_BUF_SIZE ||
+    size > VIRTIO_VSOCK_MAX_PKT_BUF_SIZE)
+    return -EINVAL;
+
+    return param_set_uint(val, kp);
+};
+
+static const struct kernel_param_ops param_ops_rx_buf_size = {
+    .set = param_set_rx_buf_size,
+    .get = param_get_uint,
+};
+
+module_param_cb(rx_buf_size, _ops_rx_buf_size, _buf_size, 
0644);

+
  static struct virtio_vsock *virtio_vsock_get(void)
  {
  return the_virtio_vsock;
@@ -261,7 +286,7 @@ virtio_transport_cancel_pkt(struct vsock_sock *vsk)
    static void virtio_vsock_rx_fill(struct virtio_vsock *vsock)
  {
-    int buf_len = VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE;
+    int buf_len = rx_buf_size;
  struct virtio_vsock_pkt *pkt;
  struct scatterlist hdr, buf, *sgs[2];
  struct virtqueue *vq;

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 8/8] vsock/virtio: make the RX buffer size tunable

2019-05-13 Thread Jason Wang 


On 2019/5/10 下午8:58, Stefano Garzarella wrote:

The RX buffer size determines the memory consumption of the
vsock/virtio guest driver, so we make it tunable through
a module parameter.

The size allowed are between 4 KB and 64 KB in order to be
compatible with old host drivers.

Suggested-by: Stefan Hajnoczi 
Signed-off-by: Stefano Garzarella 



I don't see much value of doing this through kernel command line. We 
should deal with them automatically like what virtio-net did. Or even a 
module parameter is better.


Thanks



---
  include/linux/virtio_vsock.h |  1 +
  net/vmw_vsock/virtio_transport.c | 27 ++-
  2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index 5a9d25be72df..b9f8c3d91f80 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -13,6 +13,7 @@
  #define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE  (1024 * 64)
  #define VIRTIO_VSOCK_MAX_BUF_SIZE 0xUL
  #define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE (1024 * 64)
+#define VIRTIO_VSOCK_MIN_PKT_BUF_SIZE  (1024 * 4)
  
  enum {

VSOCK_VQ_RX = 0, /* for host to guest data */
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index af1d2ce12f54..732398b4e28f 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -66,6 +66,31 @@ struct virtio_vsock {
u32 guest_cid;
  };
  
+static unsigned int rx_buf_size = VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE;

+
+static int param_set_rx_buf_size(const char *val, const struct kernel_param 
*kp)
+{
+   unsigned int size;
+   int ret;
+
+   ret = kstrtouint(val, 0, );
+   if (ret)
+   return ret;
+
+   if (size < VIRTIO_VSOCK_MIN_PKT_BUF_SIZE ||
+   size > VIRTIO_VSOCK_MAX_PKT_BUF_SIZE)
+   return -EINVAL;
+
+   return param_set_uint(val, kp);
+};
+
+static const struct kernel_param_ops param_ops_rx_buf_size = {
+   .set = param_set_rx_buf_size,
+   .get = param_get_uint,
+};
+
+module_param_cb(rx_buf_size, _ops_rx_buf_size, _buf_size, 0644);
+
  static struct virtio_vsock *virtio_vsock_get(void)
  {
return the_virtio_vsock;
@@ -261,7 +286,7 @@ virtio_transport_cancel_pkt(struct vsock_sock *vsk)
  
  static void virtio_vsock_rx_fill(struct virtio_vsock *vsock)

  {
-   int buf_len = VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE;
+   int buf_len = rx_buf_size;
struct virtio_vsock_pkt *pkt;
struct scatterlist hdr, buf, *sgs[2];
struct virtqueue *vq;

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization