[PATCH v2] virtio-vsock: add VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature bit

[PATCH v2] virtio-vsock: add VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature bit

[Stefano Garzarella]

Initially virtio-vsock only supported the stream type, which is why
there was no feature. Later we added the seqpacket type and in the future
we may have other types (e.g. datagram).

seqpacket is an extension of stream, so it might be implied that if
seqpacket is supported, stream is too, but this might not be true for
other types.

As we discussed here [1] should be better to add a new
VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature bit to avoid this implication.

Let's also add normative sections to better define the behavior when
VIRTIO_VSOCK_F_NO_IMPLIED_STREAM is negotiated or not.

[1] http://markmail.org/message/2s3qd74drgjxkvte

Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v2:
- rewrote better normative statements [Cornelia]
- updated conformance.tex with the new sections [Cornelia]

v1: https://markmail.org/message/ybtl4trifrhjnkfe
---
 conformance.tex  |  2 ++
 virtio-vsock.tex | 28 ++++++++++++++++++++++++----
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/conformance.tex b/conformance.tex
index 663e7c3..98626ba 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -229,6 +229,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 / Feature bits}
 \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}
@@ -500,6 +501,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 / Feature bits}
 \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..b3ff45a 100644
--- a/virtio-vsock.tex
+++ b/virtio-vsock.tex
@@ -16,15 +16,35 @@ \subsection{Virtqueues}\label{sec:Device Types / Socket Device / Virtqueues}
 
 \subsection{Feature bits}\label{sec:Device Types / Socket Device / Feature bits}
 
-If no feature bit is set, only stream socket type is supported.
-If VIRTIO_VSOCK_F_SEQPACKET has been negotiated, the device MAY act
-as if VIRTIO_VSOCK_F_STREAM has also been negotiated.
-
 \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_NO_IMPLIED_STREAM (2)] stream socket type is not implied.
 \end{description}
 
+\drivernormative{\subsubsection}{Feature bits}{Device Types / Socket Device / Feature bits}
+
+The driver SHOULD accept the VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature if
+offered by the device.
+
+If no feature bit has been negotiated, the driver SHOULD act as if
+VIRTIO_VSOCK_F_STREAM has been negotiated.
+
+If VIRTIO_VSOCK_F_SEQPACKET has been negotiated, but not
+VIRTIO_VSOCK_F_NO_IMPLIED_STREAM, the driver MAY act as if
+VIRTIO_VSOCK_F_STREAM has also been negotiated.
+
+\devicenormative{\subsubsection}{Feature bits}{Device Types / Socket Device / Feature bits}
+
+The device SHOULD offer the VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature.
+
+If no feature bit has been negotiated, the device SHOULD act as if
+VIRTIO_VSOCK_F_STREAM has been negotiated.
+
+If VIRTIO_VSOCK_F_SEQPACKET has been negotiated, but not
+VIRTIO_VSOCK_F_NO_IMPLIED_STREAM, the device MAY act as if
+VIRTIO_VSOCK_F_STREAM has also been negotiated.
+
 \subsection{Device configuration layout}\label{sec:Device Types / Socket Device / Device configuration layout}
 
 Socket device configuration uses the following layout structure:
-- 
2.36.1

[virtio-comment] Re: [PATCH v2] virtio-vsock: add VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature bit

[Cornelia Huck]

On Mon, Jul 18 2022, Stefano Garzarella <sgarzare@redhat.com> wrote:

> Initially virtio-vsock only supported the stream type, which is why
> there was no feature. Later we added the seqpacket type and in the future
> we may have other types (e.g. datagram).
>
> seqpacket is an extension of stream, so it might be implied that if
> seqpacket is supported, stream is too, but this might not be true for
> other types.
>
> As we discussed here [1] should be better to add a new
> VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature bit to avoid this implication.
>
> Let's also add normative sections to better define the behavior when
> VIRTIO_VSOCK_F_NO_IMPLIED_STREAM is negotiated or not.
>
> [1] http://markmail.org/message/2s3qd74drgjxkvte
>
> Suggested-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> v2:
> - rewrote better normative statements [Cornelia]
> - updated conformance.tex with the new sections [Cornelia]
>
> v1: https://markmail.org/message/ybtl4trifrhjnkfe
> ---
>  conformance.tex  |  2 ++
>  virtio-vsock.tex | 28 ++++++++++++++++++++++++----
>  2 files changed, 26 insertions(+), 4 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>


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

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

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

