On Tue, Mar 30, 2021 at 12:50:06PM +0300, Arseny Krasnov wrote:
>
> On 30.03.2021 11:55, Stefan Hajnoczi wrote:
> > 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).
>
> If we are talking about case, when two threads writes to one socket at the
> same time,
>
> in Linux it is possible that two message will interleave(for vsock). But as i
> know, for example
>
> when TCP socket is used, both 'write()' calls will be serialized. May be it
> is bug of vsock: when
>
> first writer goes out of space, it will sleep. Then second writer tries to
> send something, but
>
> as free space is over, it will sleep too. Then, credit update is received
> from peer. Both sender's
>
> will be woken up, but sender which grab socket lock first will continue to
> send it's message.
>
> So may be we can add something like semaphore to new/vmw_vsock/af_vsock.c
> which will
>
> serialize two 'write()' calls: second sender enters 'write()' path, only when
> first left this path.
>
> My implementation doesn't care about that, because i wanted to add semaphore
> later, by another
>
> patch.Yes, that is definitely an issue that the driver needs to take care of if we don't have unique message IDs. Thanks for explaining! Stefan
signature.asc
Description: PGP signature
_______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
