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 | 28 +++++++++++++----------
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, 45 insertions(+), 27 deletions(-)
v4 -> v5:
- Move bitwise and out of le32_to_cpu() in 0003.
v3 -> v4:
- 'sendXXX()' renamed to 'send*()' in 0002- commit msg.
- Comment about bit restore updated in 0003-.
- 'same' renamed to 'similar' in 0003- commit msg.
- u32 used instead of uint32_t in 0003-.
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 <[email protected]>
--
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 <[email protected]>
Reviewed-by: Stefano Garzarella <[email protected]>
---
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(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index f249622ef11b..feaf650affbe 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -178,15 +178,15 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
* 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;
}
}
@@ -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
* the remaining bytes, and we want to deliver it
diff --git a/include/uapi/linux/virtio_vsock.h b/include/uapi/linux/virtio_vsock.h
index 3dd3555b2740..8485b004a5f8 100644
--- a/include/uapi/linux/virtio_vsock.h
+++ b/include/uapi/linux/virtio_vsock.h
@@ -97,7 +97,7 @@ enum virtio_vsock_shutdown {
/* VIRTIO_VSOCK_OP_RW flags values */
enum virtio_vsock_rw {
- VIRTIO_VSOCK_SEQ_EOR = 1,
+ VIRTIO_VSOCK_SEQ_EOM = 1,
};
#endif /* _UAPI_LINUX_VIRTIO_VSOCK_H */
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 081e7ae93cb1..4d5a93beceb0 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -77,7 +77,7 @@ virtio_transport_alloc_pkt(struct virtio_vsock_pkt_info *info,
if (msg_data_left(info->msg) == 0 &&
info->type == VIRTIO_VSOCK_TYPE_SEQPACKET)
- pkt->hdr.flags |= cpu_to_le32(VIRTIO_VSOCK_SEQ_EOR);
+ pkt->hdr.flags |= cpu_to_le32(VIRTIO_VSOCK_SEQ_EOM);
}
trace_virtio_transport_alloc_pkt(src_cid, src_port,
@@ -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,
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,
@@ -1044,12 +1044,12 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
/* 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
'MSG_EOR' handling has similar 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 <[email protected]>
---
drivers/vhost/vsock.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index feaf650affbe..938aefbc75ec 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -114,7 +114,7 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
size_t nbytes;
size_t iov_len, payload_len;
int head;
- bool restore_flag = false;
+ u32 flags_to_restore = 0;
spin_lock_bh(&vsock->send_pkt_list_lock);
if (list_empty(&vsock->send_pkt_list)) {
@@ -179,15 +179,20 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
* created dynamically and are initialized with header
* of current packet(except length). But in case of
* 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.
+ * bit (VIRTIO_VSOCK_SEQ_EOM) and MSG_EOR bit
+ * (VIRTIO_VSOCK_SEQ_EOR) if set. Otherwise,
+ * there will be sequence of packets with these
+ * bits set. After initialized header will be copied to
+ * rx buffer, these required bits will be restored.
*/
if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOM) {
pkt->hdr.flags &= ~cpu_to_le32(VIRTIO_VSOCK_SEQ_EOM);
- restore_flag = true;
+ flags_to_restore |= VIRTIO_VSOCK_SEQ_EOM;
+
+ if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOR) {
+ pkt->hdr.flags &= ~cpu_to_le32(VIRTIO_VSOCK_SEQ_EOR);
+ flags_to_restore |= VIRTIO_VSOCK_SEQ_EOR;
+ }
}
}
@@ -224,8 +229,7 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
* 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
If packet has 'EOR' bit - set MSG_EOR in 'recvmsg()' flags.
Signed-off-by: Arseny Krasnov <[email protected]>
Reviewed-by: Stefano Garzarella <[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
Set 'MSG_EOR' in one of message sent, check that 'MSG_EOR'
is visible in corresponding message at receiver.
Signed-off-by: Arseny Krasnov <[email protected]>
Reviewed-by: Stefano Garzarella <[email protected]>
---
tools/testing/vsock/vsock_test.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index 67766bfe176f..2a3638c0a008 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -282,6 +282,7 @@ static void test_stream_msg_peek_server(const struct test_opts *opts)
}
#define MESSAGES_CNT 7
+#define MSG_EOR_IDX (MESSAGES_CNT / 2)
static void test_seqpacket_msg_bounds_client(const struct test_opts *opts)
{
int fd;
@@ -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 (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);
@@ -324,6 +325,11 @@ static void test_seqpacket_msg_bounds_server(const struct test_opts *opts)
perror("message bound violated");
exit(EXIT_FAILURE);
}
+
+ if ((i == MSG_EOR_IDX) ^ !!(msg.msg_flags & MSG_EOR)) {
+ perror("MSG_EOR");
+ exit(EXIT_FAILURE);
+ }
}
close(fd);
--
2.25.1
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]>
Reviewed-by: Stefano Garzarella <[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
This bit is used to handle POSIX MSG_EOR flag passed from
userspace in 'send*()' system calls. It marks end of each
record and is visible to receiver using 'recvmsg()' system
call.
Signed-off-by: Arseny Krasnov <[email protected]>
Reviewed-by: Stefano Garzarella <[email protected]>
---
include/uapi/linux/virtio_vsock.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/virtio_vsock.h b/include/uapi/linux/virtio_vsock.h
index 8485b004a5f8..64738838bee5 100644
--- a/include/uapi/linux/virtio_vsock.h
+++ b/include/uapi/linux/virtio_vsock.h
@@ -98,6 +98,7 @@ enum virtio_vsock_shutdown {
/* VIRTIO_VSOCK_OP_RW flags values */
enum virtio_vsock_rw {
VIRTIO_VSOCK_SEQ_EOM = 1,
+ VIRTIO_VSOCK_SEQ_EOR = 2,
};
#endif /* _UAPI_LINUX_VIRTIO_VSOCK_H */
--
2.25.1
On Fri, Sep 03, 2021 at 03:32:35PM +0300, Arseny Krasnov wrote:
>'MSG_EOR' handling has similar 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 <[email protected]>
>---
> drivers/vhost/vsock.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index feaf650affbe..938aefbc75ec 100644
>--- a/drivers/vhost/vsock.c
>+++ b/drivers/vhost/vsock.c
>@@ -114,7 +114,7 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
> size_t nbytes;
> size_t iov_len, payload_len;
> int head;
>- bool restore_flag = false;
>+ u32 flags_to_restore = 0;
>
> spin_lock_bh(&vsock->send_pkt_list_lock);
> if (list_empty(&vsock->send_pkt_list)) {
>@@ -179,15 +179,20 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
> * created dynamically and are initialized with header
> * of current packet(except length). But in case of
> * 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.
>+ * bit (VIRTIO_VSOCK_SEQ_EOM) and MSG_EOR bit
>+ * (VIRTIO_VSOCK_SEQ_EOR) if set. Otherwise,
>+ * there will be sequence of packets with these
>+ * bits set. After initialized header will be copied to
>+ * rx buffer, these required bits will be restored.
> */
> if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOM) {
> pkt->hdr.flags &= ~cpu_to_le32(VIRTIO_VSOCK_SEQ_EOM);
>- restore_flag = true;
>+ flags_to_restore |= VIRTIO_VSOCK_SEQ_EOM;
>+
>+ if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOR) {
>+ pkt->hdr.flags &= ~cpu_to_le32(VIRTIO_VSOCK_SEQ_EOR);
>+ flags_to_restore |= VIRTIO_VSOCK_SEQ_EOR;
>+ }
> }
> }
>
>@@ -224,8 +229,7 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
> * 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
>
Reviewed-by: Stefano Garzarella <[email protected]>
On Fri, 3 Sep 2021 15:30:13 +0300 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.
Assuming you want this merged to net-next:
# Form letter - net-next is closed
We have already sent the networking pull request for 5.15
and therefore net-next is closed for new drivers, features,
code refactoring and optimizations. We are currently accepting
bug fixes only.
Please repost when net-next reopens after 5.15-rc1 is cut.
Look out for the announcement on the mailing list or check:
http://vger.kernel.org/~davem/net-next.html
RFC patches sent for review only are obviously welcome at any time.
On Fri, Sep 03, 2021 at 03:30:13PM +0300, 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.
I'm prepared to merge this for this window,
but I'm not sure who's supposed to ack the net/vmw_vsock/af_vsock.c
bits. It's a harmless variable renaming so maybe it does not matter.
The rest is virtio stuff so I guess my tree is ok.
Objections, anyone?
> 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 | 28 +++++++++++++----------
> 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, 45 insertions(+), 27 deletions(-)
>
> v4 -> v5:
> - Move bitwise and out of le32_to_cpu() in 0003.
>
> v3 -> v4:
> - 'sendXXX()' renamed to 'send*()' in 0002- commit msg.
> - Comment about bit restore updated in 0003-.
> - 'same' renamed to 'similar' in 0003- commit msg.
> - u32 used instead of uint32_t in 0003-.
>
> 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 <[email protected]>
> --
> 2.25.1
On 05.09.2021 18:55, Michael S. Tsirkin wrote:
> On Fri, Sep 03, 2021 at 03:30:13PM +0300, 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.
>
> I'm prepared to merge this for this window,
> but I'm not sure who's supposed to ack the net/vmw_vsock/af_vsock.c
> bits. It's a harmless variable renaming so maybe it does not matter.
>
> The rest is virtio stuff so I guess my tree is ok.
>
> Objections, anyone?
https://lkml.org/lkml/2021/9/3/76 this is v4. It is same as v5 in af_vsock.c changes.
It has Reviewed by from Stefano Garzarella.
>
>
>> 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 | 28 +++++++++++++----------
>> 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, 45 insertions(+), 27 deletions(-)
>>
>> v4 -> v5:
>> - Move bitwise and out of le32_to_cpu() in 0003.
>>
>> v3 -> v4:
>> - 'sendXXX()' renamed to 'send*()' in 0002- commit msg.
>> - Comment about bit restore updated in 0003-.
>> - 'same' renamed to 'similar' in 0003- commit msg.
>> - u32 used instead of uint32_t in 0003-.
>>
>> 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 <[email protected]>
>> --
>> 2.25.1
>
On Sun, Sep 05, 2021 at 07:02:44PM +0300, Arseny Krasnov wrote:
>
> On 05.09.2021 18:55, Michael S. Tsirkin wrote:
> > On Fri, Sep 03, 2021 at 03:30:13PM +0300, 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.
> >
> > I'm prepared to merge this for this window,
> > but I'm not sure who's supposed to ack the net/vmw_vsock/af_vsock.c
> > bits. It's a harmless variable renaming so maybe it does not matter.
> >
> > The rest is virtio stuff so I guess my tree is ok.
> >
> > Objections, anyone?
>
> https://lkml.org/lkml/2021/9/3/76 this is v4. It is same as v5 in af_vsock.c changes.
>
> It has Reviewed by from Stefano Garzarella.
Is Stefano the maintainer for af_vsock then?
I wasn't sure.
> >
> >
> >> 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 | 28 +++++++++++++----------
> >> 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, 45 insertions(+), 27 deletions(-)
> >>
> >> v4 -> v5:
> >> - Move bitwise and out of le32_to_cpu() in 0003.
> >>
> >> v3 -> v4:
> >> - 'sendXXX()' renamed to 'send*()' in 0002- commit msg.
> >> - Comment about bit restore updated in 0003-.
> >> - 'same' renamed to 'similar' in 0003- commit msg.
> >> - u32 used instead of uint32_t in 0003-.
> >>
> >> 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 <[email protected]>
> >> --
> >> 2.25.1
> >
On 05.09.2021 19:19, Michael S. Tsirkin wrote:
> On Sun, Sep 05, 2021 at 07:02:44PM +0300, Arseny Krasnov wrote:
>> On 05.09.2021 18:55, Michael S. Tsirkin wrote:
>>> On Fri, Sep 03, 2021 at 03:30:13PM +0300, 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.
>>> I'm prepared to merge this for this window,
>>> but I'm not sure who's supposed to ack the net/vmw_vsock/af_vsock.c
>>> bits. It's a harmless variable renaming so maybe it does not matter.
>>>
>>> The rest is virtio stuff so I guess my tree is ok.
>>>
>>> Objections, anyone?
>> https://lkml.org/lkml/2021/9/3/76 this is v4. It is same as v5 in af_vsock.c changes.
>>
>> It has Reviewed by from Stefano Garzarella.
> Is Stefano the maintainer for af_vsock then?
> I wasn't sure.
Ack, let's wait for maintainer's comment
>
>>>
>>>> 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 | 28 +++++++++++++----------
>>>> 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, 45 insertions(+), 27 deletions(-)
>>>>
>>>> v4 -> v5:
>>>> - Move bitwise and out of le32_to_cpu() in 0003.
>>>>
>>>> v3 -> v4:
>>>> - 'sendXXX()' renamed to 'send*()' in 0002- commit msg.
>>>> - Comment about bit restore updated in 0003-.
>>>> - 'same' renamed to 'similar' in 0003- commit msg.
>>>> - u32 used instead of uint32_t in 0003-.
>>>>
>>>> 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 <[email protected]>
>>>> --
>>>> 2.25.1
>
On Sun, Sep 05, 2021 at 07:21:10PM +0300, Arseny Krasnov wrote:
>
> On 05.09.2021 19:19, Michael S. Tsirkin wrote:
> > On Sun, Sep 05, 2021 at 07:02:44PM +0300, Arseny Krasnov wrote:
> >> On 05.09.2021 18:55, Michael S. Tsirkin wrote:
> >>> On Fri, Sep 03, 2021 at 03:30:13PM +0300, 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.
> >>> I'm prepared to merge this for this window,
> >>> but I'm not sure who's supposed to ack the net/vmw_vsock/af_vsock.c
> >>> bits. It's a harmless variable renaming so maybe it does not matter.
> >>>
> >>> The rest is virtio stuff so I guess my tree is ok.
> >>>
> >>> Objections, anyone?
> >> https://lkml.org/lkml/2021/9/3/76 this is v4. It is same as v5 in af_vsock.c changes.
> >>
> >> It has Reviewed by from Stefano Garzarella.
> > Is Stefano the maintainer for af_vsock then?
> > I wasn't sure.
> Ack, let's wait for maintainer's comment
The specific patch is a trivial variable renaming so
I parked this in my tree for now, will merge unless I
hear any objections in the next couple of days.
> >>>
> >>>> 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 | 28 +++++++++++++----------
> >>>> 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, 45 insertions(+), 27 deletions(-)
> >>>>
> >>>> v4 -> v5:
> >>>> - Move bitwise and out of le32_to_cpu() in 0003.
> >>>>
> >>>> v3 -> v4:
> >>>> - 'sendXXX()' renamed to 'send*()' in 0002- commit msg.
> >>>> - Comment about bit restore updated in 0003-.
> >>>> - 'same' renamed to 'similar' in 0003- commit msg.
> >>>> - u32 used instead of uint32_t in 0003-.
> >>>>
> >>>> 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 <[email protected]>
> >>>> --
> >>>> 2.25.1
> >
On Sun, Sep 05, 2021 at 04:18:52PM -0400, Michael S. Tsirkin wrote:
>On Sun, Sep 05, 2021 at 07:21:10PM +0300, Arseny Krasnov wrote:
>>
>> On 05.09.2021 19:19, Michael S. Tsirkin wrote:
>> > On Sun, Sep 05, 2021 at 07:02:44PM +0300, Arseny Krasnov wrote:
>> >> On 05.09.2021 18:55, Michael S. Tsirkin wrote:
>> >>> On Fri, Sep 03, 2021 at 03:30:13PM +0300, 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.
>> >>> I'm prepared to merge this for this window,
>> >>> but I'm not sure who's supposed to ack the net/vmw_vsock/af_vsock.c
>> >>> bits. It's a harmless variable renaming so maybe it does not matter.
>> >>>
>> >>> The rest is virtio stuff so I guess my tree is ok.
>> >>>
>> >>> Objections, anyone?
>> >> https://lkml.org/lkml/2021/9/3/76 this is v4. It is same as v5 in af_vsock.c changes.
>> >>
>> >> It has Reviewed by from Stefano Garzarella.
>> > Is Stefano the maintainer for af_vsock then?
>> > I wasn't sure.
I'm maintaining virtio-vsock stuff, but I'm reviewing most of the
af_vsock patches. We don't have an entry for it in MAINTAINERS, maybe we
should.
>> Ack, let's wait for maintainer's comment
>
>
>The specific patch is a trivial variable renaming so
>I parked this in my tree for now, will merge unless I
>hear any objections in the next couple of days.
I agree, I think your tree is fine, since this series is mostly about
virtio-vsock.
Thanks,
Stefano
On Mon, Sep 06, 2021 at 09:33:15AM +0200, Stefano Garzarella wrote:
> On Sun, Sep 05, 2021 at 04:18:52PM -0400, Michael S. Tsirkin wrote:
> > On Sun, Sep 05, 2021 at 07:21:10PM +0300, Arseny Krasnov wrote:
> > >
> > > On 05.09.2021 19:19, Michael S. Tsirkin wrote:
> > > > On Sun, Sep 05, 2021 at 07:02:44PM +0300, Arseny Krasnov wrote:
> > > >> On 05.09.2021 18:55, Michael S. Tsirkin wrote:
> > > >>> On Fri, Sep 03, 2021 at 03:30:13PM +0300, 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.
> > > >>> I'm prepared to merge this for this window,
> > > >>> but I'm not sure who's supposed to ack the net/vmw_vsock/af_vsock.c
> > > >>> bits. It's a harmless variable renaming so maybe it does not matter.
> > > >>>
> > > >>> The rest is virtio stuff so I guess my tree is ok.
> > > >>>
> > > >>> Objections, anyone?
> > > >> https://lkml.org/lkml/2021/9/3/76 this is v4. It is same as v5 in af_vsock.c changes.
> > > >>
> > > >> It has Reviewed by from Stefano Garzarella.
> > > > Is Stefano the maintainer for af_vsock then?
> > > > I wasn't sure.
>
> I'm maintaining virtio-vsock stuff, but I'm reviewing most of the af_vsock
> patches. We don't have an entry for it in MAINTAINERS, maybe we should.
Yea, please add that. And the test I guess?
It's now Dave and while he's great as we all know,
reducing the load on him is a good thing to do.
> > > Ack, let's wait for maintainer's comment
> >
> >
> > The specific patch is a trivial variable renaming so
> > I parked this in my tree for now, will merge unless I
> > hear any objections in the next couple of days.
>
> I agree, I think your tree is fine, since this series is mostly about
> virtio-vsock.
>
> Thanks,
> Stefano