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

[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 <stefanha@xxxxxxxxxx>
> > Signed-off-by: Stefano Garzarella <sgarzare@xxxxxxxxxx>
>
>
> 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ÂÂÂÂÂÂÂ 0xFFFFFFFFUL
> > Â #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, &size);
> > +ÂÂÂ 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, &param_ops_rx_buf_size, &rx_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;