* [PATCH net-next v4 1/6] virtio/vsock: rename 'EOR' to 'EOM' bit.
2021-09-03 6:13 [PATCH net-next v4 0/6] virtio/vsock: introduce MSG_EOR flag for SEQPACKET Arseny Krasnov
@ 2021-09-03 6:14 ` Arseny Krasnov
2021-09-03 6:15 ` [PATCH net-next v4 2/6] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOR' bit Arseny Krasnov
` (4 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Arseny Krasnov @ 2021-09-03 6:14 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 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 <arseny.krasnov@kaspersky.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next v4 2/6] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOR' bit.
2021-09-03 6:13 [PATCH net-next v4 0/6] virtio/vsock: introduce MSG_EOR flag for SEQPACKET Arseny Krasnov
2021-09-03 6:14 ` [PATCH net-next v4 1/6] virtio/vsock: rename 'EOR' to 'EOM' bit Arseny Krasnov
@ 2021-09-03 6:15 ` Arseny Krasnov
2021-09-05 15:50 ` Michael S. Tsirkin
2021-09-03 6:15 ` [PATCH net-next v4 3/6] vhost/vsock: support MSG_EOR bit processing Arseny Krasnov
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Arseny Krasnov @ 2021-09-03 6:15 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
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 <arseny.krasnov@kaspersky.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v4 2/6] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOR' bit.
2021-09-03 6:15 ` [PATCH net-next v4 2/6] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOR' bit Arseny Krasnov
@ 2021-09-05 15:50 ` Michael S. Tsirkin
0 siblings, 0 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2021-09-05 15:50 UTC (permalink / raw)
To: Arseny Krasnov
Cc: Stefan Hajnoczi, Stefano Garzarella, Jason Wang, David S. Miller,
Jakub Kicinski, Norbert Slusarek, Andra Paraschiv,
Colin Ian King, kvm, virtualization, netdev, linux-kernel, stsp2,
oxffffaa
On Fri, Sep 03, 2021 at 09:15:20AM +0300, Arseny Krasnov wrote:
> 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 <arseny.krasnov@kaspersky.com>
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Spec patch for this?
> ---
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v4 2/6] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOR' bit.
@ 2021-09-05 15:50 ` Michael S. Tsirkin
0 siblings, 0 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2021-09-05 15:50 UTC (permalink / raw)
To: Arseny Krasnov
Cc: Andra Paraschiv, kvm, netdev, stsp2, linux-kernel,
virtualization, oxffffaa, Norbert Slusarek, Stefan Hajnoczi,
Colin Ian King, Jakub Kicinski, David S. Miller
On Fri, Sep 03, 2021 at 09:15:20AM +0300, Arseny Krasnov wrote:
> 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 <arseny.krasnov@kaspersky.com>
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Spec patch for this?
> ---
> 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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v4 2/6] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOR' bit.
2021-09-05 15:50 ` Michael S. Tsirkin
(?)
@ 2021-09-05 15:52 ` Arseny Krasnov
-1 siblings, 0 replies; 17+ messages in thread
From: Arseny Krasnov @ 2021-09-05 15:52 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Stefan Hajnoczi, Stefano Garzarella, Jason Wang, David S. Miller,
Jakub Kicinski, Norbert Slusarek, Andra Paraschiv,
Colin Ian King, kvm, virtualization, netdev, linux-kernel, stsp2,
oxffffaa
On 05.09.2021 18:50, Michael S. Tsirkin wrote:
> On Fri, Sep 03, 2021 at 09:15:20AM +0300, Arseny Krasnov wrote:
>> 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 <arseny.krasnov@kaspersky.com>
>> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> Spec patch for this?
Hello, here it is
https://lists.oasis-open.org/archives/virtio-comment/202109/msg00008.html
>
>> ---
>> 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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next v4 3/6] vhost/vsock: support MSG_EOR bit processing
2021-09-03 6:13 [PATCH net-next v4 0/6] virtio/vsock: introduce MSG_EOR flag for SEQPACKET Arseny Krasnov
2021-09-03 6:14 ` [PATCH net-next v4 1/6] virtio/vsock: rename 'EOR' to 'EOM' bit Arseny Krasnov
2021-09-03 6:15 ` [PATCH net-next v4 2/6] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOR' bit Arseny Krasnov
@ 2021-09-03 6:15 ` Arseny Krasnov
2021-09-03 6:55 ` Stefano Garzarella
2021-09-03 12:05 ` kernel test robot
2021-09-03 6:15 ` [PATCH net-next v4 4/6] virtio/vsock: " Arseny Krasnov
` (2 subsequent siblings)
5 siblings, 2 replies; 17+ messages in thread
From: Arseny Krasnov @ 2021-09-03 6:15 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
'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 <arseny.krasnov@kaspersky.com>
---
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..93e8d635e18f 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v4 3/6] vhost/vsock: support MSG_EOR bit processing
2021-09-03 6:15 ` [PATCH net-next v4 3/6] vhost/vsock: support MSG_EOR bit processing Arseny Krasnov
@ 2021-09-03 6:55 ` Stefano Garzarella
2021-09-03 12:05 ` kernel test robot
1 sibling, 0 replies; 17+ messages in thread
From: Stefano Garzarella @ 2021-09-03 6:55 UTC (permalink / raw)
To: Arseny Krasnov
Cc: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, David S. Miller,
Jakub Kicinski, Norbert Slusarek, Colin Ian King,
Andra Paraschiv, kvm, virtualization, netdev, linux-kernel,
stsp2, oxffffaa
On Fri, Sep 03, 2021 at 09:15:38AM +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 <arseny.krasnov@kaspersky.com>
>---
> 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..93e8d635e18f 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 <sgarzare@redhat.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v4 3/6] vhost/vsock: support MSG_EOR bit processing
@ 2021-09-03 6:55 ` Stefano Garzarella
0 siblings, 0 replies; 17+ messages in thread
From: Stefano Garzarella @ 2021-09-03 6:55 UTC (permalink / raw)
To: Arseny Krasnov
Cc: Andra Paraschiv, kvm, Michael S. Tsirkin, netdev, stsp2,
linux-kernel, virtualization, oxffffaa, Norbert Slusarek,
Stefan Hajnoczi, Colin Ian King, Jakub Kicinski, David S. Miller
On Fri, Sep 03, 2021 at 09:15:38AM +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 <arseny.krasnov@kaspersky.com>
>---
> 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..93e8d635e18f 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 <sgarzare@redhat.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v4 3/6] vhost/vsock: support MSG_EOR bit processing
2021-09-03 6:55 ` Stefano Garzarella
@ 2021-09-03 12:14 ` Stefano Garzarella
-1 siblings, 0 replies; 17+ messages in thread
From: Stefano Garzarella @ 2021-09-03 12:14 UTC (permalink / raw)
To: Arseny Krasnov
Cc: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, David S. Miller,
Jakub Kicinski, Norbert Slusarek, Colin Ian King,
Andra Paraschiv, kvm, virtualization, netdev, linux-kernel,
stsp2, oxffffaa
On Fri, Sep 03, 2021 at 08:55:39AM +0200, Stefano Garzarella wrote:
>On Fri, Sep 03, 2021 at 09:15:38AM +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 <arseny.krasnov@kaspersky.com>
>>---
>>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..93e8d635e18f 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)) {
^
Ooops, le32_to_cpu() should close before bitwise and operator.
I missed this, but kernel test robot discovered :-)
>>+ 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 <sgarzare@redhat.com>
NACK
Please resend fixing the issue.
Stefano
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v4 3/6] vhost/vsock: support MSG_EOR bit processing
@ 2021-09-03 12:14 ` Stefano Garzarella
0 siblings, 0 replies; 17+ messages in thread
From: Stefano Garzarella @ 2021-09-03 12:14 UTC (permalink / raw)
To: Arseny Krasnov
Cc: Andra Paraschiv, kvm, Michael S. Tsirkin, netdev, stsp2,
linux-kernel, virtualization, oxffffaa, Norbert Slusarek,
Stefan Hajnoczi, Colin Ian King, Jakub Kicinski, David S. Miller
On Fri, Sep 03, 2021 at 08:55:39AM +0200, Stefano Garzarella wrote:
>On Fri, Sep 03, 2021 at 09:15:38AM +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 <arseny.krasnov@kaspersky.com>
>>---
>>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..93e8d635e18f 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)) {
^
Ooops, le32_to_cpu() should close before bitwise and operator.
I missed this, but kernel test robot discovered :-)
>>+ 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 <sgarzare@redhat.com>
NACK
Please resend fixing the issue.
Stefano
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v4 3/6] vhost/vsock: support MSG_EOR bit processing
2021-09-03 12:14 ` Stefano Garzarella
(?)
@ 2021-09-03 12:34 ` Arseny Krasnov
-1 siblings, 0 replies; 17+ messages in thread
From: Arseny Krasnov @ 2021-09-03 12:34 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, David S. Miller,
Jakub Kicinski, Norbert Slusarek, Colin Ian King,
Andra Paraschiv, kvm, virtualization, netdev, linux-kernel,
stsp2, oxffffaa
On 03.09.2021 15:14, Stefano Garzarella wrote:
> On Fri, Sep 03, 2021 at 08:55:39AM +0200, Stefano Garzarella wrote:
>> On Fri, Sep 03, 2021 at 09:15:38AM +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 <arseny.krasnov@kaspersky.com>
>>> ---
>>> 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..93e8d635e18f 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)) {
> ^
> Ooops, le32_to_cpu() should close before bitwise and operator.
> I missed this, but kernel test robot discovered :-)
Sorry ><
>
>>> + 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 <sgarzare@redhat.com>
> NACK
>
> Please resend fixing the issue.
Done, v5 is sent
>
> Stefano
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v4 3/6] vhost/vsock: support MSG_EOR bit processing
2021-09-03 6:15 ` [PATCH net-next v4 3/6] vhost/vsock: support MSG_EOR bit processing Arseny Krasnov
@ 2021-09-03 12:05 ` kernel test robot
2021-09-03 12:05 ` kernel test robot
1 sibling, 0 replies; 17+ messages in thread
From: kernel test robot @ 2021-09-03 12:05 UTC (permalink / raw)
To: Arseny Krasnov, Stefan Hajnoczi, Stefano Garzarella,
Michael S. Tsirkin, Jason Wang, David S. Miller, Jakub Kicinski,
Norbert Slusarek, Colin Ian King
Cc: kbuild-all, netdev
[-- Attachment #1: Type: text/plain, Size: 7727 bytes --]
Hi Arseny,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Arseny-Krasnov/virtio-vsock-introduce-MSG_EOR-flag-for-SEQPACKET/20210903-141720
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 29ce8f9701072fc221d9c38ad952de1a9578f95c
config: i386-randconfig-s001-20210903 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-rc1-dirty
# https://github.com/0day-ci/linux/commit/18c4eca4204f01fb1f94bf35e760436cb537d9b3
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Arseny-Krasnov/virtio-vsock-introduce-MSG_EOR-flag-for-SEQPACKET/20210903-141720
git checkout 18c4eca4204f01fb1f94bf35e760436cb537d9b3
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/vhost/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/vhost/vsock.c:192:37: sparse: sparse: restricted __le32 degrades to integer
>> drivers/vhost/vsock.c:192:37: sparse: sparse: cast to restricted __le32
vim +192 drivers/vhost/vsock.c
89
90 static void
91 vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
92 struct vhost_virtqueue *vq)
93 {
94 struct vhost_virtqueue *tx_vq = &vsock->vqs[VSOCK_VQ_TX];
95 int pkts = 0, total_len = 0;
96 bool added = false;
97 bool restart_tx = false;
98
99 mutex_lock(&vq->mutex);
100
101 if (!vhost_vq_get_backend(vq))
102 goto out;
103
104 if (!vq_meta_prefetch(vq))
105 goto out;
106
107 /* Avoid further vmexits, we're already processing the virtqueue */
108 vhost_disable_notify(&vsock->dev, vq);
109
110 do {
111 struct virtio_vsock_pkt *pkt;
112 struct iov_iter iov_iter;
113 unsigned out, in;
114 size_t nbytes;
115 size_t iov_len, payload_len;
116 int head;
117 u32 flags_to_restore = 0;
118
119 spin_lock_bh(&vsock->send_pkt_list_lock);
120 if (list_empty(&vsock->send_pkt_list)) {
121 spin_unlock_bh(&vsock->send_pkt_list_lock);
122 vhost_enable_notify(&vsock->dev, vq);
123 break;
124 }
125
126 pkt = list_first_entry(&vsock->send_pkt_list,
127 struct virtio_vsock_pkt, list);
128 list_del_init(&pkt->list);
129 spin_unlock_bh(&vsock->send_pkt_list_lock);
130
131 head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
132 &out, &in, NULL, NULL);
133 if (head < 0) {
134 spin_lock_bh(&vsock->send_pkt_list_lock);
135 list_add(&pkt->list, &vsock->send_pkt_list);
136 spin_unlock_bh(&vsock->send_pkt_list_lock);
137 break;
138 }
139
140 if (head == vq->num) {
141 spin_lock_bh(&vsock->send_pkt_list_lock);
142 list_add(&pkt->list, &vsock->send_pkt_list);
143 spin_unlock_bh(&vsock->send_pkt_list_lock);
144
145 /* We cannot finish yet if more buffers snuck in while
146 * re-enabling notify.
147 */
148 if (unlikely(vhost_enable_notify(&vsock->dev, vq))) {
149 vhost_disable_notify(&vsock->dev, vq);
150 continue;
151 }
152 break;
153 }
154
155 if (out) {
156 virtio_transport_free_pkt(pkt);
157 vq_err(vq, "Expected 0 output buffers, got %u\n", out);
158 break;
159 }
160
161 iov_len = iov_length(&vq->iov[out], in);
162 if (iov_len < sizeof(pkt->hdr)) {
163 virtio_transport_free_pkt(pkt);
164 vq_err(vq, "Buffer len [%zu] too small\n", iov_len);
165 break;
166 }
167
168 iov_iter_init(&iov_iter, READ, &vq->iov[out], in, iov_len);
169 payload_len = pkt->len - pkt->off;
170
171 /* If the packet is greater than the space available in the
172 * buffer, we split it using multiple buffers.
173 */
174 if (payload_len > iov_len - sizeof(pkt->hdr)) {
175 payload_len = iov_len - sizeof(pkt->hdr);
176
177 /* As we are copying pieces of large packet's buffer to
178 * small rx buffers, headers of packets in rx queue are
179 * created dynamically and are initialized with header
180 * of current packet(except length). But in case of
181 * SOCK_SEQPACKET, we also must clear message delimeter
182 * bit (VIRTIO_VSOCK_SEQ_EOM) and MSG_EOR bit
183 * (VIRTIO_VSOCK_SEQ_EOR) if set. Otherwise,
184 * there will be sequence of packets with these
185 * bits set. After initialized header will be copied to
186 * rx buffer, these required bits will be restored.
187 */
188 if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOM) {
189 pkt->hdr.flags &= ~cpu_to_le32(VIRTIO_VSOCK_SEQ_EOM);
190 flags_to_restore |= VIRTIO_VSOCK_SEQ_EOM;
191
> 192 if (le32_to_cpu(pkt->hdr.flags & VIRTIO_VSOCK_SEQ_EOR)) {
193 pkt->hdr.flags &= ~cpu_to_le32(VIRTIO_VSOCK_SEQ_EOR);
194 flags_to_restore |= VIRTIO_VSOCK_SEQ_EOR;
195 }
196 }
197 }
198
199 /* Set the correct length in the header */
200 pkt->hdr.len = cpu_to_le32(payload_len);
201
202 nbytes = copy_to_iter(&pkt->hdr, sizeof(pkt->hdr), &iov_iter);
203 if (nbytes != sizeof(pkt->hdr)) {
204 virtio_transport_free_pkt(pkt);
205 vq_err(vq, "Faulted on copying pkt hdr\n");
206 break;
207 }
208
209 nbytes = copy_to_iter(pkt->buf + pkt->off, payload_len,
210 &iov_iter);
211 if (nbytes != payload_len) {
212 virtio_transport_free_pkt(pkt);
213 vq_err(vq, "Faulted on copying pkt buf\n");
214 break;
215 }
216
217 /* Deliver to monitoring devices all packets that we
218 * will transmit.
219 */
220 virtio_transport_deliver_tap_pkt(pkt);
221
222 vhost_add_used(vq, head, sizeof(pkt->hdr) + payload_len);
223 added = true;
224
225 pkt->off += payload_len;
226 total_len += payload_len;
227
228 /* If we didn't send all the payload we can requeue the packet
229 * to send it with the next available buffer.
230 */
231 if (pkt->off < pkt->len) {
232 pkt->hdr.flags |= cpu_to_le32(flags_to_restore);
233
234 /* We are queueing the same virtio_vsock_pkt to handle
235 * the remaining bytes, and we want to deliver it
236 * to monitoring devices in the next iteration.
237 */
238 pkt->tap_delivered = false;
239
240 spin_lock_bh(&vsock->send_pkt_list_lock);
241 list_add(&pkt->list, &vsock->send_pkt_list);
242 spin_unlock_bh(&vsock->send_pkt_list_lock);
243 } else {
244 if (pkt->reply) {
245 int val;
246
247 val = atomic_dec_return(&vsock->queued_replies);
248
249 /* Do we have resources to resume tx
250 * processing?
251 */
252 if (val + 1 == tx_vq->num)
253 restart_tx = true;
254 }
255
256 virtio_transport_free_pkt(pkt);
257 }
258 } while(likely(!vhost_exceeds_weight(vq, ++pkts, total_len)));
259 if (added)
260 vhost_signal(&vsock->dev, vq);
261
262 out:
263 mutex_unlock(&vq->mutex);
264
265 if (restart_tx)
266 vhost_poll_queue(&tx_vq->poll);
267 }
268
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33807 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next v4 3/6] vhost/vsock: support MSG_EOR bit processing
@ 2021-09-03 12:05 ` kernel test robot
0 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2021-09-03 12:05 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 7943 bytes --]
Hi Arseny,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Arseny-Krasnov/virtio-vsock-introduce-MSG_EOR-flag-for-SEQPACKET/20210903-141720
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 29ce8f9701072fc221d9c38ad952de1a9578f95c
config: i386-randconfig-s001-20210903 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-rc1-dirty
# https://github.com/0day-ci/linux/commit/18c4eca4204f01fb1f94bf35e760436cb537d9b3
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Arseny-Krasnov/virtio-vsock-introduce-MSG_EOR-flag-for-SEQPACKET/20210903-141720
git checkout 18c4eca4204f01fb1f94bf35e760436cb537d9b3
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/vhost/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/vhost/vsock.c:192:37: sparse: sparse: restricted __le32 degrades to integer
>> drivers/vhost/vsock.c:192:37: sparse: sparse: cast to restricted __le32
vim +192 drivers/vhost/vsock.c
89
90 static void
91 vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
92 struct vhost_virtqueue *vq)
93 {
94 struct vhost_virtqueue *tx_vq = &vsock->vqs[VSOCK_VQ_TX];
95 int pkts = 0, total_len = 0;
96 bool added = false;
97 bool restart_tx = false;
98
99 mutex_lock(&vq->mutex);
100
101 if (!vhost_vq_get_backend(vq))
102 goto out;
103
104 if (!vq_meta_prefetch(vq))
105 goto out;
106
107 /* Avoid further vmexits, we're already processing the virtqueue */
108 vhost_disable_notify(&vsock->dev, vq);
109
110 do {
111 struct virtio_vsock_pkt *pkt;
112 struct iov_iter iov_iter;
113 unsigned out, in;
114 size_t nbytes;
115 size_t iov_len, payload_len;
116 int head;
117 u32 flags_to_restore = 0;
118
119 spin_lock_bh(&vsock->send_pkt_list_lock);
120 if (list_empty(&vsock->send_pkt_list)) {
121 spin_unlock_bh(&vsock->send_pkt_list_lock);
122 vhost_enable_notify(&vsock->dev, vq);
123 break;
124 }
125
126 pkt = list_first_entry(&vsock->send_pkt_list,
127 struct virtio_vsock_pkt, list);
128 list_del_init(&pkt->list);
129 spin_unlock_bh(&vsock->send_pkt_list_lock);
130
131 head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
132 &out, &in, NULL, NULL);
133 if (head < 0) {
134 spin_lock_bh(&vsock->send_pkt_list_lock);
135 list_add(&pkt->list, &vsock->send_pkt_list);
136 spin_unlock_bh(&vsock->send_pkt_list_lock);
137 break;
138 }
139
140 if (head == vq->num) {
141 spin_lock_bh(&vsock->send_pkt_list_lock);
142 list_add(&pkt->list, &vsock->send_pkt_list);
143 spin_unlock_bh(&vsock->send_pkt_list_lock);
144
145 /* We cannot finish yet if more buffers snuck in while
146 * re-enabling notify.
147 */
148 if (unlikely(vhost_enable_notify(&vsock->dev, vq))) {
149 vhost_disable_notify(&vsock->dev, vq);
150 continue;
151 }
152 break;
153 }
154
155 if (out) {
156 virtio_transport_free_pkt(pkt);
157 vq_err(vq, "Expected 0 output buffers, got %u\n", out);
158 break;
159 }
160
161 iov_len = iov_length(&vq->iov[out], in);
162 if (iov_len < sizeof(pkt->hdr)) {
163 virtio_transport_free_pkt(pkt);
164 vq_err(vq, "Buffer len [%zu] too small\n", iov_len);
165 break;
166 }
167
168 iov_iter_init(&iov_iter, READ, &vq->iov[out], in, iov_len);
169 payload_len = pkt->len - pkt->off;
170
171 /* If the packet is greater than the space available in the
172 * buffer, we split it using multiple buffers.
173 */
174 if (payload_len > iov_len - sizeof(pkt->hdr)) {
175 payload_len = iov_len - sizeof(pkt->hdr);
176
177 /* As we are copying pieces of large packet's buffer to
178 * small rx buffers, headers of packets in rx queue are
179 * created dynamically and are initialized with header
180 * of current packet(except length). But in case of
181 * SOCK_SEQPACKET, we also must clear message delimeter
182 * bit (VIRTIO_VSOCK_SEQ_EOM) and MSG_EOR bit
183 * (VIRTIO_VSOCK_SEQ_EOR) if set. Otherwise,
184 * there will be sequence of packets with these
185 * bits set. After initialized header will be copied to
186 * rx buffer, these required bits will be restored.
187 */
188 if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOM) {
189 pkt->hdr.flags &= ~cpu_to_le32(VIRTIO_VSOCK_SEQ_EOM);
190 flags_to_restore |= VIRTIO_VSOCK_SEQ_EOM;
191
> 192 if (le32_to_cpu(pkt->hdr.flags & VIRTIO_VSOCK_SEQ_EOR)) {
193 pkt->hdr.flags &= ~cpu_to_le32(VIRTIO_VSOCK_SEQ_EOR);
194 flags_to_restore |= VIRTIO_VSOCK_SEQ_EOR;
195 }
196 }
197 }
198
199 /* Set the correct length in the header */
200 pkt->hdr.len = cpu_to_le32(payload_len);
201
202 nbytes = copy_to_iter(&pkt->hdr, sizeof(pkt->hdr), &iov_iter);
203 if (nbytes != sizeof(pkt->hdr)) {
204 virtio_transport_free_pkt(pkt);
205 vq_err(vq, "Faulted on copying pkt hdr\n");
206 break;
207 }
208
209 nbytes = copy_to_iter(pkt->buf + pkt->off, payload_len,
210 &iov_iter);
211 if (nbytes != payload_len) {
212 virtio_transport_free_pkt(pkt);
213 vq_err(vq, "Faulted on copying pkt buf\n");
214 break;
215 }
216
217 /* Deliver to monitoring devices all packets that we
218 * will transmit.
219 */
220 virtio_transport_deliver_tap_pkt(pkt);
221
222 vhost_add_used(vq, head, sizeof(pkt->hdr) + payload_len);
223 added = true;
224
225 pkt->off += payload_len;
226 total_len += payload_len;
227
228 /* If we didn't send all the payload we can requeue the packet
229 * to send it with the next available buffer.
230 */
231 if (pkt->off < pkt->len) {
232 pkt->hdr.flags |= cpu_to_le32(flags_to_restore);
233
234 /* We are queueing the same virtio_vsock_pkt to handle
235 * the remaining bytes, and we want to deliver it
236 * to monitoring devices in the next iteration.
237 */
238 pkt->tap_delivered = false;
239
240 spin_lock_bh(&vsock->send_pkt_list_lock);
241 list_add(&pkt->list, &vsock->send_pkt_list);
242 spin_unlock_bh(&vsock->send_pkt_list_lock);
243 } else {
244 if (pkt->reply) {
245 int val;
246
247 val = atomic_dec_return(&vsock->queued_replies);
248
249 /* Do we have resources to resume tx
250 * processing?
251 */
252 if (val + 1 == tx_vq->num)
253 restart_tx = true;
254 }
255
256 virtio_transport_free_pkt(pkt);
257 }
258 } while(likely(!vhost_exceeds_weight(vq, ++pkts, total_len)));
259 if (added)
260 vhost_signal(&vsock->dev, vq);
261
262 out:
263 mutex_unlock(&vq->mutex);
264
265 if (restart_tx)
266 vhost_poll_queue(&tx_vq->poll);
267 }
268
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33807 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next v4 4/6] virtio/vsock: support MSG_EOR bit processing
2021-09-03 6:13 [PATCH net-next v4 0/6] virtio/vsock: introduce MSG_EOR flag for SEQPACKET Arseny Krasnov
` (2 preceding siblings ...)
2021-09-03 6:15 ` [PATCH net-next v4 3/6] vhost/vsock: support MSG_EOR bit processing Arseny Krasnov
@ 2021-09-03 6:15 ` Arseny Krasnov
2021-09-03 6:16 ` [PATCH net-next v4 5/6] af_vsock: rename variables in receive loop Arseny Krasnov
2021-09-03 6:16 ` [PATCH net-next v4 6/6] vsock_test: update message bounds test for MSG_EOR Arseny Krasnov
5 siblings, 0 replies; 17+ messages in thread
From: Arseny Krasnov @ 2021-09-03 6:15 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
If packet has 'EOR' bit - set MSG_EOR in 'recvmsg()' flags.
Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.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 related [flat|nested] 17+ messages in thread
* [PATCH net-next v4 5/6] af_vsock: rename variables in receive loop
2021-09-03 6:13 [PATCH net-next v4 0/6] virtio/vsock: introduce MSG_EOR flag for SEQPACKET Arseny Krasnov
` (3 preceding siblings ...)
2021-09-03 6:15 ` [PATCH net-next v4 4/6] virtio/vsock: " Arseny Krasnov
@ 2021-09-03 6:16 ` Arseny Krasnov
2021-09-03 6:16 ` [PATCH net-next v4 6/6] vsock_test: update message bounds test for MSG_EOR Arseny Krasnov
5 siblings, 0 replies; 17+ messages in thread
From: Arseny Krasnov @ 2021-09-03 6:16 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
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>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.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 related [flat|nested] 17+ messages in thread
* [PATCH net-next v4 6/6] vsock_test: update message bounds test for MSG_EOR
2021-09-03 6:13 [PATCH net-next v4 0/6] virtio/vsock: introduce MSG_EOR flag for SEQPACKET Arseny Krasnov
` (4 preceding siblings ...)
2021-09-03 6:16 ` [PATCH net-next v4 5/6] af_vsock: rename variables in receive loop Arseny Krasnov
@ 2021-09-03 6:16 ` Arseny Krasnov
5 siblings, 0 replies; 17+ messages in thread
From: Arseny Krasnov @ 2021-09-03 6:16 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
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>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.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 related [flat|nested] 17+ messages in thread