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 <redacted>
--
2.25.1
This bit is used to mark end of messages('EOM' - end of message), while
'VIRIO_VSOCK_SEQ_EOR' is used to pass MSG_EOR. 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 | 3 ++-
net/vmw_vsock/virtio_transport_common.c | 14 +++++++-------
3 files changed, 15 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;
It works in the same way as 'end-of-message' bit: if packet has
'EOM' bit, also check for 'EOR' bit.
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>
---
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>
---
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);
On Tue, Aug 10, 2021 at 02:39:53PM +0300, Arseny Krasnov wrote:
The title is confusing, we are renaming EOR in EOM.
quoted hunk
This bit is used to mark end of messages('EOM' - end of message), while
'VIRIO_VSOCK_SEQ_EOR' is used to pass MSG_EOR. 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 | 3 ++-
net/vmw_vsock/virtio_transport_common.c | 14 +++++++-------
3 files changed, 15 insertions(+), 14 deletions(-)
* small rx buffers, headers of packets in rx queue are
* created dynamically and are initialized with
header
* of current packet(except length). But in case of
- * SOCK_SEQPACKET, we also must clear record delimeter
- * bit(VIRTIO_VSOCK_SEQ_EOR). Otherwise, instead of one
- * packet with delimeter(which marks end of record),
+ * SOCK_SEQPACKET, we also must clear message delimeter
+ * bit(VIRTIO_VSOCK_SEQ_EOM). Otherwise, instead of one
+ * packet with delimeter(which marks end of message),
* there will be sequence of packets with delimeter
* bit set. After initialized header will be copied to
* rx buffer, this bit will be restored.
*/
- if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOR) {
- pkt->hdr.flags &= ~cpu_to_le32(VIRTIO_VSOCK_SEQ_EOR);
+ if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOM) {
+ pkt->hdr.flags &= ~cpu_to_le32(VIRTIO_VSOCK_SEQ_EOM);
restore_flag = true;
}
}
*/
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
* the remaining bytes, and we want to deliver it
goto out;
}
- 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,
/* If there is space in the last packet queued, we copy the
* new packet in its buffer. We avoid this if the last packet
- * queued has VIRTIO_VSOCK_SEQ_EOR set, because this is
- * delimiter of SEQPACKET record, so 'pkt' is the first packet
- * of a new record.
+ * queued has VIRTIO_VSOCK_SEQ_EOM set, because this is
+ * delimiter of SEQPACKET message, so 'pkt' is the first packet
+ * of a new message.
*/
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;
--
2.25.1
^
I'm not sure this is needed, VIRTIO_VSOCK_SEQ_EOR is represented in the
cpu endianess.
I think here you can simpy do `flags_to_restore |= VIRTIO_VSOCK_SEQ_EOR`
then use `pkt->hdr.flags |= cpu_to_le32(flags_to_restore);` as you
already do.
* 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
On Tue, Aug 10, 2021 at 02:41:00PM +0300, Arseny Krasnov wrote:
quoted hunk
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>
---
net/vmw_vsock/af_vsock.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
On Tue, Aug 10, 2021 at 02:41:16PM +0300, Arseny Krasnov wrote:
quoted hunk
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>
---
tools/testing/vsock/vsock_test.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
/* Send several messages, one with MSG_EOR flag */
for (int i = 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);