* [PATCH] vsock/virtio: add support for MSG_PEEK
@ 2019-09-26 18:23 Matias Ezequiel Vara Larsen
2019-09-26 19:33 ` Eric Dumazet
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Matias Ezequiel Vara Larsen @ 2019-09-26 18:23 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 | 50 +++++++++++++++++++++++++++++++--
1 file changed, 47 insertions(+), 3 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 94cc0fa..938f2ed 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -264,6 +264,50 @@ 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, total = 0;
+ int err = -EFAULT;
+
+ spin_lock_bh(&vvs->rx_lock);
+
+ list_for_each_entry(pkt, &vvs->rx_queue, list) {
+ if (total == len)
+ break;
+
+ bytes = len - total;
+ if (bytes > pkt->len - pkt->off)
+ bytes = pkt->len - pkt->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 + pkt->off, bytes);
+ if (err)
+ goto out;
+
+ spin_lock_bh(&vvs->rx_lock);
+
+ total += bytes;
+ }
+
+ 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 +374,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] 15+ messages in thread
* Re: [PATCH] vsock/virtio: add support for MSG_PEEK
2019-09-26 18:23 [PATCH] vsock/virtio: add support for MSG_PEEK Matias Ezequiel Vara Larsen
2019-09-26 19:33 ` Eric Dumazet
@ 2019-09-26 19:33 ` Eric Dumazet
2019-09-27 8:55 ` Stefano Garzarella
2019-09-27 8:55 ` Stefano Garzarella
2019-09-27 16:44 ` [PATCH v2] " Matias Ezequiel Vara Larsen
2019-09-27 16:44 ` Matias Ezequiel Vara Larsen
3 siblings, 2 replies; 15+ messages in thread
From: Eric Dumazet @ 2019-09-26 19:33 UTC (permalink / raw)
To: Matias Ezequiel Vara Larsen, stefanha
Cc: davem, kvm, virtualization, netdev, linux-kernel, sgarzare
On 9/26/19 11:23 AM, 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 | 50 +++++++++++++++++++++++++++++++--
> 1 file changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 94cc0fa..938f2ed 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -264,6 +264,50 @@ 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, total = 0;
> + int err = -EFAULT;
> +
> + spin_lock_bh(&vvs->rx_lock);
> +
> + list_for_each_entry(pkt, &vvs->rx_queue, list) {
> + if (total == len)
> + break;
> +
> + bytes = len - total;
> + if (bytes > pkt->len - pkt->off)
> + bytes = pkt->len - pkt->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 + pkt->off, bytes);
> + if (err)
> + goto out;
> +
> + spin_lock_bh(&vvs->rx_lock);
> +
> + total += bytes;
> + }
> +
> + spin_unlock_bh(&vvs->rx_lock);
> +
> + return total;
> +
> +out:
> + if (total)
> + err = total;
> + return err;
> +}
>
This seems buggy to me.
virtio_transport_recv_enqueue() seems to be able to add payload to the last packet in the queue.
The loop you wrote here would miss newly added chunks while the vvs->rx_lock spinlock has been released.
virtio_transport_stream_do_dequeue() is ok, because it makes sure pkt->off == pkt->len
before cleaning the packet from the queue.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] vsock/virtio: add support for MSG_PEEK
2019-09-26 18:23 [PATCH] vsock/virtio: add support for MSG_PEEK Matias Ezequiel Vara Larsen
@ 2019-09-26 19:33 ` Eric Dumazet
2019-09-26 19:33 ` Eric Dumazet
` (2 subsequent siblings)
3 siblings, 0 replies; 15+ messages in thread
From: Eric Dumazet @ 2019-09-26 19:33 UTC (permalink / raw)
To: Matias Ezequiel Vara Larsen, stefanha
Cc: kvm, netdev, linux-kernel, virtualization, davem, sgarzare
On 9/26/19 11:23 AM, 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 | 50 +++++++++++++++++++++++++++++++--
> 1 file changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 94cc0fa..938f2ed 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -264,6 +264,50 @@ 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, total = 0;
> + int err = -EFAULT;
> +
> + spin_lock_bh(&vvs->rx_lock);
> +
> + list_for_each_entry(pkt, &vvs->rx_queue, list) {
> + if (total == len)
> + break;
> +
> + bytes = len - total;
> + if (bytes > pkt->len - pkt->off)
> + bytes = pkt->len - pkt->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 + pkt->off, bytes);
> + if (err)
> + goto out;
> +
> + spin_lock_bh(&vvs->rx_lock);
> +
> + total += bytes;
> + }
> +
> + spin_unlock_bh(&vvs->rx_lock);
> +
> + return total;
> +
> +out:
> + if (total)
> + err = total;
> + return err;
> +}
>
This seems buggy to me.
virtio_transport_recv_enqueue() seems to be able to add payload to the last packet in the queue.
The loop you wrote here would miss newly added chunks while the vvs->rx_lock spinlock has been released.
virtio_transport_stream_do_dequeue() is ok, because it makes sure pkt->off == pkt->len
before cleaning the packet from the queue.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] vsock/virtio: add support for MSG_PEEK
2019-09-26 19:33 ` Eric Dumazet
@ 2019-09-27 8:55 ` Stefano Garzarella
2019-09-27 13:37 ` Eric Dumazet
2019-09-27 13:37 ` Eric Dumazet
2019-09-27 8:55 ` Stefano Garzarella
1 sibling, 2 replies; 15+ messages in thread
From: Stefano Garzarella @ 2019-09-27 8:55 UTC (permalink / raw)
To: Eric Dumazet, Matias Ezequiel Vara Larsen
Cc: stefanha, davem, kvm, virtualization, netdev, linux-kernel
On Thu, Sep 26, 2019 at 12:33:36PM -0700, Eric Dumazet wrote:
>
>
> On 9/26/19 11:23 AM, 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 | 50 +++++++++++++++++++++++++++++++--
> > 1 file changed, 47 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > index 94cc0fa..938f2ed 100644
> > --- a/net/vmw_vsock/virtio_transport_common.c
> > +++ b/net/vmw_vsock/virtio_transport_common.c
> > @@ -264,6 +264,50 @@ 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, total = 0;
> > + int err = -EFAULT;
> > +
> > + spin_lock_bh(&vvs->rx_lock);
> > +
> > + list_for_each_entry(pkt, &vvs->rx_queue, list) {
> > + if (total == len)
> > + break;
> > +
> > + bytes = len - total;
> > + if (bytes > pkt->len - pkt->off)
> > + bytes = pkt->len - pkt->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 + pkt->off, bytes);
> > + if (err)
> > + goto out;
> > +
> > + spin_lock_bh(&vvs->rx_lock);
> > +
> > + total += bytes;
> > + }
> > +
> > + spin_unlock_bh(&vvs->rx_lock);
> > +
> > + return total;
> > +
> > +out:
> > + if (total)
> > + err = total;
> > + return err;
> > +}
> >
>
> This seems buggy to me.
>
> virtio_transport_recv_enqueue() seems to be able to add payload to the last packet in the queue.
>
> The loop you wrote here would miss newly added chunks while the vvs->rx_lock spinlock has been released.
>
> virtio_transport_stream_do_dequeue() is ok, because it makes sure pkt->off == pkt->len
> before cleaning the packet from the queue.
Good catch!
Maybe we can solve in this way:
list_for_each_entry(pkt, &vvs->rx_queue, list) {
size_t off = pkt->off;
if (total == len)
break;
while (total < len && off < pkt->len) {
/* using 'off' instead of 'pkt->off' */
...
total += bytes;
off += bytes;
}
}
What do you think?
Cheers,
Stefano
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] vsock/virtio: add support for MSG_PEEK
2019-09-26 19:33 ` Eric Dumazet
2019-09-27 8:55 ` Stefano Garzarella
@ 2019-09-27 8:55 ` Stefano Garzarella
1 sibling, 0 replies; 15+ messages in thread
From: Stefano Garzarella @ 2019-09-27 8:55 UTC (permalink / raw)
To: Eric Dumazet, Matias Ezequiel Vara Larsen
Cc: kvm, netdev, linux-kernel, virtualization, stefanha, davem
On Thu, Sep 26, 2019 at 12:33:36PM -0700, Eric Dumazet wrote:
>
>
> On 9/26/19 11:23 AM, 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 | 50 +++++++++++++++++++++++++++++++--
> > 1 file changed, 47 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > index 94cc0fa..938f2ed 100644
> > --- a/net/vmw_vsock/virtio_transport_common.c
> > +++ b/net/vmw_vsock/virtio_transport_common.c
> > @@ -264,6 +264,50 @@ 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, total = 0;
> > + int err = -EFAULT;
> > +
> > + spin_lock_bh(&vvs->rx_lock);
> > +
> > + list_for_each_entry(pkt, &vvs->rx_queue, list) {
> > + if (total == len)
> > + break;
> > +
> > + bytes = len - total;
> > + if (bytes > pkt->len - pkt->off)
> > + bytes = pkt->len - pkt->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 + pkt->off, bytes);
> > + if (err)
> > + goto out;
> > +
> > + spin_lock_bh(&vvs->rx_lock);
> > +
> > + total += bytes;
> > + }
> > +
> > + spin_unlock_bh(&vvs->rx_lock);
> > +
> > + return total;
> > +
> > +out:
> > + if (total)
> > + err = total;
> > + return err;
> > +}
> >
>
> This seems buggy to me.
>
> virtio_transport_recv_enqueue() seems to be able to add payload to the last packet in the queue.
>
> The loop you wrote here would miss newly added chunks while the vvs->rx_lock spinlock has been released.
>
> virtio_transport_stream_do_dequeue() is ok, because it makes sure pkt->off == pkt->len
> before cleaning the packet from the queue.
Good catch!
Maybe we can solve in this way:
list_for_each_entry(pkt, &vvs->rx_queue, list) {
size_t off = pkt->off;
if (total == len)
break;
while (total < len && off < pkt->len) {
/* using 'off' instead of 'pkt->off' */
...
total += bytes;
off += bytes;
}
}
What do you think?
Cheers,
Stefano
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] vsock/virtio: add support for MSG_PEEK
2019-09-27 8:55 ` Stefano Garzarella
@ 2019-09-27 13:37 ` Eric Dumazet
2019-09-27 15:38 ` Matias Ezequiel Vara Larsen
2019-09-27 15:38 ` Matias Ezequiel Vara Larsen
2019-09-27 13:37 ` Eric Dumazet
1 sibling, 2 replies; 15+ messages in thread
From: Eric Dumazet @ 2019-09-27 13:37 UTC (permalink / raw)
To: Stefano Garzarella, Eric Dumazet, Matias Ezequiel Vara Larsen
Cc: stefanha, davem, kvm, virtualization, netdev, linux-kernel
On 9/27/19 1:55 AM, Stefano Garzarella wrote:
> Good catch!
>
> Maybe we can solve in this way:
>
> list_for_each_entry(pkt, &vvs->rx_queue, list) {
> size_t off = pkt->off;
>
> if (total == len)
> break;
>
> while (total < len && off < pkt->len) {
> /* using 'off' instead of 'pkt->off' */
> ...
>
> total += bytes;
> off += bytes;
> }
> }
>
> What do you think?
>
Maybe, but I need to see a complete patch, evil is in the details :)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] vsock/virtio: add support for MSG_PEEK
2019-09-27 8:55 ` Stefano Garzarella
2019-09-27 13:37 ` Eric Dumazet
@ 2019-09-27 13:37 ` Eric Dumazet
1 sibling, 0 replies; 15+ messages in thread
From: Eric Dumazet @ 2019-09-27 13:37 UTC (permalink / raw)
To: Stefano Garzarella, Eric Dumazet, Matias Ezequiel Vara Larsen
Cc: kvm, netdev, linux-kernel, virtualization, stefanha, davem
On 9/27/19 1:55 AM, Stefano Garzarella wrote:
> Good catch!
>
> Maybe we can solve in this way:
>
> list_for_each_entry(pkt, &vvs->rx_queue, list) {
> size_t off = pkt->off;
>
> if (total == len)
> break;
>
> while (total < len && off < pkt->len) {
> /* using 'off' instead of 'pkt->off' */
> ...
>
> total += bytes;
> off += bytes;
> }
> }
>
> What do you think?
>
Maybe, but I need to see a complete patch, evil is in the details :)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] vsock/virtio: add support for MSG_PEEK
2019-09-27 13:37 ` Eric Dumazet
@ 2019-09-27 15:38 ` Matias Ezequiel Vara Larsen
2019-09-27 15:38 ` Matias Ezequiel Vara Larsen
1 sibling, 0 replies; 15+ messages in thread
From: Matias Ezequiel Vara Larsen @ 2019-09-27 15:38 UTC (permalink / raw)
To: Eric Dumazet, Stefano Garzarella
Cc: stefanha, davem, kvm, virtualization, netdev, linux-kernel
On Fri, Sep 27, 2019 at 06:37:00AM -0700, Eric Dumazet wrote:
>
>
> On 9/27/19 1:55 AM, Stefano Garzarella wrote:
>
> > Good catch!
> >
> > Maybe we can solve in this way:
> >
> > list_for_each_entry(pkt, &vvs->rx_queue, list) {
> > size_t off = pkt->off;
> >
> > if (total == len)
> > break;
> >
> > while (total < len && off < pkt->len) {
> > /* using 'off' instead of 'pkt->off' */
> > ...
> >
> > total += bytes;
> > off += bytes;
> > }
> > }
> >
> > What do you think?
> >
>
> Maybe, but I need to see a complete patch, evil is in the details :)
>
Thanks both for your comments, I will take them into account and submit
a second version.
Matias
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] vsock/virtio: add support for MSG_PEEK
2019-09-27 13:37 ` Eric Dumazet
2019-09-27 15:38 ` Matias Ezequiel Vara Larsen
@ 2019-09-27 15:38 ` Matias Ezequiel Vara Larsen
1 sibling, 0 replies; 15+ messages in thread
From: Matias Ezequiel Vara Larsen @ 2019-09-27 15:38 UTC (permalink / raw)
To: Eric Dumazet, Stefano Garzarella
Cc: kvm, netdev, linux-kernel, virtualization, stefanha, davem
On Fri, Sep 27, 2019 at 06:37:00AM -0700, Eric Dumazet wrote:
>
>
> On 9/27/19 1:55 AM, Stefano Garzarella wrote:
>
> > Good catch!
> >
> > Maybe we can solve in this way:
> >
> > list_for_each_entry(pkt, &vvs->rx_queue, list) {
> > size_t off = pkt->off;
> >
> > if (total == len)
> > break;
> >
> > while (total < len && off < pkt->len) {
> > /* using 'off' instead of 'pkt->off' */
> > ...
> >
> > total += bytes;
> > off += bytes;
> > }
> > }
> >
> > What do you think?
> >
>
> Maybe, but I need to see a complete patch, evil is in the details :)
>
Thanks both for your comments, I will take them into account and submit
a second version.
Matias
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2] vsock/virtio: add support for MSG_PEEK
2019-09-26 18:23 [PATCH] vsock/virtio: add support for MSG_PEEK Matias Ezequiel Vara Larsen
` (2 preceding siblings ...)
2019-09-27 16:44 ` [PATCH v2] " Matias Ezequiel Vara Larsen
@ 2019-09-27 16:44 ` Matias Ezequiel Vara Larsen
2019-09-27 18:59 ` David Miller
` (3 more replies)
3 siblings, 4 replies; 15+ messages in thread
From: Matias Ezequiel Vara Larsen @ 2019-09-27 16:44 UTC (permalink / raw)
To: stefanha
Cc: davem, kvm, virtualization, netdev, linux-kernel, matiasevara,
sgarzare, eric.dumazet
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 | 55 +++++++++++++++++++++++++++++++--
1 file changed, 52 insertions(+), 3 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 94cc0fa..cf15751 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -264,6 +264,55 @@ 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, total = 0, off;
+ int err = -EFAULT;
+
+ spin_lock_bh(&vvs->rx_lock);
+
+ list_for_each_entry(pkt, &vvs->rx_queue, list) {
+ off = pkt->off;
+
+ if (total == len)
+ break;
+
+ while (total < len && off < pkt->len) {
+ 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;
+ }
+ }
+
+ 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 +379,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] 15+ messages in thread
* [PATCH v2] vsock/virtio: add support for MSG_PEEK
2019-09-26 18:23 [PATCH] vsock/virtio: add support for MSG_PEEK Matias Ezequiel Vara Larsen
2019-09-26 19:33 ` Eric Dumazet
2019-09-26 19:33 ` Eric Dumazet
@ 2019-09-27 16:44 ` Matias Ezequiel Vara Larsen
2019-09-27 16:44 ` Matias Ezequiel Vara Larsen
3 siblings, 0 replies; 15+ messages in thread
From: Matias Ezequiel Vara Larsen @ 2019-09-27 16:44 UTC (permalink / raw)
To: stefanha
Cc: kvm, matiasevara, netdev, linux-kernel, virtualization,
eric.dumazet, davem, 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 | 55 +++++++++++++++++++++++++++++++--
1 file changed, 52 insertions(+), 3 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 94cc0fa..cf15751 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -264,6 +264,55 @@ 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, total = 0, off;
+ int err = -EFAULT;
+
+ spin_lock_bh(&vvs->rx_lock);
+
+ list_for_each_entry(pkt, &vvs->rx_queue, list) {
+ off = pkt->off;
+
+ if (total == len)
+ break;
+
+ while (total < len && off < pkt->len) {
+ 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;
+ }
+ }
+
+ 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 +379,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] 15+ messages in thread
* Re: [PATCH v2] vsock/virtio: add support for MSG_PEEK
2019-09-27 16:44 ` Matias Ezequiel Vara Larsen
2019-09-27 18:59 ` David Miller
@ 2019-09-27 18:59 ` David Miller
2019-09-30 14:12 ` Stefano Garzarella
2019-09-30 14:12 ` Stefano Garzarella
3 siblings, 0 replies; 15+ messages in thread
From: David Miller @ 2019-09-27 18:59 UTC (permalink / raw)
To: matiasevara
Cc: stefanha, kvm, virtualization, netdev, linux-kernel, sgarzare,
eric.dumazet
This is net-next material.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] vsock/virtio: add support for MSG_PEEK
2019-09-27 16:44 ` Matias Ezequiel Vara Larsen
@ 2019-09-27 18:59 ` David Miller
2019-09-27 18:59 ` David Miller
` (2 subsequent siblings)
3 siblings, 0 replies; 15+ messages in thread
From: David Miller @ 2019-09-27 18:59 UTC (permalink / raw)
To: matiasevara
Cc: eric.dumazet, kvm, netdev, linux-kernel, virtualization,
stefanha, sgarzare
This is net-next material.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] vsock/virtio: add support for MSG_PEEK
2019-09-27 16:44 ` Matias Ezequiel Vara Larsen
` (2 preceding siblings ...)
2019-09-30 14:12 ` Stefano Garzarella
@ 2019-09-30 14:12 ` Stefano Garzarella
3 siblings, 0 replies; 15+ messages in thread
From: Stefano Garzarella @ 2019-09-30 14:12 UTC (permalink / raw)
To: Matias Ezequiel Vara Larsen
Cc: stefanha, davem, kvm, virtualization, netdev, linux-kernel, eric.dumazet
Hi Matias,
On Fri, Sep 27, 2019 at 04:44:23PM +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 | 55 +++++++++++++++++++++++++++++++--
> 1 file changed, 52 insertions(+), 3 deletions(-)
The patch LGTM. As David pointed out, this patch should go into net-next.
Since now net-next is open, you can resend with net-next tag [1] and
with:
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Tested-by: Stefano Garzarella <sgarzare@redhat.com>
Thanks,
Stefano
[1] https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 94cc0fa..cf15751 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -264,6 +264,55 @@ 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, total = 0, off;
> + int err = -EFAULT;
> +
> + spin_lock_bh(&vvs->rx_lock);
> +
> + list_for_each_entry(pkt, &vvs->rx_queue, list) {
> + off = pkt->off;
> +
> + if (total == len)
> + break;
> +
> + while (total < len && off < pkt->len) {
> + 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;
> + }
> + }
> +
> + 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 +379,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 [flat|nested] 15+ messages in thread
* Re: [PATCH v2] vsock/virtio: add support for MSG_PEEK
2019-09-27 16:44 ` Matias Ezequiel Vara Larsen
2019-09-27 18:59 ` David Miller
2019-09-27 18:59 ` David Miller
@ 2019-09-30 14:12 ` Stefano Garzarella
2019-09-30 14:12 ` Stefano Garzarella
3 siblings, 0 replies; 15+ messages in thread
From: Stefano Garzarella @ 2019-09-30 14:12 UTC (permalink / raw)
To: Matias Ezequiel Vara Larsen
Cc: eric.dumazet, kvm, netdev, linux-kernel, virtualization, stefanha, davem
Hi Matias,
On Fri, Sep 27, 2019 at 04:44:23PM +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 | 55 +++++++++++++++++++++++++++++++--
> 1 file changed, 52 insertions(+), 3 deletions(-)
The patch LGTM. As David pointed out, this patch should go into net-next.
Since now net-next is open, you can resend with net-next tag [1] and
with:
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Tested-by: Stefano Garzarella <sgarzare@redhat.com>
Thanks,
Stefano
[1] https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 94cc0fa..cf15751 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -264,6 +264,55 @@ 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, total = 0, off;
> + int err = -EFAULT;
> +
> + spin_lock_bh(&vvs->rx_lock);
> +
> + list_for_each_entry(pkt, &vvs->rx_queue, list) {
> + off = pkt->off;
> +
> + if (total == len)
> + break;
> +
> + while (total < len && off < pkt->len) {
> + 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;
> + }
> + }
> +
> + 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 +379,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 [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-09-30 14:12 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-26 18:23 [PATCH] vsock/virtio: add support for MSG_PEEK Matias Ezequiel Vara Larsen
2019-09-26 19:33 ` Eric Dumazet
2019-09-26 19:33 ` Eric Dumazet
2019-09-27 8:55 ` Stefano Garzarella
2019-09-27 13:37 ` Eric Dumazet
2019-09-27 15:38 ` Matias Ezequiel Vara Larsen
2019-09-27 15:38 ` Matias Ezequiel Vara Larsen
2019-09-27 13:37 ` Eric Dumazet
2019-09-27 8:55 ` Stefano Garzarella
2019-09-27 16:44 ` [PATCH v2] " Matias Ezequiel Vara Larsen
2019-09-27 16:44 ` Matias Ezequiel Vara Larsen
2019-09-27 18:59 ` David Miller
2019-09-27 18:59 ` David Miller
2019-09-30 14:12 ` Stefano Garzarella
2019-09-30 14:12 ` Stefano Garzarella
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.