Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field

[Laura Loghin <lauralg@amazon.com>]

On 6/14/22 14:57, Halil Pasic wrote:

> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> On Mon, 13 Jun 2022 13:40:38 +0300
> Laura Loghin <lauralg@amazon.com> wrote:
>
> > @@ -57,6 +62,25 @@ \subsection{Device configuration layout}\label{sec:Device Types / Socket Device
> >  \hline
> >  \end{tabular}
> >
> > +The following driver-read-only field, \field{data_max_size} only exists if
> > +VIRTIO_VSOCK_F_SIZE_MAX is set. This field specifies the maximum packet payload
> > +size for the driver to use.
> > +
> > +\devicenormative{\subsubsection}{Device configuration layout}{Device Types / Socket Device / Device configuration layout}
> > +
> > +The device MUST NOT change the value exposed through \field{data_max_size}.
> > +
> > +\drivernormative{\subsubsection}{Device configuration layout}{Device Types / Socket Device / Device configuration layout}
> > +
> > +A driver SHOULD negotiate VIRTIO_VSOCK_F_SIZE_MAX if the device offers it.
> > +
> > +If the driver negotiates VIRTIO_VSOCK_F_SIZE_MAX, the receive buffers it
> > +supplies for a packet MUST have a total size that doesn't exceed the size
> > +\field{data_max_size} (plus header length).
> > +
> > +If the driver negotiates VIRTIO_VSOCK_F_SIZE_MAX, it MUST  NOT transmit packets
> > +of size exceeding the value of \field{data_max_size} (plus header length).
> > +
> >       
> Hi and sorry for being late to the party!
>
> I believe I do understand why do we want to put a restriction on the
> size of the transmitted packets, but I would appreciate if you could
> explain to me why do we want to limit the receive buffer size.
>     
My initial proposal was having a limit only for the size of the TX
packets, but Stefano suggested to add it for RX as well, and I found
that okay, since both the RX and TX buffers are provided by the driver,
so they are untrusted. This way we would limit the memory allocated for
both types of buffers, and consequently for the packets as well. 
> Also I find the wording regarding a little bit ambiguous because
> in a networking context it also makes sense to talk about the size of the
> receive buffer. I guess hear we are talking about a single virtio buffer
> (a descriptor chain described potentially non-continuous (or compact in
> the mathematical sense of the word) which is composed from as many
> continuous chunks of memory as many descriptors are contained within the
> descriptor chain). If we are indeed talking about a single virtio buffer,
> I don't understand the plural. If not, I'm not sure what are we talking
> about.
>     
Hmm so my understanding when reading the spec was that a virtio buffer is
corresponding to a single descriptor (not to a descriptor chain).
Did I misunderstand it the whole time? :(
> Also, do we have some sort of packets may not cross virtio buffer
> boundaries, or even a single packet per single viritio buffer rule for
> vsock. I did a quick search and could not find any. Did I overlook
> something? Should we spell this out?
>     
I think the rule would be the same as for the other devices, at least
the ones that I know some things about, so I might be wrong with this
assumption: one message or transmission unit (not sure what would be
the best word here). e.g. request for block, packet for net/vsock
is corresponding to exactly one descriptor chain (so one virtio buffer
if you consider a buffer as being a descriptor chain).
> @Michael, Conny: What do you think?
>
> Regards,
> Halil
>     
Thanks,
Laura

Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.