Re: [RFC PATCH v1 2/4] virtio/vsock: support MSG_PEEK for SOCK_SEQPACKET

[Stefano Garzarella]

On Sun, Jun 18, 2023 at 09:24:49AM +0300, Arseniy Krasnov wrote:
> This adds support of MSG_PEEK flag for SOCK_SEQPACKET type of socket.
> Difference with SOCK_STREAM is that this callback returns either length
> of the message or error.
>
> Signed-off-by: Arseniy Krasnov <AVKrasnov@xxxxxxxxxxxxxx>
> ---
> net/vmw_vsock/virtio_transport_common.c | 63 +++++++++++++++++++++++--
> 1 file changed, 60 insertions(+), 3 deletions(-)
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 2ee40574c339..352d042b130b 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -460,6 +460,63 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
> 	return err;
> }
>
> +static ssize_t
> +virtio_transport_seqpacket_do_peek(struct vsock_sock *vsk,
> +				   struct msghdr *msg)
> +{
> +	struct virtio_vsock_sock *vvs = vsk->trans;
> +	struct sk_buff *skb;
> +	size_t total, len;
> +
> +	spin_lock_bh(&vvs->rx_lock);
> +
> +	if (!vvs->msg_count) {
> +		spin_unlock_bh(&vvs->rx_lock);
> +		return 0;
> +	}
> +
> +	total = 0;
> +	len = msg_data_left(msg);
> +
> +	skb_queue_walk(&vvs->rx_queue, skb) {
> +		struct virtio_vsock_hdr *hdr;
> +
> +		if (total < len) {
> +			size_t bytes;
> +			int err;
> +
> +			bytes = len - total;
> +			if (bytes > skb->len)
> +				bytes = skb->len;
> +
> +			spin_unlock_bh(&vvs->rx_lock);
> +
> +			/* sk_lock is held by caller so no one else can dequeue.
> +			 * Unlock rx_lock since memcpy_to_msg() may sleep.
> +			 */
> +			err = memcpy_to_msg(msg, skb->data, bytes);
> +			if (err)
> +				return err;
> +
> +			spin_lock_bh(&vvs->rx_lock);
> +		}
> +
> +		total += skb->len;
> +		hdr = virtio_vsock_hdr(skb);
> +
> +		if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM) {
> +			if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOR)
> +				msg->msg_flags |= MSG_EOR;
> +
> +			break;
> +		}
> +	}
> +
> +	spin_unlock_bh(&vvs->rx_lock);
> +
> +	return total;

Should we return the minimum between total and len?

Thanks,
Stefano

> +}
> +
> static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,
> 						 struct msghdr *msg,
> 						 int flags)
> @@ -554,9 +611,9 @@ virtio_transport_seqpacket_dequeue(struct vsock_sock *vsk,
> 				   int flags)
> {
> 	if (flags & MSG_PEEK)
> -		return -EOPNOTSUPP;
> -
> -	return virtio_transport_seqpacket_do_dequeue(vsk, msg, flags);
> +		return virtio_transport_seqpacket_do_peek(vsk, msg);
> +	else
> +		return virtio_transport_seqpacket_do_dequeue(vsk, msg, flags);
> }
> EXPORT_SYMBOL_GPL(virtio_transport_seqpacket_dequeue);
>
> --
> 2.25.1