Re: [External] [PATCH v2] virtio-vsock: add VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature bit

[Bobby Eshleman]

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

Hey Stefano,

Please excuse the late reply to this thread.

I am trying to think through the imagined usage of this bit. Following the
referenced threads,
I am not sure I can clearly see under what version combinations of
driver/device that the bit will be used.

Given that feature bits being zero imply stream support, and
VIRTIO_VSOCK_F_{STREAM,SEQPACKET,DGRAM}
all existing (dgram in the future), how does NO_IMPLIED_STREAM=1 differ
from STREAM=0, SEQPACKET=0,
and DGRAM=1 (for example)?

Thanks,
Bobby

PS. Sorry for the odd [External] prefix... That comes from our company
email system.

From: "Stefano Garzarella"<sgarzare@redhat.com>
Date: Mon, Jul 18, 2022, 7:27 AM
Subject: [External] [PATCH v2] virtio-vsock: add
VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature bit
To: <virtio-comment@lists.oasis-open.org>
Cc: <jiang.wang@bytedance.com>, "Stefan
Hajnoczi"<stefanha@redhat.com>, "Arseniy
Krasnov"<AVKrasnov@sberdevices.ru>, "Michael S . Tsirkin"<mst@redhat.com>, <
bobby.eshleman@bytedance.com>, "Cornelia Huck"<cohuck@redhat.com>, "Stefano
Garzarella"<sgarzare@redhat.com>
Initially virtio-vsock only supported the stream type, which is why there
was no feature. Later we added the seqpacket type and in the future we may
have other types (e.g. datagram). seqpacket is an extension of stream, so
it might be implied that if seqpacket is supported, stream is too, but this
might not be true for other types. As we discussed here [1] should be
better to add a new VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature bit to avoid
this implication. Let's also add normative sections to better define the
behavior when VIRTIO_VSOCK_F_NO_IMPLIED_STREAM is negotiated or not. [1]
http://markmail.org/message/2s3qd74drgjxkvte Suggested-by: Michael S.
Tsirkin <mst@redhat.com> Signed-off-by: Stefano Garzarella <
sgarzare@redhat.com> --- v2: - rewrote better normative statements
[Cornelia] - updated conformance.tex with the new sections [Cornelia] v1:
https://markmail.org/message/ybtl4trifrhjnkfe --- conformance.tex | 2 ++
virtio-vsock.tex | 28 ++++++++++++++++++++++++---- 2 files changed, 26
insertions(+), 4 deletions(-) diff --git a/conformance.tex
b/conformance.tex index 663e7c3..98626ba 100644 --- a/conformance.tex +++
b/conformance.tex @@ -229,6 +229,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 / Feature bits} \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} @@ -500,6 +501,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 / Feature bits} \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..b3ff45a 100644 ---
a/virtio-vsock.tex +++ b/virtio-vsock.tex @@ -16,15 +16,35 @@
\subsection{Virtqueues}\label{sec:Device Types / Socket Device /
Virtqueues} \subsection{Feature bits}\label{sec:Device Types / Socket
Device / Feature bits} -If no feature bit is set, only stream socket type
is supported. -If VIRTIO_VSOCK_F_SEQPACKET has been negotiated, the device
MAY act -as if VIRTIO_VSOCK_F_STREAM has also been negotiated. -
\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_NO_IMPLIED_STREAM (2)] stream socket type
is not implied. \end{description} +\drivernormative{\subsubsection}{Feature
bits}{Device Types / Socket Device / Feature bits} + +The driver SHOULD
accept the VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature if +offered by the
device. + +If no feature bit has been negotiated, the driver SHOULD act as
if +VIRTIO_VSOCK_F_STREAM has been negotiated. + +If
VIRTIO_VSOCK_F_SEQPACKET has been negotiated, but not
+VIRTIO_VSOCK_F_NO_IMPLIED_STREAM, the driver MAY act as if
+VIRTIO_VSOCK_F_STREAM has also been negotiated. +
+\devicenormative{\subsubsection}{Feature bits}{Device Types / Socket
Device / Feature bits} + +The device SHOULD offer the
VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature. + +If no feature bit has been
negotiated, the device SHOULD act as if +VIRTIO_VSOCK_F_STREAM has been
negotiated. + +If VIRTIO_VSOCK_F_SEQPACKET has been negotiated, but not
+VIRTIO_VSOCK_F_NO_IMPLIED_STREAM, the device MAY act as if
+VIRTIO_VSOCK_F_STREAM has also been negotiated. + \subsection{Device
configuration layout}\label{sec:Device Types / Socket Device / Device
configuration layout} Socket device configuration uses the following layout
structure: -- 2.36.1

[-- Attachment #2: Type: text/html, Size: 10218 bytes --]

Re: [External] [PATCH v2] virtio-vsock: add VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature bit

[Stefano Garzarella]

Hi Bobby,

On Mon, Jul 18, 2022 at 11:17:26AM -0700, Bobby Eshleman wrote:
>Hey Stefano,
>
>Please excuse the late reply to this thread.

No problems at all!

>
>I am trying to think through the imagined usage of this bit. Following the
>referenced threads,
>I am not sure I can clearly see under what version combinations of
>driver/device that the bit will be used.
>

The main reason is to continue to support the current implementation.

If we had F_STREAM from the beginning when we introduced virtio-vsock, 
then there would have been no need for F_NO_IMPLIED_STREAM, but 
unfortunately, that was not the case.

If we look at the current implementation in Linux (virtio-vsock and 
vhost-vsock), F_STREAM is not negotiated. We added it in the 
specification, but not yet implemented.

That's because the driver and device were designed to support only 
stream sockets, so it was implied as a feature.
Then we added seqpacket, which is somehow an extension of stream 
sockets, so we only negotiated F_SEQPACKET (unfortunately we merged the 
patch too early before we better defined F_STREAM). Leaving support for 
stream sockets implied.

At this point we realized that if we would add another type, completely 
unrelated to stream (e.g. datagram), it would be complicated to 
negotiate only datagram support without stream.

>Given that feature bits being zero imply stream support, and
>VIRTIO_VSOCK_F_{STREAM,SEQPACKET,DGRAM}
>all existing (dgram in the future), how does NO_IMPLIED_STREAM=1 differ
>from STREAM=0, SEQPACKET=0,
>and DGRAM=1 (for example)?

Unfortunately, even when the feature bits are non-zero (F_SEQPACKET 
negotiated), we still support stream sockets implicitly.

So adding F_NO_IMPLIED_STREAM, just to make sure that if F_STREAM is not 
negotiated, then really the device/driver doesn't support it.
As we wrote in the specification, all drivers and devices in the future 
should negotiate F_NO_IMPLIED_STREAM to avoid ambiguity.

It's basically a way to be compatible with the current implementation 
and have backward compatibility in the future.

Thanks,
Stefano

Re: [PATCH v2] virtio-vsock: add VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature bit

[Stefano Garzarella]

On Mon, Jul 18, 2022 at 04:26:53PM +0200, Stefano Garzarella wrote:
>Initially virtio-vsock only supported the stream type, which is why
>there was no feature. Later we added the seqpacket type and in the future
>we may have other types (e.g. datagram).
>
>seqpacket is an extension of stream, so it might be implied that if
>seqpacket is supported, stream is too, but this might not be true for
>other types.
>
>As we discussed here [1] should be better to add a new
>VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature bit to avoid this implication.
>
>Let's also add normative sections to better define the behavior when
>VIRTIO_VSOCK_F_NO_IMPLIED_STREAM is negotiated or not.
>
>[1] http://markmail.org/message/2s3qd74drgjxkvte
>

PR opened.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/142


Thanks,
Stefano

>Suggested-by: Michael S. Tsirkin <mst@redhat.com>
>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>---
>v2:
>- rewrote better normative statements [Cornelia]
>- updated conformance.tex with the new sections [Cornelia]
>
>v1: https://markmail.org/message/ybtl4trifrhjnkfe
>---
> conformance.tex  |  2 ++
> virtio-vsock.tex | 28 ++++++++++++++++++++++++----
> 2 files changed, 26 insertions(+), 4 deletions(-)
>
>diff --git a/conformance.tex b/conformance.tex
>index 663e7c3..98626ba 100644
>--- a/conformance.tex
>+++ b/conformance.tex
>@@ -229,6 +229,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 / Feature bits}
> \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}
>@@ -500,6 +501,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 / Feature bits}
> \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..b3ff45a 100644
>--- a/virtio-vsock.tex
>+++ b/virtio-vsock.tex
>@@ -16,15 +16,35 @@ \subsection{Virtqueues}\label{sec:Device Types / Socket Device / Virtqueues}
>
> \subsection{Feature bits}\label{sec:Device Types / Socket Device / Feature bits}
>
>-If no feature bit is set, only stream socket type is supported.
>-If VIRTIO_VSOCK_F_SEQPACKET has been negotiated, the device MAY act
>-as if VIRTIO_VSOCK_F_STREAM has also been negotiated.
>-
> \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_NO_IMPLIED_STREAM (2)] stream socket type is not implied.
> \end{description}
>
>+\drivernormative{\subsubsection}{Feature bits}{Device Types / Socket Device / Feature bits}
>+
>+The driver SHOULD accept the VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature if
>+offered by the device.
>+
>+If no feature bit has been negotiated, the driver SHOULD act as if
>+VIRTIO_VSOCK_F_STREAM has been negotiated.
>+
>+If VIRTIO_VSOCK_F_SEQPACKET has been negotiated, but not
>+VIRTIO_VSOCK_F_NO_IMPLIED_STREAM, the driver MAY act as if
>+VIRTIO_VSOCK_F_STREAM has also been negotiated.
>+
>+\devicenormative{\subsubsection}{Feature bits}{Device Types / Socket Device / Feature bits}
>+
>+The device SHOULD offer the VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature.
>+
>+If no feature bit has been negotiated, the device SHOULD act as if
>+VIRTIO_VSOCK_F_STREAM has been negotiated.
>+
>+If VIRTIO_VSOCK_F_SEQPACKET has been negotiated, but not
>+VIRTIO_VSOCK_F_NO_IMPLIED_STREAM, the device MAY act as if
>+VIRTIO_VSOCK_F_STREAM has also been negotiated.
>+
> \subsection{Device configuration layout}\label{sec:Device Types / Socket Device / Device configuration layout}
>
> Socket device configuration uses the following layout structure:
>-- 
>2.36.1
>

Re: [PATCH v2] virtio-vsock: add VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature bit

[Michael S. Tsirkin]

On Mon, Jul 18, 2022 at 04:26:53PM +0200, Stefano Garzarella wrote:
> Initially virtio-vsock only supported the stream type, which is why
> there was no feature. Later we added the seqpacket type and in the future
> we may have other types (e.g. datagram).
> 
> seqpacket is an extension of stream, so it might be implied that if
> seqpacket is supported, stream is too, but this might not be true for
> other types.
> 
> As we discussed here [1] should be better to add a new
> VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature bit to avoid this implication.
> 
> Let's also add normative sections to better define the behavior when
> VIRTIO_VSOCK_F_NO_IMPLIED_STREAM is negotiated or not.
> 
> [1] http://markmail.org/message/2s3qd74drgjxkvte
> 
> Suggested-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
> v2:
> - rewrote better normative statements [Cornelia]
> - updated conformance.tex with the new sections [Cornelia]
> 
> v1: https://markmail.org/message/ybtl4trifrhjnkfe
> ---
>  conformance.tex  |  2 ++
>  virtio-vsock.tex | 28 ++++++++++++++++++++++++----
>  2 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/conformance.tex b/conformance.tex
> index 663e7c3..98626ba 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -229,6 +229,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 / Feature bits}
>  \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}
> @@ -500,6 +501,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 / Feature bits}
>  \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..b3ff45a 100644
> --- a/virtio-vsock.tex
> +++ b/virtio-vsock.tex
> @@ -16,15 +16,35 @@ \subsection{Virtqueues}\label{sec:Device Types / Socket Device / Virtqueues}
>  
>  \subsection{Feature bits}\label{sec:Device Types / Socket Device / Feature bits}
>  
> -If no feature bit is set, only stream socket type is supported.
> -If VIRTIO_VSOCK_F_SEQPACKET has been negotiated, the device MAY act
> -as if VIRTIO_VSOCK_F_STREAM has also been negotiated.
> -
>  \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_NO_IMPLIED_STREAM (2)] stream socket type is not implied.
>  \end{description}
>  
> +\drivernormative{\subsubsection}{Feature bits}{Device Types / Socket Device / Feature bits}
> +
> +The driver SHOULD accept the VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature if
> +offered by the device.
> +
> +If no feature bit has been negotiated, the driver SHOULD act as if
> +VIRTIO_VSOCK_F_STREAM has been negotiated.
> +
> +If VIRTIO_VSOCK_F_SEQPACKET has been negotiated, but not
> +VIRTIO_VSOCK_F_NO_IMPLIED_STREAM, the driver MAY act as if
> +VIRTIO_VSOCK_F_STREAM has also been negotiated.
> +
> +\devicenormative{\subsubsection}{Feature bits}{Device Types / Socket Device / Feature bits}
> +
> +The device SHOULD offer the VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature.
> +
> +If no feature bit has been negotiated, the device SHOULD act as if
> +VIRTIO_VSOCK_F_STREAM has been negotiated.
> +
> +If VIRTIO_VSOCK_F_SEQPACKET has been negotiated, but not
> +VIRTIO_VSOCK_F_NO_IMPLIED_STREAM, the device MAY act as if
> +VIRTIO_VSOCK_F_STREAM has also been negotiated.
> +
>  \subsection{Device configuration layout}\label{sec:Device Types / Socket Device / Device configuration layout}
>  
>  Socket device configuration uses the following layout structure:
> -- 
> 2.36.1

