Re: [virtio-comment] Re: [MASSMAIL KLMS] Re: [virtio-comment] [RFC PATCH v4 2/2] virtio-vsock: SOCK_SEQPACKET description

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3437 bytes --]

On Tue, Mar 30, 2021 at 09:15:39AM +0300, Arseny Krasnov wrote:
> On 30.03.2021 00:28, Stefano Garzarella wrote:
> > On Mon, Mar 29, 2021 at 08:33:27PM +0300, Arseny Krasnov wrote:
> >> On 29.03.2021 19:11, Stefan Hajnoczi wrote:
> >>> On Fri, Mar 26, 2021 at 12:02:50PM +0300, Arseny Krasnov wrote:
> >>>> @@ -98,6 +102,10 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op
> >>>>  #define VIRTIO_VSOCK_OP_CREDIT_UPDATE  6
> >>>>  /* Request the peer to send the credit info to us */
> >>>>  #define VIRTIO_VSOCK_OP_CREDIT_REQUEST 7
> >>>> +/* Message begin for SOCK_SEQPACKET */
> >>>> +#define VIRTIO_VSOCK_OP_SEQ_BEGIN      8
> >>>> +/* Message end for SOCK_SEQPACKET */
> >>>> +#define VIRTIO_VSOCK_OP_SEQ_END        9
> >>> The struct virtio_vsock_hdr->flags field is le32 and currently unused.
> >>> Could 24 bits be used for a unique message id and 8 bits for flags? 1
> >>> flag bit could be used for end-of-message and the remaining 7 bits could
> >>> be reserved. That way SEQ_BEGIN and SEQ_END are not necessary.  
> >>> Pressure
> >>> on the virtqueue would be reduced and performance should be comparable
> >>> to SOCK_STREAM.
> >> Well, my first versions of SOCK_SEQPACKET implementation, worked
> >> something like this: i used flags field of header as length of whole
> >> message. I discussed it with Stefano Garzarella, and he told that it 
> >> will
> >> be better to use special "header" in packet's payload, to keep some
> >> SOCK_SEQPACKET specific data, instead of reusing packet's header
> >> fields.
> > IIRC in the first implementation SEQ_BEGIN was an empty message and we 
> > didn't added the msg_id yet. So since we needed to carry both id and 
> > total length, I suggested to use the payload to put these extra 
> > information.
> >
> > IIUC what Stefan is suggesting is a bit different and I think it should 
> > be cool to implement: we can remove the boundary packets, use only 8 
> > bits for the flags, and add a new field to reuse the 24 unused bits, 
> > maybe also 16 bits would be enough.
> > At that point we will only use the EOR flag to know the last packet.
> >
> > The main difference will be that the receiver will know the total size 
> > only when the last packet is received.
> >
> > Do you see any issue on that approach?
> 
> It will work, except we can't check that any packet of message,
> 
> except last(with EOR bit) was dropped, since receiver don't know
> 
> real length of message. If it is ok, then i can implement it.

The credit mechanism ensures that packets are not dropped, so it's not
necessary to check for this condition.

By the way, is a unique message ID needed? My understanding is:

If two messages are being sent on a socket at the same time either their
order is serialized (whichever message began first) or it is undefined
(whichever message completes first). I wonder if POSIX specifies this
and if Linux implements it (e.g. with AF_UNIX SOCK_SEQPACKET messages
that are multiple pages long and exceed sndbuf)?

Depending on these semantics maybe we don't need a unique message ID.
Instead the driver transmits messages sequentially. RW packets for
different messages on the same socket will never be interleaved.
Therefore the unique message ID is not needed and just the MSG_EOR flag
is enough to indicate message boundaries.

What do you think?

Stefan

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization