Re: [PATCH v2 8/8] vsock/virtio: make the RX buffer size tunable
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
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
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
[PATCH v2 8/8] vsock/virtio: make the RX buffer size tunable
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 --- 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; -- 2.20.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization