* [RFC] VSOCK: add support for MSG_PEEK
@ 2019-09-22 17:48 Matias Ezequiel Vara Larsen
2019-09-23 7:58 ` Stefano Garzarella
0 siblings, 1 reply; 3+ messages in thread
From: Matias Ezequiel Vara Larsen @ 2019-09-22 17:48 UTC (permalink / raw)
To: stefanha
Cc: davem, kvm, virtualization, netdev, linux-kernel, matiasevara,
sgarzare
This patch adds support for MSG_PEEK. In such a case, packets are not
removed from the rx_queue and credit updates are not sent.
Signed-off-by: Matias Ezequiel Vara Larsen <matiasevara@gmail.com>
---
net/vmw_vsock/virtio_transport_common.c | 59 +++++++++++++++++++++++++++++++--
1 file changed, 56 insertions(+), 3 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 94cc0fa..830e890 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -264,6 +264,59 @@ static int virtio_transport_send_credit_update(struct vsock_sock *vsk,
}
static ssize_t
+virtio_transport_stream_do_peek(struct vsock_sock *vsk,
+ struct msghdr *msg,
+ size_t len)
+{
+ struct virtio_vsock_sock *vvs = vsk->trans;
+ struct virtio_vsock_pkt *pkt;
+ size_t bytes, off = 0, total = 0;
+ int err = -EFAULT;
+
+ spin_lock_bh(&vvs->rx_lock);
+
+ if (list_empty(&vvs->rx_queue)) {
+ spin_unlock_bh(&vvs->rx_lock);
+ return 0;
+ }
+
+ pkt = list_first_entry(&vvs->rx_queue,
+ struct virtio_vsock_pkt, list);
+ do {
+ bytes = len - total;
+ if (bytes > pkt->len - off)
+ bytes = pkt->len - off;
+
+ /* sk_lock is held by caller so no one else can dequeue.
+ * Unlock rx_lock since memcpy_to_msg() may sleep.
+ */
+ spin_unlock_bh(&vvs->rx_lock);
+
+ err = memcpy_to_msg(msg, pkt->buf + off, bytes);
+ if (err)
+ goto out;
+
+ spin_lock_bh(&vvs->rx_lock);
+
+ total += bytes;
+ off += bytes;
+ if (off == pkt->len) {
+ pkt = list_next_entry(pkt, list);
+ off = 0;
+ }
+ } while ((total < len) && !list_is_first(&pkt->list, &vvs->rx_queue));
+
+ spin_unlock_bh(&vvs->rx_lock);
+
+ return total;
+
+out:
+ if (total)
+ err = total;
+ return err;
+}
+
+static ssize_t
virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
struct msghdr *msg,
size_t len)
@@ -330,9 +383,9 @@ virtio_transport_stream_dequeue(struct vsock_sock *vsk,
size_t len, int flags)
{
if (flags & MSG_PEEK)
- return -EOPNOTSUPP;
-
- return virtio_transport_stream_do_dequeue(vsk, msg, len);
+ return virtio_transport_stream_do_peek(vsk, msg, len);
+ else
+ return virtio_transport_stream_do_dequeue(vsk, msg, len);
}
EXPORT_SYMBOL_GPL(virtio_transport_stream_dequeue);
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [RFC] VSOCK: add support for MSG_PEEK
2019-09-22 17:48 [RFC] VSOCK: add support for MSG_PEEK Matias Ezequiel Vara Larsen
@ 2019-09-23 7:58 ` Stefano Garzarella
2019-09-23 19:16 ` Matias Ezequiel Vara Larsen
0 siblings, 1 reply; 3+ messages in thread
From: Stefano Garzarella @ 2019-09-23 7:58 UTC (permalink / raw)
To: Matias Ezequiel Vara Larsen
Cc: stefanha, davem, kvm, virtualization, netdev, linux-kernel
Hi Matias,
thanks for this patch!
Since this patch only concerns virtio_transport,
I'd use the 'vsock/virtio' prefix in the commit title:
"vsock/virtio: add support for MSG_PEEK"
Some comments below:
On Sun, Sep 22, 2019 at 05:48:27PM +0000, Matias Ezequiel Vara Larsen wrote:
> This patch adds support for MSG_PEEK. In such a case, packets are not
> removed from the rx_queue and credit updates are not sent.
>
> Signed-off-by: Matias Ezequiel Vara Larsen <matiasevara@gmail.com>
> ---
> net/vmw_vsock/virtio_transport_common.c | 59 +++++++++++++++++++++++++++++++--
> 1 file changed, 56 insertions(+), 3 deletions(-)
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 94cc0fa..830e890 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -264,6 +264,59 @@ static int virtio_transport_send_credit_update(struct vsock_sock *vsk,
> }
>
> static ssize_t
> +virtio_transport_stream_do_peek(struct vsock_sock *vsk,
> + struct msghdr *msg,
> + size_t len)
> +{
> + struct virtio_vsock_sock *vvs = vsk->trans;
> + struct virtio_vsock_pkt *pkt;
> + size_t bytes, off = 0, total = 0;
> + int err = -EFAULT;
> +
> + spin_lock_bh(&vvs->rx_lock);
> +
What about using list_for_each_entry() to cycle through the queued packets?
> + if (list_empty(&vvs->rx_queue)) {
> + spin_unlock_bh(&vvs->rx_lock);
> + return 0;
> + }
> +
> + pkt = list_first_entry(&vvs->rx_queue,
> + struct virtio_vsock_pkt, list);
> + do {
pkt->off contains the offset inside the packet where the unread data starts.
So here we should initialize 'off':
off = pkt->off;
Or just use pkt->off later (without increasing it as in the dequeue).
> + bytes = len - total;
> + if (bytes > pkt->len - off)
> + bytes = pkt->len - off;
> +
> + /* sk_lock is held by caller so no one else can dequeue.
> + * Unlock rx_lock since memcpy_to_msg() may sleep.
> + */
> + spin_unlock_bh(&vvs->rx_lock);
> +
> + err = memcpy_to_msg(msg, pkt->buf + off, bytes);
> + if (err)
> + goto out;
> +
> + spin_lock_bh(&vvs->rx_lock);
> +
> + total += bytes;
Using list_for_each_entry(), here we can just do:
(or better, at the beginning of the cycle)
if (total == len)
break;
removing the next part...
> + off += bytes;
> + if (off == pkt->len) {
> + pkt = list_next_entry(pkt, list);
> + off = 0;
> + }
> + } while ((total < len) && !list_is_first(&pkt->list, &vvs->rx_queue));
...until here.
> +
> + spin_unlock_bh(&vvs->rx_lock);
> +
> + return total;
> +
> +out:
> + if (total)
> + err = total;
> + return err;
> +}
> +
> +static ssize_t
> virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
> struct msghdr *msg,
> size_t len)
> @@ -330,9 +383,9 @@ virtio_transport_stream_dequeue(struct vsock_sock *vsk,
> size_t len, int flags)
> {
> if (flags & MSG_PEEK)
> - return -EOPNOTSUPP;
> -
> - return virtio_transport_stream_do_dequeue(vsk, msg, len);
> + return virtio_transport_stream_do_peek(vsk, msg, len);
> + else
> + return virtio_transport_stream_do_dequeue(vsk, msg, len);
> }
> EXPORT_SYMBOL_GPL(virtio_transport_stream_dequeue);
>
The rest looks good to me!
Thanks,
Stefano
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [RFC] VSOCK: add support for MSG_PEEK
2019-09-23 7:58 ` Stefano Garzarella
@ 2019-09-23 19:16 ` Matias Ezequiel Vara Larsen
0 siblings, 0 replies; 3+ messages in thread
From: Matias Ezequiel Vara Larsen @ 2019-09-23 19:16 UTC (permalink / raw)
To: Stefano Garzarella
Cc: stefanha, davem, kvm, virtualization, netdev, linux-kernel
On Mon, Sep 23, 2019 at 09:58:30AM +0200, Stefano Garzarella wrote:
> Hi Matias,
> thanks for this patch!
>
> Since this patch only concerns virtio_transport,
> I'd use the 'vsock/virtio' prefix in the commit title:
> "vsock/virtio: add support for MSG_PEEK"
>
> Some comments below:
>
> On Sun, Sep 22, 2019 at 05:48:27PM +0000, Matias Ezequiel Vara Larsen wrote:
> > This patch adds support for MSG_PEEK. In such a case, packets are not
> > removed from the rx_queue and credit updates are not sent.
> >
> > Signed-off-by: Matias Ezequiel Vara Larsen <matiasevara@gmail.com>
> > ---
> > net/vmw_vsock/virtio_transport_common.c | 59 +++++++++++++++++++++++++++++++--
> > 1 file changed, 56 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > index 94cc0fa..830e890 100644
> > --- a/net/vmw_vsock/virtio_transport_common.c
> > +++ b/net/vmw_vsock/virtio_transport_common.c
> > @@ -264,6 +264,59 @@ static int virtio_transport_send_credit_update(struct vsock_sock *vsk,
> > }
> >
> > static ssize_t
> > +virtio_transport_stream_do_peek(struct vsock_sock *vsk,
> > + struct msghdr *msg,
> > + size_t len)
> > +{
> > + struct virtio_vsock_sock *vvs = vsk->trans;
> > + struct virtio_vsock_pkt *pkt;
> > + size_t bytes, off = 0, total = 0;
> > + int err = -EFAULT;
> > +
> > + spin_lock_bh(&vvs->rx_lock);
> > +
>
> What about using list_for_each_entry() to cycle through the queued packets?
>
> > + if (list_empty(&vvs->rx_queue)) {
> > + spin_unlock_bh(&vvs->rx_lock);
> > + return 0;
> > + }
> > +
> > + pkt = list_first_entry(&vvs->rx_queue,
> > + struct virtio_vsock_pkt, list);
> > + do {
>
> pkt->off contains the offset inside the packet where the unread data starts.
> So here we should initialize 'off':
>
> off = pkt->off;
>
> Or just use pkt->off later (without increasing it as in the dequeue).
>
> > + bytes = len - total;
> > + if (bytes > pkt->len - off)
> > + bytes = pkt->len - off;
> > +
> > + /* sk_lock is held by caller so no one else can dequeue.
> > + * Unlock rx_lock since memcpy_to_msg() may sleep.
> > + */
> > + spin_unlock_bh(&vvs->rx_lock);
> > +
> > + err = memcpy_to_msg(msg, pkt->buf + off, bytes);
> > + if (err)
> > + goto out;
> > +
> > + spin_lock_bh(&vvs->rx_lock);
> > +
> > + total += bytes;
>
> Using list_for_each_entry(), here we can just do:
> (or better, at the beginning of the cycle)
>
> if (total == len)
> break;
>
> removing the next part...
>
> > + off += bytes;
> > + if (off == pkt->len) {
> > + pkt = list_next_entry(pkt, list);
> > + off = 0;
> > + }
> > + } while ((total < len) && !list_is_first(&pkt->list, &vvs->rx_queue));
>
> ...until here.
>
> > +
> > + spin_unlock_bh(&vvs->rx_lock);
> > +
> > + return total;
> > +
> > +out:
> > + if (total)
> > + err = total;
> > + return err;
> > +}
> > +
> > +static ssize_t
> > virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
> > struct msghdr *msg,
> > size_t len)
> > @@ -330,9 +383,9 @@ virtio_transport_stream_dequeue(struct vsock_sock *vsk,
> > size_t len, int flags)
> > {
> > if (flags & MSG_PEEK)
> > - return -EOPNOTSUPP;
> > -
> > - return virtio_transport_stream_do_dequeue(vsk, msg, len);
> > + return virtio_transport_stream_do_peek(vsk, msg, len);
> > + else
> > + return virtio_transport_stream_do_dequeue(vsk, msg, len);
> > }
> > EXPORT_SYMBOL_GPL(virtio_transport_stream_dequeue);
> >
>
> The rest looks good to me!
>
> Thanks,
> Stefano
Thanks Stefano. Based on your comments, I will modify the patch and
resubmit it.
Matias
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-09-23 19:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-22 17:48 [RFC] VSOCK: add support for MSG_PEEK Matias Ezequiel Vara Larsen
2019-09-23 7:58 ` Stefano Garzarella
2019-09-23 19:16 ` Matias Ezequiel Vara Larsen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox