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 message,
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(6):
virtio/vsock: rename 'EOR' to 'EOM' bit.
virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOR' bit.
vhost/vsock: support MSG_EOR bit processing
virtio/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(-)
v2 -> v3:
- 'virtio/vsock: rename 'EOR' to 'EOM' bit.' - commit message updated.
- 'VIRTIO_VSOCK_SEQ_EOR' bit add moved to separate patch.
- 'vhost/vsock: support MSG_EOR bit processing' - commit message
updated.
- 'vhost/vsock: support MSG_EOR bit processing' - removed unneeded
'le32_to_cpu()', because input argument was already in CPU
endianness.
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 <redacted>
--
2.25.1
This current implemented bit is used to mark end of messages
('EOM' - end of message), not records('EOR' - end of record).
Also rename 'record' to 'message' in implementation as it is
different things.
Signed-off-by: Arseny Krasnov <redacted>
---
drivers/vhost/vsock.c | 12 ++++++------
include/uapi/linux/virtio_vsock.h | 2 +-
net/vmw_vsock/virtio_transport_common.c | 14 +++++++-------
3 files changed, 14 insertions(+), 14 deletions(-)
@@ -225,7 +225,7 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,*/if(pkt->off<pkt->len){if(restore_flag)-pkt->hdr.flags|=cpu_to_le32(VIRTIO_VSOCK_SEQ_EOR);+pkt->hdr.flags|=cpu_to_le32(VIRTIO_VSOCK_SEQ_EOM);/* We are queueing the same virtio_vsock_pkt to handle*theremainingbytes,andwewanttodeliverit
@@ -457,7 +457,7 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,dequeued_len+=pkt_len;}-if(le32_to_cpu(pkt->hdr.flags)&VIRTIO_VSOCK_SEQ_EOR){+if(le32_to_cpu(pkt->hdr.flags)&VIRTIO_VSOCK_SEQ_EOM){msg_ready=true;vvs->msg_count--;}
@@ -1029,7 +1029,7 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,gotoout;}-if(le32_to_cpu(pkt->hdr.flags)&VIRTIO_VSOCK_SEQ_EOR)+if(le32_to_cpu(pkt->hdr.flags)&VIRTIO_VSOCK_SEQ_EOM)vvs->msg_count++;/* Try to copy small packets into the buffer of last packet queued,
@@ -1044,12 +1044,12 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,/* If there is space in the last packet queued, we copy the*newpacketinitsbuffer.Weavoidthisifthelastpacket-*queuedhasVIRTIO_VSOCK_SEQ_EORset,becausethisis-*delimiterofSEQPACKETrecord,so'pkt'isthefirstpacket-*ofanewrecord.+*queuedhasVIRTIO_VSOCK_SEQ_EOMset,becausethisis+*delimiterofSEQPACKETmessage,so'pkt'isthefirstpacket+*ofanewmessage.*/if((pkt->len<=last_pkt->buf_len-last_pkt->len)&&-!(le32_to_cpu(last_pkt->hdr.flags)&VIRTIO_VSOCK_SEQ_EOR)){+!(le32_to_cpu(last_pkt->hdr.flags)&VIRTIO_VSOCK_SEQ_EOM)){memcpy(last_pkt->buf+last_pkt->len,pkt->buf,pkt->len);last_pkt->len+=pkt->len;
This bit is used to handle POSIX MSG_EOR flag passed from
userspace in 'sendXXX()' system calls. It marks end of each
record and is visible to receiver using 'recvmsg()' system
call.
Signed-off-by: Arseny Krasnov <redacted>
---
include/uapi/linux/virtio_vsock.h | 1 +
1 file changed, 1 insertion(+)
'MSG_EOR' handling has same logic as 'MSG_EOM' - if bit present
in packet's header, reset it to 0. Then restore it back if packet
processing wasn't completed. Instead of bool variable for each
flag, bit mask variable was added: it has logical OR of 'MSG_EOR'
and 'MSG_EOM' if needed, to restore flags, this variable is ORed
with flags field of packet.
Signed-off-by: Arseny Krasnov <redacted>
---
drivers/vhost/vsock.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
@@ -224,8 +229,7 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,*tosenditwiththenextavailablebuffer.*/if(pkt->off<pkt->len){-if(restore_flag)-pkt->hdr.flags|=cpu_to_le32(VIRTIO_VSOCK_SEQ_EOM);+pkt->hdr.flags|=cpu_to_le32(flags_to_restore);/* We are queueing the same virtio_vsock_pkt to handle*theremainingbytes,andwewanttodeliverit
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 <redacted>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
---
net/vmw_vsock/af_vsock.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
@@ -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;elseerr=len-msg_data_left(msg);/* Always set MSG_TRUNC if real length of packet is*biggerthanuser'sbuffer.*/-if(record_len>len)+if(msg_len>len)msg->msg_flags|=MSG_TRUNC;}
Set 'MSG_EOR' in one of message sent, check that 'MSG_EOR'
is visible in corresponding message at receiver.
Signed-off-by: Arseny Krasnov <redacted>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
---
tools/testing/vsock/vsock_test.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
@@ -294,7 +295,7 @@ static void test_seqpacket_msg_bounds_client(const struct test_opts *opts)/* Send several messages, one with MSG_EOR flag */for(inti=0;i<MESSAGES_CNT;i++)-send_byte(fd,1,0);+send_byte(fd,1,(i==MSG_EOR_IDX)?MSG_EOR:0);control_writeln("SENDDONE");close(fd);
Hello, please ping :)
On 16.08.2021 11:50, Arseny Krasnov wrote:
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 message,
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(6):
virtio/vsock: rename 'EOR' to 'EOM' bit.
virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOR' bit.
vhost/vsock: support MSG_EOR bit processing
virtio/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(-)
v2 -> v3:
- 'virtio/vsock: rename 'EOR' to 'EOM' bit.' - commit message updated.
- 'VIRTIO_VSOCK_SEQ_EOR' bit add moved to separate patch.
- 'vhost/vsock: support MSG_EOR bit processing' - commit message
updated.
- 'vhost/vsock: support MSG_EOR bit processing' - removed unneeded
'le32_to_cpu()', because input argument was already in CPU
endianness.
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 <redacted>
On Mon, Aug 16, 2021 at 11:51:09AM +0300, Arseny Krasnov wrote:
This current implemented bit is used to mark end of messages
('EOM' - end of message), not records('EOR' - end of record).
Also rename 'record' to 'message' in implementation as it is
different things.
Signed-off-by: Arseny Krasnov <redacted>
---
drivers/vhost/vsock.c | 12 ++++++------
include/uapi/linux/virtio_vsock.h | 2 +-
net/vmw_vsock/virtio_transport_common.c | 14 +++++++-------
3 files changed, 14 insertions(+), 14 deletions(-)
On Mon, Aug 16, 2021 at 11:51:23AM +0300, Arseny Krasnov wrote:
This bit is used to handle POSIX MSG_EOR flag passed from
userspace in 'sendXXX()' system calls. It marks end of each
Maybe better 'send*()'.
quoted hunk
record and is visible to receiver using 'recvmsg()' system
call.
Signed-off-by: Arseny Krasnov <redacted>
---
include/uapi/linux/virtio_vsock.h | 1 +
1 file changed, 1 insertion(+)
On Mon, Aug 16, 2021 at 11:51:40AM +0300, Arseny Krasnov wrote:
'MSG_EOR' handling has same logic as 'MSG_EOM' - if bit present
s/same/similar
quoted hunk
in packet's header, reset it to 0. Then restore it back if packet
processing wasn't completed. Instead of bool variable for each
flag, bit mask variable was added: it has logical OR of 'MSG_EOR'
and 'MSG_EOM' if needed, to restore flags, this variable is ORed
with flags field of packet.
Signed-off-by: Arseny Krasnov <redacted>
---
drivers/vhost/vsock.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
* to send it with the next available buffer.
*/
if (pkt->off < pkt->len) {
- if (restore_flag)
- pkt->hdr.flags |= cpu_to_le32(VIRTIO_VSOCK_SEQ_EOM);
+ pkt->hdr.flags |= cpu_to_le32(flags_to_restore);
/* We are queueing the same virtio_vsock_pkt to handle
* the remaining bytes, and we want to deliver it
--
2.25.1
Hi Arseny,
On Mon, Aug 23, 2021 at 09:41:16PM +0300, Arseny Krasnov wrote:
Hello, please ping :)
Sorry, I was off last week.
I left some minor comments in the patches.
Let's wait a bit for other comments before next version, also on the
spec, then I think you can send the next version without RFC tag.
The target should be the net-next tree, since this is a new feature.
Thanks,
Stefano
Caution: This is an external email. Be cautious while opening links or attachments.
Hi Arseny,
On Mon, Aug 23, 2021 at 09:41:16PM +0300, Arseny Krasnov wrote:
quoted
Hello, please ping :)
Sorry, I was off last week.
I left some minor comments in the patches.
Let's wait a bit for other comments before next version, also on the
spec, then I think you can send the next version without RFC tag.
The target should be the net-next tree, since this is a new feature.
Hello,
E.g. next version will be [net-next] instead of [RFC] for both
kernel and spec patches?
Thank You
On Tue, Aug 24, 2021 at 01:18:06PM +0300, Arseny Krasnov wrote:
On 24.08.2021 13:05, Stefano Garzarella wrote:
quoted
Caution: This is an external email. Be cautious while opening links or attachments.
Hi Arseny,
On Mon, Aug 23, 2021 at 09:41:16PM +0300, Arseny Krasnov wrote:
quoted
Hello, please ping :)
Sorry, I was off last week.
I left some minor comments in the patches.
Let's wait a bit for other comments before next version, also on the
spec, then I think you can send the next version without RFC tag.
The target should be the net-next tree, since this is a new feature.
Hello,
E.g. next version will be [net-next] instead of [RFC] for both
kernel and spec patches?
Nope, net-next tag is useful only for kernel patches (net tree -
Documentation/networking/netdev-FAQ.rst).
Thanks,
Stefano
Caution: This is an external email. Be cautious while opening links or attachments.
On Tue, Aug 24, 2021 at 01:18:06PM +0300, Arseny Krasnov wrote:
quoted
On 24.08.2021 13:05, Stefano Garzarella wrote:
quoted
Caution: This is an external email. Be cautious while opening links or attachments.
Hi Arseny,
On Mon, Aug 23, 2021 at 09:41:16PM +0300, Arseny Krasnov wrote:
quoted
Hello, please ping :)
Sorry, I was off last week.
I left some minor comments in the patches.
Let's wait a bit for other comments before next version, also on the
spec, then I think you can send the next version without RFC tag.
The target should be the net-next tree, since this is a new feature.
Hello,
E.g. next version will be [net-next] instead of [RFC] for both
kernel and spec patches?
Nope, net-next tag is useful only for kernel patches (net tree -
Documentation/networking/netdev-FAQ.rst).
Caution: This is an external email. Be cautious while opening links or attachments.
On Tue, Aug 24, 2021 at 01:18:06PM +0300, Arseny Krasnov wrote:
quoted
On 24.08.2021 13:05, Stefano Garzarella wrote:
quoted
Caution: This is an external email. Be cautious while opening links or attachments.
Hi Arseny,
On Mon, Aug 23, 2021 at 09:41:16PM +0300, Arseny Krasnov wrote:
quoted
Hello, please ping :)
Sorry, I was off last week.
I left some minor comments in the patches.
Let's wait a bit for other comments before next version, also on the
spec, then I think you can send the next version without RFC tag.
The target should be the net-next tree, since this is a new feature.
Hello,
E.g. next version will be [net-next] instead of [RFC] for both
kernel and spec patches?
Nope, net-next tag is useful only for kernel patches (net tree -
Documentation/networking/netdev-FAQ.rst).
Ack
Hello,
as there are no new comments on this week, i can send
new patchsets for both kernel and spec today. Kernel patches
will be with 'net-next' tag instead of RFC, spec patches will be
without RFC tag.
Thank You