2021-08-10 16:27:16

by Arseny Krasnov

[permalink] [raw]
Subject: [RFC PATCH v2 0/5] virtio/vsock: introduce MSG_EOR flag for SEQPACKET

This patchset implements support of MSG_EOR bit for SEQPACKET
AF_VSOCK sockets over virtio transport.
First we need to define 'messages' and 'records' like this:
Message is result of sending calls: 'write()', 'send()', 'sendmsg()'
etc. It has fixed maximum length, and it bounds are visible using
return from receive calls: 'read()', 'recv()', 'recvmsg()' etc.
Current implementation based on message definition above.
Record has unlimited length, it consists of multiple messages,
and bounds of record are visible via MSG_EOR flag returned from
'recvmsg()' call. Sender passes MSG_EOR to sending system call and
receiver will see MSG_EOR when corresponding message will be processed.
Idea of patchset comes from POSIX: it says that SEQPACKET
supports record boundaries which are visible for receiver using
MSG_EOR bit. So, it looks like MSG_EOR is enough thing for SEQPACKET
and we don't need to maintain boundaries of corresponding send -
receive system calls. But, for 'sendXXX()' and 'recXXX()' POSIX says,
that all these calls operates with messages, e.g. 'sendXXX()' sends
message, while 'recXXX()' reads messages and for SEQPACKET, 'recXXX()'
must read one entire message from socket, dropping all out of size
bytes. Thus, both message boundaries and MSG_EOR bit must be supported
to follow POSIX rules.
To support MSG_EOR new bit was added along with existing
'VIRTIO_VSOCK_SEQ_EOR': 'VIRTIO_VSOCK_SEQ_EOM'(end-of-message) - now it
works in the same way as 'VIRTIO_VSOCK_SEQ_EOR'. But 'VIRTIO_VSOCK_SEQ_EOR'
is used to mark 'MSG_EOR' bit passed from userspace.
This patchset includes simple test for MSG_EOR.

Arseny Krasnov(5):
virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOM' bit
vhost/vsock: support MSG_EOR bit processing
virito/vsock: support MSG_EOR bit processing
af_vsock: rename variables in receive loop
vsock_test: update message bounds test for MSG_EOR

drivers/vhost/vsock.c | 22 +++++++++++++---------
include/uapi/linux/virtio_vsock.h | 3 ++-
net/vmw_vsock/af_vsock.c | 10 +++++-----
net/vmw_vsock/virtio_transport_common.c | 23 +++++++++++++++--------
tools/testing/vsock/vsock_test.c | 8 +++++++-
5 files changed, 42 insertions(+), 24 deletions(-)

v1 -> v2:
- 'VIRTIO_VSOCK_SEQ_EOR' is renamed to 'VIRTIO_VSOCK_SEQ_EOM', to
support backward compatibility.
- use bitmask of flags to restore in vhost.c, instead of separated
bool variable for each flag.
- test for EAGAIN removed, as logically it is not part of this
patchset(will be sent separately).
- cover letter updated(added part with POSIX description).

Signed-off-by: Arseny Krasnov <[email protected]>

--
2.25.1


2021-08-10 16:27:31

by Arseny Krasnov

[permalink] [raw]
Subject: [RFC PATCH v2 3/5] virito/vsock: support MSG_EOR bit processing

If packet has 'EOR' bit - set MSG_EOR in 'recvmsg()' flags.

Signed-off-by: Arseny Krasnov <[email protected]>
---
net/vmw_vsock/virtio_transport_common.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 4d5a93beceb0..59ee1be5a6dd 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -76,8 +76,12 @@ virtio_transport_alloc_pkt(struct virtio_vsock_pkt_info *info,
goto out;

if (msg_data_left(info->msg) == 0 &&
- info->type == VIRTIO_VSOCK_TYPE_SEQPACKET)
+ info->type == VIRTIO_VSOCK_TYPE_SEQPACKET) {
pkt->hdr.flags |= cpu_to_le32(VIRTIO_VSOCK_SEQ_EOM);
+
+ if (info->msg->msg_flags & MSG_EOR)
+ pkt->hdr.flags |= cpu_to_le32(VIRTIO_VSOCK_SEQ_EOR);
+ }
}

trace_virtio_transport_alloc_pkt(src_cid, src_port,
@@ -460,6 +464,9 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,
if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOM) {
msg_ready = true;
vvs->msg_count--;
+
+ if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOR)
+ msg->msg_flags |= MSG_EOR;
}

virtio_transport_dec_rx_pkt(vvs, pkt);
--
2.25.1

2021-08-10 16:27:39

by Arseny Krasnov

[permalink] [raw]
Subject: [RFC PATCH v2 4/5] af_vsock: rename variables in receive loop

Record is supported via MSG_EOR flag, while current logic operates
with message, so rename variables from 'record' to 'message'.

Signed-off-by: Arseny Krasnov <[email protected]>
---
net/vmw_vsock/af_vsock.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 3e02cc3b24f8..e2c0cfb334d2 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -2014,7 +2014,7 @@ static int __vsock_seqpacket_recvmsg(struct sock *sk, struct msghdr *msg,
{
const struct vsock_transport *transport;
struct vsock_sock *vsk;
- ssize_t record_len;
+ ssize_t msg_len;
long timeout;
int err = 0;
DEFINE_WAIT(wait);
@@ -2028,9 +2028,9 @@ static int __vsock_seqpacket_recvmsg(struct sock *sk, struct msghdr *msg,
if (err <= 0)
goto out;

- record_len = transport->seqpacket_dequeue(vsk, msg, flags);
+ msg_len = transport->seqpacket_dequeue(vsk, msg, flags);

- if (record_len < 0) {
+ if (msg_len < 0) {
err = -ENOMEM;
goto out;
}
@@ -2044,14 +2044,14 @@ static int __vsock_seqpacket_recvmsg(struct sock *sk, struct msghdr *msg,
* packet.
*/
if (flags & MSG_TRUNC)
- err = record_len;
+ err = msg_len;
else
err = len - msg_data_left(msg);

/* Always set MSG_TRUNC if real length of packet is
* bigger than user's buffer.
*/
- if (record_len > len)
+ if (msg_len > len)
msg->msg_flags |= MSG_TRUNC;
}

--
2.25.1

2021-08-11 09:10:31

by Stefano Garzarella

[permalink] [raw]
Subject: Re: [RFC PATCH v2 3/5] virito/vsock: support MSG_EOR bit processing

On Tue, Aug 10, 2021 at 02:40:32PM +0300, Arseny Krasnov wrote:
>If packet has 'EOR' bit - set MSG_EOR in 'recvmsg()' flags.
>
>Signed-off-by: Arseny Krasnov <[email protected]>
>---
> net/vmw_vsock/virtio_transport_common.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 4d5a93beceb0..59ee1be5a6dd 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -76,8 +76,12 @@ virtio_transport_alloc_pkt(struct virtio_vsock_pkt_info *info,
> goto out;
>
> if (msg_data_left(info->msg) == 0 &&
>- info->type == VIRTIO_VSOCK_TYPE_SEQPACKET)
>+ info->type == VIRTIO_VSOCK_TYPE_SEQPACKET) {
> pkt->hdr.flags |= cpu_to_le32(VIRTIO_VSOCK_SEQ_EOM);
>+
>+ if (info->msg->msg_flags & MSG_EOR)
>+ pkt->hdr.flags |= cpu_to_le32(VIRTIO_VSOCK_SEQ_EOR);
>+ }
> }
>
> trace_virtio_transport_alloc_pkt(src_cid, src_port,
>@@ -460,6 +464,9 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,
> if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOM) {
> msg_ready = true;
> vvs->msg_count--;
>+
>+ if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOR)
>+ msg->msg_flags |= MSG_EOR;
> }
>
> virtio_transport_dec_rx_pkt(vvs, pkt);
>--
>2.25.1
>

Reviewed-by: Stefano Garzarella <[email protected]>

2021-08-11 09:13:22

by Stefano Garzarella

[permalink] [raw]
Subject: Re: [RFC PATCH v2 4/5] af_vsock: rename variables in receive loop

On Tue, Aug 10, 2021 at 02:41:00PM +0300, Arseny Krasnov wrote:
>Record is supported via MSG_EOR flag, while current logic operates
>with message, so rename variables from 'record' to 'message'.
>
>Signed-off-by: Arseny Krasnov <[email protected]>
>---
> net/vmw_vsock/af_vsock.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 3e02cc3b24f8..e2c0cfb334d2 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -2014,7 +2014,7 @@ static int __vsock_seqpacket_recvmsg(struct sock *sk, struct msghdr *msg,
> {
> const struct vsock_transport *transport;
> struct vsock_sock *vsk;
>- ssize_t record_len;
>+ ssize_t msg_len;
> long timeout;
> int err = 0;
> DEFINE_WAIT(wait);
>@@ -2028,9 +2028,9 @@ static int __vsock_seqpacket_recvmsg(struct sock *sk, struct msghdr *msg,
> if (err <= 0)
> goto out;
>
>- record_len = transport->seqpacket_dequeue(vsk, msg, flags);
>+ msg_len = transport->seqpacket_dequeue(vsk, msg, flags);
>
>- if (record_len < 0) {
>+ if (msg_len < 0) {
> err = -ENOMEM;
> goto out;
> }
>@@ -2044,14 +2044,14 @@ static int __vsock_seqpacket_recvmsg(struct sock *sk, struct msghdr *msg,
> * packet.
> */
> if (flags & MSG_TRUNC)
>- err = record_len;
>+ err = msg_len;
> else
> err = len - msg_data_left(msg);
>
> /* Always set MSG_TRUNC if real length of packet is
> * bigger than user's buffer.
> */
>- if (record_len > len)
>+ if (msg_len > len)
> msg->msg_flags |= MSG_TRUNC;
> }
>
>--
>2.25.1
>

Reviewed-by: Stefano Garzarella <[email protected]>