* Re: [PATCH net-next v4 3/6] vhost/vsock: support MSG_EOR bit processing
[not found] ` <20210903061541.3187840-1-arseny.krasnov@kaspersky.com>
@ 2021-09-03 6:55 ` Stefano Garzarella
2021-09-03 12:14 ` Stefano Garzarella
0 siblings, 1 reply; 3+ 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] 3+ messages in thread
* Re: [PATCH net-next v4 3/6] vhost/vsock: support MSG_EOR bit processing
2021-09-03 6:55 ` [PATCH net-next v4 3/6] vhost/vsock: support MSG_EOR bit processing Stefano Garzarella
@ 2021-09-03 12:14 ` Stefano Garzarella
0 siblings, 0 replies; 3+ 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] 3+ messages in thread
* Re: [PATCH net-next v4 2/6] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOR' bit.
[not found] ` <20210903061523.3187714-1-arseny.krasnov@kaspersky.com>
@ 2021-09-05 15:50 ` Michael S. Tsirkin
0 siblings, 0 replies; 3+ 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] 3+ messages in thread
end of thread, other threads:[~2021-09-05 15:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20210903061353.3187150-1-arseny.krasnov@kaspersky.com>
[not found] ` <20210903061541.3187840-1-arseny.krasnov@kaspersky.com>
2021-09-03 6:55 ` [PATCH net-next v4 3/6] vhost/vsock: support MSG_EOR bit processing Stefano Garzarella
2021-09-03 12:14 ` Stefano Garzarella
[not found] ` <20210903061523.3187714-1-arseny.krasnov@kaspersky.com>
2021-09-05 15:50 ` [PATCH net-next v4 2/6] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOR' bit Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox