Re: Re: [virtio-comment] [PATCH v2 04/11] transport-fabrics: introduce Stream Transmission

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2797 bytes --]

On Fri, Jun 02, 2023 at 10:26:48AM +0800, zhenwei pi wrote:
> On 5/31/23 23:20, Stefan Hajnoczi wrote:
> > On Thu, May 04, 2023 at 04:19:03PM +0800, zhenwei pi wrote:
> > > +                  | +------+
> > > +                  | |flags |  -> VIRTIO_OF_DESC_F_WRITE
> > > +                  | +------+
> > > +                  |
> > > + DATA             |>+------+  -> 0
> > > +                    |......|
> > > +                    +------+  -> 1
> > > +\end{lstlisting}
> > 
> > I think this is more flexible (and has more protocol overhead) than
> > necessary. When the device has used a virtqueue buffer, it indicates how
> > many bytes were used (this can be less than the totaly number of F_WRITE
> > bytes available). I don't think there is a need to communicate F_WRITE
> > descriptors, especially in the Completion. Just a Completion with a
> > 'length' field instead of an 'ndesc' field followed by data is enough.
> > 
> 
> I guest this is not enough. For example, a initiator want to read 3 desc:
> desc0[n bytes], desc1[m bytes], desc2[1 byte]. desc[2] is expected to read a
> u8 status.
> 
> the target fills desc0[n - x bytes], desc1[m - y bytes], desc2[1 byte], the
> 'length' is (n - x + m - y + 1), we should decode each descriptor and fill
> the driver buffer correctly.(otherwise, if x + y > 0, desc[2] is never
> filled)

No, the framing really doesn't matter - that's what the spec says, after
all. The framing could be [n, m, 1] like in your example or [1, 1, n-2,
m-1, 1, 1], both are valid. What matters is that the device knows at
which offset the 1-byte status field must be written.

It is the VIRTIO specification that determines how to find the offset,
not the framing of the virtqueue buffer elements. (Again, the spec
explicitly forbids depending on framing.)

In other words, the virtio-blk spec says that the status byte is the
last writeable byte and that's how the device knows the offset. The
framing doesn't matter.

> > Since VIRTIO has flexible framing
> > (https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html#x1-390004),
> > there isn't really a need to communicate the F_WRITE descriptors at all,
> > just the maximum number of used bytes that the initiator expects.
> > 
> > Can you explain why you chose to transmit F_WRITE descriptors in both
> > the Command and the Completion? Maybe I missed a reason why it's
> > important.
> 
> Just keep the flags same to the descriptor from the command, give the
> initiator a hint 'this is a read descriptor'.

Sending virtqueue element information across the wire seems inefficient
to me. I think the protocol can be optimized for stream (TCP) and keyed
(RDMA) fabrics by omitting aspects that are not strictly needed.

Stefan

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