* [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field @ 2022-06-13 10:40 Laura Loghin 2022-06-13 11:50 ` Cornelia Huck 2022-06-14 11:57 ` Halil Pasic 0 siblings, 2 replies; 21+ messages in thread From: Laura Loghin @ 2022-06-13 10:40 UTC (permalink / raw) To: virtio-comment; +Cc: 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. Fixes: https://github.com/oasis-tcs/virtio-spec/issues/140 Signed-off-by: Laura Loghin <lauralg@amazon.com> --- conformance.tex | 2 ++ virtio-vsock.tex | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/conformance.tex b/conformance.tex index 42f8537..77f7583 100644 --- a/conformance.tex +++ b/conformance.tex @@ -222,6 +222,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets} A socket driver MUST conform to the following normative statements: \begin{itemize} +\item \ref{drivernormative:Device Types / Socket Device / Device configuration layout} \item \ref{drivernormative:Device Types / Socket Device / Device Operation / Buffer Space Management} \item \ref{drivernormative:Device Types / Socket Device / Device Operation / Receive and Transmit} \item \ref{drivernormative:Device Types / Socket Device / Device Operation / Device Events} @@ -481,6 +482,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets} A socket device MUST conform to the following normative statements: \begin{itemize} +\item \ref{devicenormative:Device Types / Socket Device / Device configuration layout} \item \ref{devicenormative:Device Types / Socket Device / Device Operation / Buffer Space Management} \item \ref{devicenormative:Device Types / Socket Device / Device Operation / Receive and Transmit} \end{itemize} diff --git a/virtio-vsock.tex b/virtio-vsock.tex index d79984d..5db6110 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,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). + \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/ ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-13 10:40 [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field Laura Loghin @ 2022-06-13 11:50 ` Cornelia Huck 2022-06-13 12:51 ` Stefano Garzarella 2022-06-14 11:57 ` Halil Pasic 1 sibling, 1 reply; 21+ messages in thread From: Cornelia Huck @ 2022-06-13 11:50 UTC (permalink / raw) To: Laura Loghin, virtio-comment; +Cc: Laura Loghin, Stefano Garzarella On Mon, Jun 13 2022, Laura Loghin <lauralg@amazon.com> 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. > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/140 > > Signed-off-by: Laura Loghin <lauralg@amazon.com> > --- > conformance.tex | 2 ++ > virtio-vsock.tex | 24 ++++++++++++++++++++++++ > 2 files changed, 26 insertions(+) I think you dropped Stefano's R-b by accident -- Stefano, I guess it still holds? (I can readd when merging). Looks good to me now; if nobody else has any comments, we can start voting. > > diff --git a/conformance.tex b/conformance.tex > index 42f8537..77f7583 100644 > --- a/conformance.tex > +++ b/conformance.tex > @@ -222,6 +222,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets} > A socket driver MUST conform to the following normative statements: > > \begin{itemize} > +\item \ref{drivernormative:Device Types / Socket Device / Device configuration layout} > \item \ref{drivernormative:Device Types / Socket Device / Device Operation / Buffer Space Management} > \item \ref{drivernormative:Device Types / Socket Device / Device Operation / Receive and Transmit} > \item \ref{drivernormative:Device Types / Socket Device / Device Operation / Device Events} > @@ -481,6 +482,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets} > A socket device MUST conform to the following normative statements: > > \begin{itemize} > +\item \ref{devicenormative:Device Types / Socket Device / Device configuration layout} > \item \ref{devicenormative:Device Types / Socket Device / Device Operation / Buffer Space Management} > \item \ref{devicenormative:Device Types / Socket Device / Device Operation / Receive and Transmit} > \end{itemize} > diff --git a/virtio-vsock.tex b/virtio-vsock.tex > index d79984d..5db6110 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,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). > + > \subsection{Device Initialization}\label{sec:Device Types / Socket Device / Device Initialization} > > \begin{enumerate} 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/ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-13 11:50 ` Cornelia Huck @ 2022-06-13 12:51 ` Stefano Garzarella 0 siblings, 0 replies; 21+ messages in thread From: Stefano Garzarella @ 2022-06-13 12:51 UTC (permalink / raw) To: Cornelia Huck; +Cc: Laura Loghin, virtio-comment On Mon, Jun 13, 2022 at 01:50:41PM +0200, Cornelia Huck wrote: >On Mon, Jun 13 2022, Laura Loghin <lauralg@amazon.com> 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. >> >> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/140 >> >> Signed-off-by: Laura Loghin <lauralg@amazon.com> >> --- >> conformance.tex | 2 ++ >> virtio-vsock.tex | 24 ++++++++++++++++++++++++ >> 2 files changed, 26 insertions(+) > >I think you dropped Stefano's R-b by accident -- Stefano, I guess it >still holds? (I can readd when merging). Yep, thanks for CCing me! Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > >Looks good to me now; if nobody else has any comments, we can start voting. > >> >> diff --git a/conformance.tex b/conformance.tex >> index 42f8537..77f7583 100644 >> --- a/conformance.tex >> +++ b/conformance.tex >> @@ -222,6 +222,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets} >> A socket driver MUST conform to the following normative statements: >> >> \begin{itemize} >> +\item \ref{drivernormative:Device Types / Socket Device / Device configuration layout} >> \item \ref{drivernormative:Device Types / Socket Device / Device Operation / Buffer Space Management} >> \item \ref{drivernormative:Device Types / Socket Device / Device Operation / Receive and Transmit} >> \item \ref{drivernormative:Device Types / Socket Device / Device Operation / Device Events} >> @@ -481,6 +482,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets} >> A socket device MUST conform to the following normative statements: >> >> \begin{itemize} >> +\item \ref{devicenormative:Device Types / Socket Device / Device configuration layout} >> \item \ref{devicenormative:Device Types / Socket Device / Device Operation / Buffer Space Management} >> \item \ref{devicenormative:Device Types / Socket Device / Device Operation / Receive and Transmit} >> \end{itemize} >> diff --git a/virtio-vsock.tex b/virtio-vsock.tex >> index d79984d..5db6110 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,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). >> + >> \subsection{Device Initialization}\label{sec:Device Types / Socket Device / Device Initialization} >> >> \begin{enumerate} > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-13 10:40 [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field Laura Loghin 2022-06-13 11:50 ` Cornelia Huck @ 2022-06-14 11:57 ` Halil Pasic 2022-06-16 16:40 ` Michael S. Tsirkin 2022-06-23 15:11 ` Laura Loghin 1 sibling, 2 replies; 21+ messages in thread From: Halil Pasic @ 2022-06-14 11:57 UTC (permalink / raw) To: Laura Loghin Cc: virtio-comment, Halil Pasic, Cornelia Huck, Michael S . Tsirkin, Stefano Garzarella 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. 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. 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 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/ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-14 11:57 ` Halil Pasic @ 2022-06-16 16:40 ` Michael S. Tsirkin 2022-06-20 9:43 ` Cornelia Huck 2022-06-23 15:36 ` Laura Loghin 2022-06-23 15:11 ` Laura Loghin 1 sibling, 2 replies; 21+ messages in thread From: Michael S. Tsirkin @ 2022-06-16 16:40 UTC (permalink / raw) To: Halil Pasic Cc: Laura Loghin, virtio-comment, Cornelia Huck, Stefano Garzarella On Tue, Jun 14, 2022 at 01:57:34PM +0200, Halil Pasic wrote: > 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. > > 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. 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-16 16:40 ` Michael S. Tsirkin @ 2022-06-20 9:43 ` Cornelia Huck 2022-06-20 9:47 ` Laura Loghin 2022-06-20 10:13 ` Michael S. Tsirkin 2022-06-23 15:36 ` Laura Loghin 1 sibling, 2 replies; 21+ messages in thread From: Cornelia Huck @ 2022-06-20 9:43 UTC (permalink / raw) To: Michael S. Tsirkin, Halil Pasic Cc: Laura Loghin, virtio-comment, Stefano Garzarella On Thu, Jun 16 2022, "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Tue, Jun 14, 2022 at 01:57:34PM +0200, Halil Pasic wrote: >> 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. >> >> 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. > > I voted "no" on the ballot, though if others feel we should apply as > is and fix up later, that is not too bad. I now switched to "no" as well; it's not too bad to fix things later, but it would be good if we had a common understanding before the change goes in. 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/ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-20 9:43 ` Cornelia Huck @ 2022-06-20 9:47 ` Laura Loghin 2022-06-20 10:13 ` Michael S. Tsirkin 1 sibling, 0 replies; 21+ messages in thread From: Laura Loghin @ 2022-06-20 9:47 UTC (permalink / raw) To: Cornelia Huck, Michael S. Tsirkin, Halil Pasic Cc: virtio-comment, Stefano Garzarella [-- Attachment #1: Type: text/plain, Size: 3165 bytes --] On 6/20/22 12:43, Cornelia Huck 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 16 2022, "Michael S. Tsirkin"<mst@redhat.com> wrote: > >> On Tue, Jun 14, 2022 at 01:57:34PM +0200, Halil Pasic wrote: >>> 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. >>> >>> 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. >> >> I voted "no" on the ballot, though if others feel we should apply as >> is and fix up later, that is not too bad. > I now switched to "no" as well; it's not too bad to fix things later, > but it would be good if we had a common understanding before the change > goes in. > Hi! Sorry, I will address your comments this week. Thanks everyone for the feedback! 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. [-- Attachment #2: Type: text/html, Size: 3954 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-20 9:43 ` Cornelia Huck 2022-06-20 9:47 ` Laura Loghin @ 2022-06-20 10:13 ` Michael S. Tsirkin 2022-06-20 10:18 ` Laura Loghin 1 sibling, 1 reply; 21+ messages in thread From: Michael S. Tsirkin @ 2022-06-20 10:13 UTC (permalink / raw) To: Cornelia Huck Cc: Halil Pasic, Laura Loghin, virtio-comment, Stefano Garzarella, stefanha On Mon, Jun 20, 2022 at 11:43:57AM +0200, Cornelia Huck wrote: > On Thu, Jun 16 2022, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Tue, Jun 14, 2022 at 01:57:34PM +0200, Halil Pasic wrote: > >> 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. > >> > >> 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. > > > > I voted "no" on the ballot, though if others feel we should apply as > > is and fix up later, that is not too bad. > > I now switched to "no" as well; it's not too bad to fix things later, > but it would be good if we had a common understanding before the change > goes in. If anyone else intends to change their votes, note today is the last opportunity. Alternatively, Laura, would you consider to address the issues? If you want us to withdraw the ballot to consider the options, that is ok too. -- MST ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-20 10:13 ` Michael S. Tsirkin @ 2022-06-20 10:18 ` Laura Loghin 2022-06-20 11:20 ` Cornelia Huck 0 siblings, 1 reply; 21+ messages in thread From: Laura Loghin @ 2022-06-20 10:18 UTC (permalink / raw) To: Michael S. Tsirkin, Cornelia Huck Cc: Halil Pasic, virtio-comment, Stefano Garzarella, stefanha [-- Attachment #1: Type: text/plain, Size: 3676 bytes --] On 6/20/22 13:13, 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 Mon, Jun 20, 2022 at 11:43:57AM +0200, Cornelia Huck wrote: >> On Thu, Jun 16 2022, "Michael S. Tsirkin"<mst@redhat.com> wrote: >> >>> On Tue, Jun 14, 2022 at 01:57:34PM +0200, Halil Pasic wrote: >>>> 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. >>>> >>>> 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. >>> >>> I voted "no" on the ballot, though if others feel we should apply as >>> is and fix up later, that is not too bad. >> I now switched to "no" as well; it's not too bad to fix things later, >> but it would be good if we had a common understanding before the change >> goes in. > If anyone else intends to change their votes, note today is > the last opportunity. > > Alternatively, Laura, would you consider to address the issues? > If you want us to withdraw the ballot to consider the options, > that is ok too. > > -- > MST > Hi, I sent a message a bit earlier. I plan to address the comments this week. I'm not yet very familiar with the voting process, but if addressing the comments means withdrawing the ballot, that is fine on my side. 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. [-- Attachment #2: Type: text/html, Size: 4604 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-20 10:18 ` Laura Loghin @ 2022-06-20 11:20 ` Cornelia Huck 2022-06-20 13:07 ` Laura Loghin 0 siblings, 1 reply; 21+ messages in thread From: Cornelia Huck @ 2022-06-20 11:20 UTC (permalink / raw) To: Laura Loghin, Michael S. Tsirkin Cc: Halil Pasic, virtio-comment, Stefano Garzarella, stefanha On Mon, Jun 20 2022, Laura Loghin <lauralg@amazon.com> wrote: > On 6/20/22 13:13, Michael S. Tsirkin wrote: >> If anyone else intends to change their votes, note today is >> the last opportunity. >> >> Alternatively, Laura, would you consider to address the issues? >> If you want us to withdraw the ballot to consider the options, >> that is ok too. >> >> -- >> MST >> > Hi, > > I sent a message a bit earlier. I plan to address the comments this week. I'm not yet very familiar with the voting process, but if addressing the comments means withdrawing the ballot, that is fine on my side. Thanks! "Withdrawing the ballot" is supposed to be an active action (so it needs to be requested explicitly IMHO, so I'd prefer to have an explict statement before doing so). If you plan to send a new version to be voted upon, please just state that you'd like the ballot to be withdrawn. Thanks! 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/ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-20 11:20 ` Cornelia Huck @ 2022-06-20 13:07 ` Laura Loghin 2022-06-20 13:29 ` Cornelia Huck 0 siblings, 1 reply; 21+ messages in thread From: Laura Loghin @ 2022-06-20 13:07 UTC (permalink / raw) To: Cornelia Huck, Michael S. Tsirkin Cc: Halil Pasic, virtio-comment, Stefano Garzarella, stefanha [-- Attachment #1: Type: text/plain, Size: 1387 bytes --] On 6/20/22 14:20, Cornelia Huck 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, Jun 20 2022, Laura Loghin<lauralg@amazon.com> wrote: > >> On 6/20/22 13:13, Michael S. Tsirkin wrote: >>> If anyone else intends to change their votes, note today is >>> the last opportunity. >>> >>> Alternatively, Laura, would you consider to address the issues? >>> If you want us to withdraw the ballot to consider the options, >>> that is ok too. >>> >>> -- >>> MST >>> >> Hi, >> >> I sent a message a bit earlier. I plan to address the comments this week. I'm not yet very familiar with the voting process, but if addressing the comments means withdrawing the ballot, that is fine on my side. Thanks! > "Withdrawing the ballot" is supposed to be an active action (so it needs > to be requested explicitly IMHO, so I'd prefer to have an explict > statement before doing so). > > If you plan to send a new version to be voted upon, please just state > that you'd like the ballot to be withdrawn. Thanks! > OK, I'd like my ballot to be withdrawn. 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. [-- Attachment #2: Type: text/html, Size: 2086 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-20 13:07 ` Laura Loghin @ 2022-06-20 13:29 ` Cornelia Huck 2022-06-24 9:53 ` Laura Loghin 0 siblings, 1 reply; 21+ messages in thread From: Cornelia Huck @ 2022-06-20 13:29 UTC (permalink / raw) To: Laura Loghin, Michael S. Tsirkin Cc: Halil Pasic, virtio-comment, Stefano Garzarella, stefanha On Mon, Jun 20 2022, Laura Loghin <lauralg@amazon.com> wrote: > On 6/20/22 14:20, Cornelia Huck 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, Jun 20 2022, Laura Loghin<lauralg@amazon.com> wrote: >> >>> On 6/20/22 13:13, Michael S. Tsirkin wrote: >>>> If anyone else intends to change their votes, note today is >>>> the last opportunity. >>>> >>>> Alternatively, Laura, would you consider to address the issues? >>>> If you want us to withdraw the ballot to consider the options, >>>> that is ok too. >>>> >>>> -- >>>> MST >>>> >>> Hi, >>> >>> I sent a message a bit earlier. I plan to address the comments this week. I'm not yet very familiar with the voting process, but if addressing the comments means withdrawing the ballot, that is fine on my side. Thanks! >> "Withdrawing the ballot" is supposed to be an active action (so it needs >> to be requested explicitly IMHO, so I'd prefer to have an explict >> statement before doing so). >> >> If you plan to send a new version to be voted upon, please just state >> that you'd like the ballot to be withdrawn. Thanks! >> > OK, I'd like my ballot to be withdrawn. Thanks for the confirmation, I just did that. 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/ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-20 13:29 ` Cornelia Huck @ 2022-06-24 9:53 ` Laura Loghin 0 siblings, 0 replies; 21+ messages in thread From: Laura Loghin @ 2022-06-24 9:53 UTC (permalink / raw) To: Cornelia Huck, Michael S. Tsirkin Cc: Halil Pasic, virtio-comment, Stefano Garzarella, stefanha [-- Attachment #1: Type: text/plain, Size: 1960 bytes --] On 6/20/22 16:29, Cornelia Huck 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, Jun 20 2022, Laura Loghin<lauralg@amazon.com> wrote: > >> On 6/20/22 14:20, Cornelia Huck 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, Jun 20 2022, Laura Loghin<lauralg@amazon.com> wrote: >>> >>>> On 6/20/22 13:13, Michael S. Tsirkin wrote: >>>>> If anyone else intends to change their votes, note today is >>>>> the last opportunity. >>>>> >>>>> Alternatively, Laura, would you consider to address the issues? >>>>> If you want us to withdraw the ballot to consider the options, >>>>> that is ok too. >>>>> >>>>> -- >>>>> MST >>>>> >>>> Hi, >>>> >>>> I sent a message a bit earlier. I plan to address the comments this week. I'm not yet very familiar with the voting process, but if addressing the comments means withdrawing the ballot, that is fine on my side. Thanks! >>> "Withdrawing the ballot" is supposed to be an active action (so it needs >>> to be requested explicitly IMHO, so I'd prefer to have an explict >>> statement before doing so). >>> >>> If you plan to send a new version to be voted upon, please just state >>> that you'd like the ballot to be withdrawn. Thanks! >>> >> OK, I'd like my ballot to be withdrawn. > Thanks for the confirmation, I just did that. > Hello, I just sent a new version https://lists.oasis-open.org/archives/virtio-comment/202206/msg00088.html in which I hopefully addressed the outstanding issues. Please have another look! 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. [-- Attachment #2: Type: text/html, Size: 3073 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-16 16:40 ` Michael S. Tsirkin 2022-06-20 9:43 ` Cornelia Huck @ 2022-06-23 15:36 ` Laura Loghin 2022-06-23 16:22 ` Michael S. Tsirkin 1 sibling, 1 reply; 21+ messages in thread From: Laura Loghin @ 2022-06-23 15:36 UTC (permalink / raw) To: Michael S. Tsirkin, Halil Pasic Cc: virtio-comment, Cornelia Huck, Stefano Garzarella [-- Attachment #1: Type: text/plain, Size: 3594 bytes --] 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<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. >> >> 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, 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 > > 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. [-- Attachment #2: Type: text/html, Size: 4488 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-23 15:36 ` Laura Loghin @ 2022-06-23 16:22 ` Michael S. Tsirkin 2022-06-24 8:23 ` Laura Loghin 0 siblings, 1 reply; 21+ messages in thread From: Michael S. Tsirkin @ 2022-06-23 16:22 UTC (permalink / raw) To: Laura Loghin Cc: Halil Pasic, virtio-comment, Cornelia Huck, Stefano Garzarella 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 <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. > > 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. > 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. > > 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. > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-23 16:22 ` Michael S. Tsirkin @ 2022-06-24 8:23 ` Laura Loghin 2022-06-27 12:59 ` Halil Pasic 0 siblings, 1 reply; 21+ messages in thread From: Laura Loghin @ 2022-06-24 8:23 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Halil Pasic, virtio-comment, Cornelia Huck, Stefano Garzarella [-- Attachment #1: Type: text/plain, Size: 5903 bytes --] 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<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. >> >> 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. [-- Attachment #2: Type: text/html, Size: 6804 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-24 8:23 ` Laura Loghin @ 2022-06-27 12:59 ` Halil Pasic 2022-06-27 13:15 ` Michael S. Tsirkin 0 siblings, 1 reply; 21+ messages in thread From: Halil Pasic @ 2022-06-27 12:59 UTC (permalink / raw) To: Laura Loghin Cc: Michael S. Tsirkin, virtio-comment, Cornelia Huck, Stefano Garzarella, Halil Pasic On Fri, 24 Jun 2022 11:23:31 +0300 Laura Loghin <lauralg@amazon.com> wrote: > 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<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. > >> > >> 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). I think in the distant past I have pointed out that the usage of the word buffer in the virtio spec is somewhat confusing. But I never took the time to make a patch which attempts to set it strait. IMHO eve if we were to use "buffer" consistently in a sense that, it is a portion of the RAM which may or may not be continuous in the address space used by virtio (guest physical or DMA depending on ...) where chained descriptors describe continuous pieces of the same, we would still have the problem, that in the normal CS world AFAIK a buffer is usually a continuous piece of memory used for a certain purpose. https://en.wikipedia.org/wiki/Data_buffer Well actually our buffers are continuous in a sense that each descriptor chain defines and has an address space of its own. In a sense that we can talk about the N-th byte of the L long buffer (N < L), but that the difference between addresses of the 0-th and the N-th byte of an virtio buffer described by a descriptor chain in DMA/guest physical address space isn't guaranteed to be N. One solution would be to define the terms: virtqueue buffer: a portion of a RAM that is composed by one or more primitive buffers and a descriptor chain such that at least one descriptor in the chain is pointed to each primitive buffer primitive buffer: a byte array in RAM which is continuous in the address space used by the spec and use this terms consistently. That is never use buffer without a qualifier. BTW I don't remember if the our primitive buffers are allowed to cross page boundaries. > 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. +1 Regards, Halil > > > 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-27 12:59 ` Halil Pasic @ 2022-06-27 13:15 ` Michael S. Tsirkin 2022-06-27 13:33 ` Laura Loghin 0 siblings, 1 reply; 21+ messages in thread From: Michael S. Tsirkin @ 2022-06-27 13:15 UTC (permalink / raw) To: Halil Pasic Cc: Laura Loghin, virtio-comment, Cornelia Huck, Stefano Garzarella On Mon, Jun 27, 2022 at 02:59:07PM +0200, Halil Pasic wrote: > On Fri, 24 Jun 2022 11:23:31 +0300 > Laura Loghin <lauralg@amazon.com> wrote: > > > 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<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. > > >> > > >> 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). well we have wording like Device reports the number of bytes it has written to memory for each buffer it uses. This is referred to as ``used length''. > I think in the distant past I have pointed out that the usage of the word > buffer in the virtio spec is somewhat confusing. But I never took the > time to make a patch which attempts to set it strait. > > IMHO eve if we were to use "buffer" consistently in a sense that, it is > a portion of the RAM which may or may not be continuous in the address > space used by virtio (guest physical or DMA depending on ...) where > chained descriptors describe continuous pieces of the same, we would > still have the problem, that in the normal CS world AFAIK a buffer is > usually a continuous piece of memory used for a certain purpose. > https://en.wikipedia.org/wiki/Data_buffer > > Well actually our buffers are continuous in a sense that each descriptor > chain defines and has an address space of its own. In a sense that we > can talk about the N-th byte of the L long buffer (N < L), but that > the difference between addresses of the 0-th and the N-th byte of an > virtio buffer described by a descriptor chain in DMA/guest physical > address space isn't guaranteed to be N. > > One solution would be to define the terms: > virtqueue buffer: a portion of a RAM that is composed by one or more > primitive buffers and a descriptor chain such that at least one > descriptor in the chain is pointed to each primitive buffer > primitive buffer: a byte array in RAM which is continuous in > the address space used by the spec > and use this terms consistently. That is never use buffer without > a qualifier. I would be inclined to avoid saying primitive buffer. saying virtqueue buffer is ok if a bit verbose. Let's start by locating places in spec which say buffer when they mean descriptor though. > > BTW I don't remember if the our primitive buffers are allowed to cross > page boundaries. Spec doesn't say they shouldn't. > > 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. > > +1 > > Regards, > Halil > > > > > > > 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-27 13:15 ` Michael S. Tsirkin @ 2022-06-27 13:33 ` Laura Loghin 2022-06-27 15:20 ` Michael S. Tsirkin 0 siblings, 1 reply; 21+ messages in thread From: Laura Loghin @ 2022-06-27 13:33 UTC (permalink / raw) To: Michael S. Tsirkin, Halil Pasic Cc: virtio-comment, Cornelia Huck, Stefano Garzarella On 6/27/22 16:15, 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 Mon, Jun 27, 2022 at 02:59:07PM +0200, Halil Pasic wrote: >> On Fri, 24 Jun 2022 11:23:31 +0300 >> Laura Loghin <lauralg@amazon.com> wrote: >> >>> 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<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. >>>>> >>>>> 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). > well we have wording like > > Device reports the number of bytes it has written to memory for > each buffer it uses. This is referred to as ``used length''. That's true, there are also sections that make you think of a buffer belonging to the entire descriptor chain. Hopefully once we update the spec, the confusion will no longer be possible. >> I think in the distant past I have pointed out that the usage of the word >> buffer in the virtio spec is somewhat confusing. But I never took the >> time to make a patch which attempts to set it strait. >> >> IMHO eve if we were to use "buffer" consistently in a sense that, it is >> a portion of the RAM which may or may not be continuous in the address >> space used by virtio (guest physical or DMA depending on ...) where >> chained descriptors describe continuous pieces of the same, we would >> still have the problem, that in the normal CS world AFAIK a buffer is >> usually a continuous piece of memory used for a certain purpose. >> https://en.wikipedia.org/wiki/Data_buffer >> >> Well actually our buffers are continuous in a sense that each descriptor >> chain defines and has an address space of its own. In a sense that we >> can talk about the N-th byte of the L long buffer (N < L), but that >> the difference between addresses of the 0-th and the N-th byte of an >> virtio buffer described by a descriptor chain in DMA/guest physical >> address space isn't guaranteed to be N. >> >> One solution would be to define the terms: >> virtqueue buffer: a portion of a RAM that is composed by one or more >> primitive buffers and a descriptor chain such that at least one >> descriptor in the chain is pointed to each primitive buffer >> primitive buffer: a byte array in RAM which is continuous in >> the address space used by the spec >> and use this terms consistently. That is never use buffer without >> a qualifier. > I would be inclined to avoid saying primitive buffer. saying > virtqueue buffer is ok if a bit verbose. > Let's start by locating places in spec which say buffer > when they mean descriptor though. > > > >> BTW I don't remember if the our primitive buffers are allowed to cross >> page boundaries. > > Spec doesn't say they shouldn't. > >>> 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. >> +1 >> >> Regards, >> Halil >> >>> >>> 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. 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-27 13:33 ` Laura Loghin @ 2022-06-27 15:20 ` Michael S. Tsirkin 0 siblings, 0 replies; 21+ messages in thread From: Michael S. Tsirkin @ 2022-06-27 15:20 UTC (permalink / raw) To: Laura Loghin Cc: Halil Pasic, virtio-comment, Cornelia Huck, Stefano Garzarella On Mon, Jun 27, 2022 at 04:33:27PM +0300, Laura Loghin wrote: > > > > 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 think this should just refer to memory: Each descriptor describes memory 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 memory. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field 2022-06-14 11:57 ` Halil Pasic 2022-06-16 16:40 ` Michael S. Tsirkin @ 2022-06-23 15:11 ` Laura Loghin 1 sibling, 0 replies; 21+ messages in thread From: Laura Loghin @ 2022-06-23 15:11 UTC (permalink / raw) To: Halil Pasic Cc: virtio-comment, Cornelia Huck, Michael S . Tsirkin, Stefano Garzarella [-- Attachment #1: Type: text/plain, Size: 3745 bytes --] 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. [-- Attachment #2: Type: text/html, Size: 4719 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2022-06-27 15:20 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-13 10:40 [virtio-comment] [PATCH v4] virtio-vsock: add max payload size config field Laura Loghin 2022-06-13 11:50 ` Cornelia Huck 2022-06-13 12:51 ` Stefano Garzarella 2022-06-14 11:57 ` Halil Pasic 2022-06-16 16:40 ` Michael S. Tsirkin 2022-06-20 9:43 ` Cornelia Huck 2022-06-20 9:47 ` Laura Loghin 2022-06-20 10:13 ` Michael S. Tsirkin 2022-06-20 10:18 ` Laura Loghin 2022-06-20 11:20 ` Cornelia Huck 2022-06-20 13:07 ` Laura Loghin 2022-06-20 13:29 ` Cornelia Huck 2022-06-24 9:53 ` Laura Loghin 2022-06-23 15:36 ` Laura Loghin 2022-06-23 16:22 ` Michael S. Tsirkin 2022-06-24 8:23 ` Laura Loghin 2022-06-27 12:59 ` Halil Pasic 2022-06-27 13:15 ` Michael S. Tsirkin 2022-06-27 13:33 ` Laura Loghin 2022-06-27 15:20 ` Michael S. Tsirkin 2022-06-23 15:11 ` Laura Loghin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.