On 6/23/22 19:22, Michael S. Tsirkin 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 Thu, Jun 23, 2022 at 06:36:50PM +0300, Laura Loghin wrote: >> On 6/16/22 19:40, Michael S. Tsirkin 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 Tue, Jun 14, 2022 at 01:57:34PM +0200, Halil Pasic wrote: >> >> On Mon, 13 Jun 2022 13:40:38 +0300 >> Laura Loghin 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. >> >> 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. >> >> I think I agree here, I don't understand the mix of "buffers" and "a >> packet" either. >> >> The way I was understanding that while reading the spec is that a buffer >> is corresponding to one descriptor, > what gave this impression? buffers can use any number of descriptors. > There are sections in the spec that make you think of a buffer as corresponding to a descriptor chain, and others that don't, for example: 2.6.5    The Virtqueue Descriptor Table: Each descriptor describes a buffer which is read-only for the device (“device-readable”) or write-only for the device (“device-writable”), but a chain of descriptors can contain both device-readable and device-writable buffers. I remember while reading the spec I was always confused about the buffer - descriptor (chain) relation, but in the end concluded that the buffer is the memory region to which a single descriptor is pointing to (probably also because when you think of a buffer you would at first assume it is contiguous in memory). This doesn't seem to be the right understanding, so I will update the patch. >> so a packet will correspond to >> multiple buffers (like for example in Linux one buffer for the packet >> header and one buffer for the payload). I wanted to limit the memory >> allocated by the driver for RX buffers and TX buffers, so that's why I >> used 'buffers' for RX. Does it make sense or did I misunderstand what >> was causing the confusion here? >> >> Thanks, >> Laura > > As above, a buffer can consist of many descriptors. See e.g. > Descriptor Chaining. > I think there are places in spec when say "descriptor" and we should > fix them to say one or more descriptors. It would be great if we could fix the spec, so that the relation between a descriptor and a buffer becomes more clear and doesn't leave room for interpretations. Laura >> I voted "no" on the ballot, though if others feel we should apply as >> is and fix up later, that is not too bad. >> >> >> >> 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? >> >> @Michael, Conny: What do you think? >> >> Regards, >> Halil >> >> >> >> 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. >> 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.