LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [RFC PATCH v2 0/5] virtio/vsock: introduce MSG_EOR flag for SEQPACKET
@ 2021-08-10 11:38 Arseny Krasnov
  2021-08-10 11:39 ` [RFC PATCH v2 1/5] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOM' bit Arseny Krasnov
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Arseny Krasnov @ 2021-08-10 11:38 UTC (permalink / raw)
  To: Stefan Hajnoczi, Stefano Garzarella, Michael S. Tsirkin,
	Jason Wang, David S. Miller, Jakub Kicinski, Arseny Krasnov,
	Norbert Slusarek, Colin Ian King, Andra Paraschiv
  Cc: kvm, virtualization, netdev, linux-kernel, stsp2, oxffffaa

	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 <arseny.krasnov@kaspersky.com>

-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC PATCH v2 1/5] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOM' bit
  2021-08-10 11:38 [RFC PATCH v2 0/5] virtio/vsock: introduce MSG_EOR flag for SEQPACKET Arseny Krasnov
@ 2021-08-10 11:39 ` Arseny Krasnov
  2021-08-11  9:00   ` Stefano Garzarella
  2021-08-10 11:40 ` [RFC PATCH v2 2/5] vhost/vsock: support MSG_EOR bit processing Arseny Krasnov
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Arseny Krasnov @ 2021-08-10 11:39 UTC (permalink / raw)
  To: Stefan Hajnoczi, Stefano Garzarella, Michael S. Tsirkin,
	Jason Wang, David S. Miller, Jakub Kicinski, Arseny Krasnov,
	Colin Ian King, Norbert Slusarek, Andra Paraschiv
  Cc: kvm, virtualization, netdev, linux-kernel, stsp2, oxffffaa

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 <arseny.krasnov@kaspersky.com>
---
 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(-)

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..64738838bee5 100644
--- a/include/uapi/linux/virtio_vsock.h
+++ b/include/uapi/linux/virtio_vsock.h
@@ -97,7 +97,8 @@ enum virtio_vsock_shutdown {
 
 /* VIRTIO_VSOCK_OP_RW flags values */
 enum virtio_vsock_rw {
-	VIRTIO_VSOCK_SEQ_EOR = 1,
+	VIRTIO_VSOCK_SEQ_EOM = 1,
+	VIRTIO_VSOCK_SEQ_EOR = 2,
 };
 
 #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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC PATCH v2 2/5] vhost/vsock: support MSG_EOR bit processing
  2021-08-10 11:38 [RFC PATCH v2 0/5] virtio/vsock: introduce MSG_EOR flag for SEQPACKET Arseny Krasnov
  2021-08-10 11:39 ` [RFC PATCH v2 1/5] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOM' bit Arseny Krasnov
@ 2021-08-10 11:40 ` Arseny Krasnov
  2021-08-11  9:06   ` Stefano Garzarella
  2021-08-10 11:40 ` [RFC PATCH v2 3/5] virito/vsock: " Arseny Krasnov
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Arseny Krasnov @ 2021-08-10 11:40 UTC (permalink / raw)
  To: Stefan Hajnoczi, Stefano Garzarella, Michael S. Tsirkin,
	Jason Wang, David S. Miller, Jakub Kicinski, Arseny Krasnov,
	Jorgen Hansen, Norbert Slusarek, Colin Ian King, Andra Paraschiv
  Cc: kvm, virtualization, netdev, linux-kernel, stsp2, oxffffaa

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 <arseny.krasnov@kaspersky.com>
---
 drivers/vhost/vsock.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index feaf650affbe..06fc132b13c8 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;
+		uint32_t flags_to_restore = 0;
 
 		spin_lock_bh(&vsock->send_pkt_list_lock);
 		if (list_empty(&vsock->send_pkt_list)) {
@@ -187,7 +187,12 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
 			 */
 			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 |= le32_to_cpu(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 |= le32_to_cpu(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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC PATCH v2 3/5] virito/vsock: support MSG_EOR bit processing
  2021-08-10 11:38 [RFC PATCH v2 0/5] virtio/vsock: introduce MSG_EOR flag for SEQPACKET Arseny Krasnov
  2021-08-10 11:39 ` [RFC PATCH v2 1/5] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOM' bit Arseny Krasnov
  2021-08-10 11:40 ` [RFC PATCH v2 2/5] vhost/vsock: support MSG_EOR bit processing Arseny Krasnov
@ 2021-08-10 11:40 ` Arseny Krasnov
  2021-08-11  9:09   ` Stefano Garzarella
  2021-08-10 11:41 ` [RFC PATCH v2 4/5] af_vsock: rename variables in receive loop Arseny Krasnov
  2021-08-10 11:41 ` [RFC PATCH v2 5/5] vsock_test: update message bounds test for MSG_EOR Arseny Krasnov
  4 siblings, 1 reply; 11+ messages in thread
From: Arseny Krasnov @ 2021-08-10 11:40 UTC (permalink / raw)
  To: Stefan Hajnoczi, Stefano Garzarella, Michael S. Tsirkin,
	Jason Wang, David S. Miller, Jakub Kicinski, Arseny Krasnov,
	Colin Ian King, Norbert Slusarek, Andra Paraschiv
  Cc: kvm, virtualization, netdev, linux-kernel, stsp2, oxffffaa

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

Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com>
---
 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC PATCH v2 4/5] af_vsock: rename variables in receive loop
  2021-08-10 11:38 [RFC PATCH v2 0/5] virtio/vsock: introduce MSG_EOR flag for SEQPACKET Arseny Krasnov
                   ` (2 preceding siblings ...)
  2021-08-10 11:40 ` [RFC PATCH v2 3/5] virito/vsock: " Arseny Krasnov
@ 2021-08-10 11:41 ` Arseny Krasnov
  2021-08-11  9:09   ` Stefano Garzarella
  2021-08-10 11:41 ` [RFC PATCH v2 5/5] vsock_test: update message bounds test for MSG_EOR Arseny Krasnov
  4 siblings, 1 reply; 11+ messages in thread
From: Arseny Krasnov @ 2021-08-10 11:41 UTC (permalink / raw)
  To: Stefan Hajnoczi, Stefano Garzarella, Michael S. Tsirkin,
	Jason Wang, David S. Miller, Jakub Kicinski, Arseny Krasnov,
	Colin Ian King, Norbert Slusarek, Andra Paraschiv
  Cc: kvm, virtualization, netdev, linux-kernel, stsp2, oxffffaa

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 <arseny.krasnov@kaspersky.com>
---
 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC PATCH v2 5/5] vsock_test: update message bounds test for MSG_EOR
  2021-08-10 11:38 [RFC PATCH v2 0/5] virtio/vsock: introduce MSG_EOR flag for SEQPACKET Arseny Krasnov
                   ` (3 preceding siblings ...)
  2021-08-10 11:41 ` [RFC PATCH v2 4/5] af_vsock: rename variables in receive loop Arseny Krasnov
@ 2021-08-10 11:41 ` Arseny Krasnov
  2021-08-11  9:12   ` Stefano Garzarella
  4 siblings, 1 reply; 11+ messages in thread
From: Arseny Krasnov @ 2021-08-10 11:41 UTC (permalink / raw)
  To: Stefan Hajnoczi, Stefano Garzarella, Michael S. Tsirkin,
	Jason Wang, David S. Miller, Jakub Kicinski, Arseny Krasnov,
	Norbert Slusarek, Andra Paraschiv, Colin Ian King
  Cc: kvm, virtualization, netdev, linux-kernel, stsp2, oxffffaa

Set 'MSG_EOR' in one of message sent, check that 'MSG_EOR'
is visible in corresponding message at receiver.

Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com>
---
 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH v2 1/5] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOM' bit
  2021-08-10 11:39 ` [RFC PATCH v2 1/5] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOM' bit Arseny Krasnov
@ 2021-08-11  9:00   ` Stefano Garzarella
  0 siblings, 0 replies; 11+ messages in thread
From: Stefano Garzarella @ 2021-08-11  9:00 UTC (permalink / raw)
  To: Arseny Krasnov
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, David S. Miller,
	Jakub Kicinski, Colin Ian King, Norbert Slusarek,
	Andra Paraschiv, kvm, virtualization, netdev, linux-kernel,
	stsp2, oxffffaa

On Tue, Aug 10, 2021 at 02:39:53PM +0300, Arseny Krasnov wrote:

The title is confusing, we are renaming EOR in EOM.

>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 <arseny.krasnov@kaspersky.com>
>---
> 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(-)
>
>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..64738838bee5 100644
>--- a/include/uapi/linux/virtio_vsock.h
>+++ b/include/uapi/linux/virtio_vsock.h
>@@ -97,7 +97,8 @@ enum virtio_vsock_shutdown {
>
> /* VIRTIO_VSOCK_OP_RW flags values */
> enum virtio_vsock_rw {
>-	VIRTIO_VSOCK_SEQ_EOR = 1,
>+	VIRTIO_VSOCK_SEQ_EOM = 1,
>+	VIRTIO_VSOCK_SEQ_EOR = 2,
         ^
I think is better to add this new flag in a separate patch.

> };
>
> #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
>

The rest LGTM!

Thanks,
Stefano


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH v2 2/5] vhost/vsock: support MSG_EOR bit processing
  2021-08-10 11:40 ` [RFC PATCH v2 2/5] vhost/vsock: support MSG_EOR bit processing Arseny Krasnov
@ 2021-08-11  9:06   ` Stefano Garzarella
  0 siblings, 0 replies; 11+ messages in thread
From: Stefano Garzarella @ 2021-08-11  9:06 UTC (permalink / raw)
  To: Arseny Krasnov
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, David S. Miller,
	Jakub Kicinski, Jorgen Hansen, Norbert Slusarek, Colin Ian King,
	Andra Paraschiv, kvm, virtualization, netdev, linux-kernel,
	stsp2, oxffffaa

On Tue, Aug 10, 2021 at 02:40:15PM +0300, Arseny Krasnov wrote:
>It works in the same way as 'end-of-message' bit: if packet has
>'EOM' bit, also check for 'EOR' bit.

Please describe all changes, e.g. the new variable to accumulate flags 
to restore.

>
>Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com>
>---
> drivers/vhost/vsock.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index feaf650affbe..06fc132b13c8 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;
>+		uint32_t flags_to_restore = 0;
>
> 		spin_lock_bh(&vsock->send_pkt_list_lock);
> 		if (list_empty(&vsock->send_pkt_list)) {
>@@ -187,7 +187,12 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
> 			 */
> 			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 |= le32_to_cpu(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 |= le32_to_cpu(VIRTIO_VSOCK_SEQ_EOR);
                                                             ^
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.

>+				}
> 			}
> 		}
>
>@@ -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
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH v2 3/5] virito/vsock: support MSG_EOR bit processing
  2021-08-10 11:40 ` [RFC PATCH v2 3/5] virito/vsock: " Arseny Krasnov
@ 2021-08-11  9:09   ` Stefano Garzarella
  0 siblings, 0 replies; 11+ messages in thread
From: Stefano Garzarella @ 2021-08-11  9:09 UTC (permalink / raw)
  To: Arseny Krasnov
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, David S. Miller,
	Jakub Kicinski, Colin Ian King, Norbert Slusarek,
	Andra Paraschiv, kvm, virtualization, netdev, linux-kernel,
	stsp2, oxffffaa

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 <arseny.krasnov@kaspersky.com>
>---
> 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 <sgarzare@redhat.com>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH v2 4/5] af_vsock: rename variables in receive loop
  2021-08-10 11:41 ` [RFC PATCH v2 4/5] af_vsock: rename variables in receive loop Arseny Krasnov
@ 2021-08-11  9:09   ` Stefano Garzarella
  0 siblings, 0 replies; 11+ messages in thread
From: Stefano Garzarella @ 2021-08-11  9:09 UTC (permalink / raw)
  To: Arseny Krasnov
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, David S. Miller,
	Jakub Kicinski, Colin Ian King, Norbert Slusarek,
	Andra Paraschiv, kvm, virtualization, netdev, linux-kernel,
	stsp2, oxffffaa

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 <arseny.krasnov@kaspersky.com>
>---
> 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 <sgarzare@redhat.com>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH v2 5/5] vsock_test: update message bounds test for MSG_EOR
  2021-08-10 11:41 ` [RFC PATCH v2 5/5] vsock_test: update message bounds test for MSG_EOR Arseny Krasnov
@ 2021-08-11  9:12   ` Stefano Garzarella
  0 siblings, 0 replies; 11+ messages in thread
From: Stefano Garzarella @ 2021-08-11  9:12 UTC (permalink / raw)
  To: Arseny Krasnov
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, David S. Miller,
	Jakub Kicinski, Norbert Slusarek, Andra Paraschiv,
	Colin Ian King, kvm, virtualization, netdev, linux-kernel, stsp2,
	oxffffaa

On Tue, Aug 10, 2021 at 02:41:16PM +0300, Arseny Krasnov wrote:
>Set 'MSG_EOR' in one of message sent, check that 'MSG_EOR'
>is visible in corresponding message at receiver.
>
>Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com>
>---
> 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
>

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-08-11  9:12 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-10 11:38 [RFC PATCH v2 0/5] virtio/vsock: introduce MSG_EOR flag for SEQPACKET Arseny Krasnov
2021-08-10 11:39 ` [RFC PATCH v2 1/5] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOM' bit Arseny Krasnov
2021-08-11  9:00   ` Stefano Garzarella
2021-08-10 11:40 ` [RFC PATCH v2 2/5] vhost/vsock: support MSG_EOR bit processing Arseny Krasnov
2021-08-11  9:06   ` Stefano Garzarella
2021-08-10 11:40 ` [RFC PATCH v2 3/5] virito/vsock: " Arseny Krasnov
2021-08-11  9:09   ` Stefano Garzarella
2021-08-10 11:41 ` [RFC PATCH v2 4/5] af_vsock: rename variables in receive loop Arseny Krasnov
2021-08-11  9:09   ` Stefano Garzarella
2021-08-10 11:41 ` [RFC PATCH v2 5/5] vsock_test: update message bounds test for MSG_EOR Arseny Krasnov
2021-08-11  9:12   ` Stefano Garzarella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).