lkml.org 
[lkml]   [2019]   [Oct]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH 07/13] vsock: handle buffer_size sockopts in the core
On Fri, Sep 27, 2019 at 01:26:57PM +0200, Stefano Garzarella wrote:
> @@ -140,18 +145,11 @@ struct vsock_transport {
> struct vsock_transport_send_notify_data *);
> int (*notify_send_post_enqueue)(struct vsock_sock *, ssize_t,
> struct vsock_transport_send_notify_data *);
> + int (*notify_buffer_size)(struct vsock_sock *, u64 *);

Is ->notify_buffer_size() called under lock_sock(sk)? If yes, please
document it.

> +static void vsock_update_buffer_size(struct vsock_sock *vsk,
> + const struct vsock_transport *transport,
> + u64 val)
> +{
> + if (val > vsk->buffer_max_size)
> + val = vsk->buffer_max_size;
> +
> + if (val < vsk->buffer_min_size)
> + val = vsk->buffer_min_size;
> +
> + if (val != vsk->buffer_size &&
> + transport && transport->notify_buffer_size)
> + transport->notify_buffer_size(vsk, &val);

Why does this function return an int if we don't check the return value?

> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index fc046c071178..bac9e7430a2e 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -403,17 +403,13 @@ int virtio_transport_do_socket_init(struct vsock_sock *vsk,
> if (psk) {
> struct virtio_vsock_sock *ptrans = psk->trans;
>
> - vvs->buf_size = ptrans->buf_size;
> - vvs->buf_size_min = ptrans->buf_size_min;
> - vvs->buf_size_max = ptrans->buf_size_max;
> vvs->peer_buf_alloc = ptrans->peer_buf_alloc;
> - } else {
> - vvs->buf_size = VIRTIO_VSOCK_DEFAULT_BUF_SIZE;
> - vvs->buf_size_min = VIRTIO_VSOCK_DEFAULT_MIN_BUF_SIZE;
> - vvs->buf_size_max = VIRTIO_VSOCK_DEFAULT_MAX_BUF_SIZE;
> }
>
> - vvs->buf_alloc = vvs->buf_size;
> + if (vsk->buffer_size > VIRTIO_VSOCK_MAX_BUF_SIZE)
> + vsk->buffer_size = VIRTIO_VSOCK_MAX_BUF_SIZE;

Hmm...this could be outside the [min, max] range. I'm not sure how much
it matters.

Another issue is that this patch drops the VIRTIO_VSOCK_MAX_BUF_SIZE
limit that used to be enforced by virtio_transport_set_buffer_size().
Now the limit is only applied at socket init time. If the buffer size
is changed later then VIRTIO_VSOCK_MAX_BUF_SIZE can be exceeded. If
that doesn't matter, why even bother with VIRTIO_VSOCK_MAX_BUF_SIZE
here?

[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2019-10-09 14:31    [W:0.323 / U:0.304 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site