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

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

[Laura Loghin]

Added a new field to the vsock device config space that
is limiting the size of the packet payload. This way
the driver is not allowed to allocate huge buffers, and
potentially fill up the entire memory.
Also defined a new feature bit for this, VIRTIO_VSOCK_F_SIZE_MAX,
in order to keep backwards compatibility.

Signed-off-by: Laura Loghin <lauralg@amazon.com>
---
 virtio-vsock.tex | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/virtio-vsock.tex b/virtio-vsock.tex
index d79984d..e6903bd 100644
--- a/virtio-vsock.tex
+++ b/virtio-vsock.tex
@@ -23,6 +23,10 @@ \subsection{Feature bits}\label{sec:Device Types / Socket Device / Feature bits}
 \begin{description}
 \item[VIRTIO_VSOCK_F_STREAM (0)] stream socket type is supported.
 \item[VIRTIO_VSOCK_F_SEQPACKET (1)] seqpacket socket type is supported.
+\item[VIRTIO_VSOCK_F_SIZE_MAX (2)] Maximum size of the packet payload is in
+	\field{data_max_size}. If offered by the device, device advises driver
+	about the value of its maximum payload size. If negotiated, the driver uses
+	\field{data_max_size} as the maximum packet payload size value.
 \end{description}
 
 \subsection{Device configuration layout}\label{sec:Device Types / Socket Device / Device configuration layout}
@@ -32,6 +36,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / Socket Device
 \begin{lstlisting}
 struct virtio_vsock_config {
 	le64 guest_cid;
+	le32 data_max_size;
 };
 \end{lstlisting}
 
@@ -57,6 +62,28 @@ \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}.
+
+The device MUST forward transmitted packets of up to \field{data_max_size}
+(plus header length) size after VIRTIO_VSOCK_F_SIZE_MAX has been successfully
+negotiated.
+
+\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, it MUST supply receive
+buffers that don'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).
+
 \subsection{Device Initialization}\label{sec:Device Types / Socket Device / Device Initialization}
 
 \begin{enumerate}
-- 
2.17.1




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.


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

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

[Stefano Garzarella]

On Wed, Apr 13, 2022 at 01:01:40PM +0300, Laura Loghin wrote:
>Added a new field to the vsock device config space that
>is limiting the size of the packet payload. This way
>the driver is not allowed to allocate huge buffers, and
>potentially fill up the entire memory.
>Also defined a new feature bit for this, VIRTIO_VSOCK_F_SIZE_MAX,
>in order to keep backwards compatibility.
>
>Signed-off-by: Laura Loghin <lauralg@amazon.com>
>---
> virtio-vsock.tex | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
>diff --git a/virtio-vsock.tex b/virtio-vsock.tex
>index d79984d..e6903bd 100644
>--- a/virtio-vsock.tex
>+++ b/virtio-vsock.tex
>@@ -23,6 +23,10 @@ \subsection{Feature bits}\label{sec:Device Types / Socket Device / Feature bits}
> \begin{description}
> \item[VIRTIO_VSOCK_F_STREAM (0)] stream socket type is supported.
> \item[VIRTIO_VSOCK_F_SEQPACKET (1)] seqpacket socket type is supported.
>+\item[VIRTIO_VSOCK_F_SIZE_MAX (2)] Maximum size of the packet payload is in
>+	\field{data_max_size}. If offered by the device, device advises driver
>+	about the value of its maximum payload size. If negotiated, the driver uses
>+	\field{data_max_size} as the maximum packet payload size value.
> \end{description}
>
> \subsection{Device configuration layout}\label{sec:Device Types / Socket Device / Device configuration layout}
>@@ -32,6 +36,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / Socket Device
> \begin{lstlisting}
> struct virtio_vsock_config {
> 	le64 guest_cid;
>+	le32 data_max_size;
> };
> \end{lstlisting}
>
>@@ -57,6 +62,28 @@ \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}.
>+
>+The device MUST forward transmitted packets of up to \field{data_max_size}
>+(plus header length) size after VIRTIO_VSOCK_F_SIZE_MAX has been successfully
>+negotiated.

Sorry, I thought we were going to delete this sentence like we did the 
one above, maybe I didn't get the meaning right though.

What do you mean here by "forward"?

Thanks,
Stefano


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

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

[Laura Loghin]

On 4/15/22 16:06, Stefano Garzarella 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 Wed, Apr 13, 2022 at 01:01:40PM +0300, Laura Loghin wrote:
>> Added a new field to the vsock device config space that
>> is limiting the size of the packet payload. This way
>> the driver is not allowed to allocate huge buffers, and
>> potentially fill up the entire memory.
>> Also defined a new feature bit for this, VIRTIO_VSOCK_F_SIZE_MAX,
>> in order to keep backwards compatibility.
>>
>> Signed-off-by: Laura Loghin <lauralg@amazon.com>
>> ---
>> virtio-vsock.tex | 27 +++++++++++++++++++++++++++
>> 1 file changed, 27 insertions(+)
>>
>> diff --git a/virtio-vsock.tex b/virtio-vsock.tex
>> index d79984d..e6903bd 100644
>> --- a/virtio-vsock.tex
>> +++ b/virtio-vsock.tex
>> @@ -23,6 +23,10 @@ \subsection{Feature bits}\label{sec:Device Types / 
>> Socket Device / Feature bits}
>> \begin{description}
>> \item[VIRTIO_VSOCK_F_STREAM (0)] stream socket type is supported.
>> \item[VIRTIO_VSOCK_F_SEQPACKET (1)] seqpacket socket type is supported.
>> +\item[VIRTIO_VSOCK_F_SIZE_MAX (2)] Maximum size of the packet 
>> payload is in
>> +      \field{data_max_size}. If offered by the device, device 
>> advises driver
>> +      about the value of its maximum payload size. If negotiated, 
>> the driver uses
>> +      \field{data_max_size} as the maximum packet payload size value.
>> \end{description}
>>
>> \subsection{Device configuration layout}\label{sec:Device Types / 
>> Socket Device / Device configuration layout}
>> @@ -32,6 +36,7 @@ \subsection{Device configuration 
>> layout}\label{sec:Device Types / Socket Device
>> \begin{lstlisting}
>> struct virtio_vsock_config {
>>       le64 guest_cid;
>> +      le32 data_max_size;
>> };
>> \end{lstlisting}
>>
>> @@ -57,6 +62,28 @@ \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}.
>> +
>> +The device MUST forward transmitted packets of up to 
>> \field{data_max_size}
>> +(plus header length) size after VIRTIO_VSOCK_F_SIZE_MAX has been 
>> successfully
>> +negotiated.
>
> Sorry, I thought we were going to delete this sentence like we did the
> one above, maybe I didn't get the meaning right though.
>
> What do you mean here by "forward"?
>
> Thanks,
> Stefano
>