Re: [External] [PATCH v2] virtio-vsock: add VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature bit

[Bobby Eshleman]

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

I see, thank you for the clarification. Sorry for the late response, I just
noticed this ACK was left in my draft box.

Thanks again,
Bobby

From: "Stefano Garzarella"<sgarzare@redhat.com>
Date: Tue, Jul 19, 2022, 1:04 AM
Subject: Re: [External] [PATCH v2] virtio-vsock: add
VIRTIO_VSOCK_F_NO_IMPLIED_STREAM feature bit
To: "Bobby Eshleman"<bobby.eshleman@bytedance.com>
Cc: <virtio-comment@lists.oasis-open.org>, <jiang.wang@bytedance.com>, "Stefan
Hajnoczi"<stefanha@redhat.com>, "Arseniy Krasnov"<AVKrasnov@sberdevices.ru>,
"Michael S . Tsirkin"<mst@redhat.com>, "Cornelia Huck"<cohuck@redhat.com>
Hi Bobby, On Mon, Jul 18, 2022 at 11:17:26AM -0700, Bobby Eshleman wrote:
>Hey Stefano, > >Please excuse the late reply to this thread. No problems
at all! > >I am trying to think through the imagined usage of this bit.
Following the >referenced threads, >I am not sure I can clearly see under
what version combinations of >driver/device that the bit will be used. >
The main reason is to continue to support the current implementation. If we
had F_STREAM from the beginning when we introduced virtio-vsock, then there
would have been no need for F_NO_IMPLIED_STREAM, but unfortunately, that
was not the case. If we look at the current implementation in Linux
(virtio-vsock and vhost-vsock), F_STREAM is not negotiated. We added it in
the specification, but not yet implemented. That's because the driver and
device were designed to support only stream sockets, so it was implied as a
feature. Then we added seqpacket, which is somehow an extension of stream
sockets, so we only negotiated F_SEQPACKET (unfortunately we merged the
patch too early before we better defined F_STREAM). Leaving support for
stream sockets implied. At this point we realized that if we would add
another type, completely unrelated to stream (e.g. datagram), it would be
complicated to negotiate only datagram support without stream. >Given that
feature bits being zero imply stream support, and
>VIRTIO_VSOCK_F_{STREAM,SEQPACKET,DGRAM} >all existing (dgram in the
future), how does NO_IMPLIED_STREAM=1 differ >from STREAM=0, SEQPACKET=0,
>and DGRAM=1 (for example)? Unfortunately, even when the feature bits are
non-zero (F_SEQPACKET negotiated), we still support stream sockets
implicitly. So adding F_NO_IMPLIED_STREAM, just to make sure that if
F_STREAM is not negotiated, then really the device/driver doesn't support
it. As we wrote in the specification, all drivers and devices in the future
should negotiate F_NO_IMPLIED_STREAM to avoid ambiguity. It's basically a
way to be compatible with the current implementation and have backward
compatibility in the future. Thanks, Stefano

[-- Attachment #2: Type: text/html, Size: 6350 bytes --]