Sorry for the confusion. Now I got super confused as well :D. So actually
"MUST NOT pass received packets" was about the packets received from the 
driver,
and "MUST forward transmitted packets" about the packets transmitted 
from the
backend to the driver. The "receive/transmit" terminology was from the 
device
perspective here. So your initial comment applied to this sentence 
actually, not the
initial one, but I forgot then that I wrote this section from the device 
perspective.
Both sentences are already implied by their corresponding ones from the 
driver
section, so I think I can delete both actually.
Thanks for raising this!

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.

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

[Laura Loghin]

On 4/15/22 18:23, Laura Loghin wrote:
> On 4/15/22 16:06, Stefano Garzarella 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 Wed, Apr 13, 2022 at 01:01:40PM +0300, Laura Loghin wrote:
>>> Added a new field to the vsock device config space that
>>> is limiting the size of the packet payload. This way
>>> the driver is not allowed to allocate huge buffers, and
>>> potentially fill up the entire memory.
>>> Also defined a new feature bit for this, VIRTIO_VSOCK_F_SIZE_MAX,
>>> in order to keep backwards compatibility.
>>>
>>> Signed-off-by: Laura Loghin <lauralg@amazon.com>
>>> ---
>>> virtio-vsock.tex | 27 +++++++++++++++++++++++++++
>>> 1 file changed, 27 insertions(+)
>>>
>>> diff --git a/virtio-vsock.tex b/virtio-vsock.tex
>>> index d79984d..e6903bd 100644
>>> --- a/virtio-vsock.tex
>>> +++ b/virtio-vsock.tex
>>> @@ -23,6 +23,10 @@ \subsection{Feature bits}\label{sec:Device Types 
>>> / Socket Device / Feature bits}
>>> \begin{description}
>>> \item[VIRTIO_VSOCK_F_STREAM (0)] stream socket type is supported.
>>> \item[VIRTIO_VSOCK_F_SEQPACKET (1)] seqpacket socket type is supported.
>>> +\item[VIRTIO_VSOCK_F_SIZE_MAX (2)] Maximum size of the packet 
>>> payload is in
>>> +      \field{data_max_size}. If offered by the device, device 
>>> advises driver
>>> +      about the value of its maximum payload size. If negotiated, 
>>> the driver uses
>>> +      \field{data_max_size} as the maximum packet payload size value.
>>> \end{description}
>>>
>>> \subsection{Device configuration layout}\label{sec:Device Types / 
>>> Socket Device / Device configuration layout}
>>> @@ -32,6 +36,7 @@ \subsection{Device configuration 
>>> layout}\label{sec:Device Types / Socket Device
>>> \begin{lstlisting}
>>> struct virtio_vsock_config {
>>>       le64 guest_cid;
>>> +      le32 data_max_size;
>>> };
>>> \end{lstlisting}
>>>
>>> @@ -57,6 +62,28 @@ \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}.
>>> +
>>> +The device MUST forward transmitted packets of up to 
>>> \field{data_max_size}
>>> +(plus header length) size after VIRTIO_VSOCK_F_SIZE_MAX has been 
>>> successfully
>>> +negotiated.
>>
>> Sorry, I thought we were going to delete this sentence like we did the
>> one above, maybe I didn't get the meaning right though.
>>
>> What do you mean here by "forward"?
>>
>> Thanks,
>> Stefano
>>
>
> Sorry for the confusion. Now I got super confused as well :D. So actually
> "MUST NOT pass received packets" was about the packets received from 
> the driver,
> and "MUST forward transmitted packets" about the packets transmitted 
> from the
> backend to the driver. The "receive/transmit" terminology was from the 
> device
> perspective here. So your initial comment applied to this sentence 
> actually, not the
> initial one, but I forgot then that I wrote this section from the 
> device perspective.
> Both sentences are already implied by their corresponding ones from 
> the driver
> section, so I think I can delete both actually.
> Thanks for raising this!
>
> Laura

Hi,


Both sentences are removed now, and I also updated a bit the second
requirement from the driver section. Before it was a bit unclear if
data_max_size is the limit for each buffer or for their sum (for
implementations that allow more than 2 buffers for a single packet).
I (hopefully) made it clearer in
https://lists.oasis-open.org/archives/virtio-comment/202204/msg00089.html.
Please, have a look!

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.





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.