[virtio-dev] Re: [PATCH] virtio-vsock: add VIRTIO_VSOCK_F_DGRAM feature bit

Stefano Garzarella
2023-09-01T12:45:22Z

On Tue, Aug 29, 2023 at 09:29:45PM +0000, Bobby Eshleman wrote:
>This adds support for datagrams to the virtio-vsock device.
>
>virtio-vsock already supports stream and seqpacket types. The existing
>message types and header fields are extended to support datagrams.
>Semantic differences between the flow types are stated, as well as any
>additional requirements for devices and drivers implementing this
>feature.
>
>Signed-off-by: Bobby Eshleman <bobby.eshleman@bytedance.com>
>---
> device-types/vsock/description.tex | 95 +++++++++++++++++++++++++++---
> 1 file changed, 88 insertions(+), 7 deletions(-)
>
>diff --git a/device-types/vsock/description.tex b/device-types/vsock/description.tex
>index 7d91d159872f..638dca8e5da1 100644
>--- a/device-types/vsock/description.tex
>+++ b/device-types/vsock/description.tex
>@@ -20,6 +20,7 @@ \subsection{Feature bits}\label{sec:Device Types / Socket Device / Feature bits}
> \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.
>+\item[VIRTIO_VSOCK_F_DGRAM (3)] datagram socket type is supported.
> \end{description}
>
> \drivernormative{\subsubsection}{Feature bits}{Device Types / Socket Device / Feature bits}
>@@ -167,17 +168,22 @@ \subsubsection{Addressing}\label{sec:Device Types / Socket Device / Device Opera
> consists of a (cid, port number) tuple. The header fields used for this are
> \field{src_cid}, \field{src_port}, \field{dst_cid}, and \field{dst_port}.
>
>-Currently stream and seqpacket sockets are supported. \field{type} is 1 (VIRTIO_VSOCK_TYPE_STREAM)
>-for stream socket types, and 2 (VIRTIO_VSOCK_TYPE_SEQPACKET) for seqpacket socket types.
>+
>+Currently stream, seqpacket, and datagram sockets are supported. \field{type} is
>+1 (VIRTIO_VSOCK_TYPE_STREAM) for stream socket types, 2 (VIRTIO_VSOCK_TYPE_SEQPACKET) for
>+seqpacket socket types, and 3 (VIRTIO_VSOCK_TYPE_DGRAM) for datagram socket types.
>
> \begin{lstlisting}
> #define VIRTIO_VSOCK_TYPE_STREAM 1
> #define VIRTIO_VSOCK_TYPE_SEQPACKET 2
>+#define VIRTIO_VSOCK_TYPE_DGRAM 3
> \end{lstlisting}
>
> Stream sockets provide in-order, guaranteed, connection-oriented delivery
> without message boundaries. Seqpacket sockets provide in-order, guaranteed,
>-connection-oriented delivery with message and record boundaries.
>+connection-oriented delivery with message and record boundaries. Datagram
>+sockets provide connection-less, best-effort delivery of messages, with no
>+order or reliability guarantees.
>
> \subsubsection{Buffer Space Management}\label{sec:Device Types / Socket Device / Device Operation / Buffer Space Management}
> \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of
>@@ -203,16 +209,19 @@ \subsubsection{Buffer Space Management}\label{sec:Device Types / Socket Device /
> previously receiving a VIRTIO_VSOCK_OP_CREDIT_REQUEST packet. This allows
> communicating updates any time a change in buffer space occurs.
>
>+\field{buf_alloc} and \field{fwd_cnt} are reserved for future use by datagram
>+sockets. These fields are not used for datagram buffer space management.
>+
> \drivernormative{\paragraph}{Device Operation: Buffer Space Management}{Device Types / Socket Device / Device Operation / Buffer Space Management}
>-VIRTIO_VSOCK_OP_RW data packets MUST only be transmitted when the peer has
>-sufficient free buffer space for the payload.
>+For stream and seqpacket flows, VIRTIO_VSOCK_OP_RW data packets MUST only be
>+transmitted when the peer has sufficient free buffer space for the payload.
>
> All packets associated with a stream flow MUST contain valid information in
> \field{buf_alloc} and \field{fwd_cnt} fields.
>
> \devicenormative{\paragraph}{Device Operation: Buffer Space
> Management}{Device Types / Socket Device / Device Operation / Buffer
> Space Management}
>-VIRTIO_VSOCK_OP_RW data packets MUST only be transmitted when the peer has
>-sufficient free buffer space for the payload.
>+For stream and seqpacket flows, VIRTIO_VSOCK_OP_RW data packets MUST only be
>+transmitted when the peer has sufficient free buffer space for the payload.
>
> All packets associated with a stream flow MUST contain valid information in
> \field{buf_alloc} and \field{fwd_cnt} fields.
>@@ -299,6 +308,78 @@ \subsubsection{Seqpacket Sockets}\label{sec:Device Types / Socket Device / Devic
> #define VIRTIO_VSOCK_SEQ_EOR (1 << 1)
> \end{lstlisting}
>
>+\subsubsection{Datagram Sockets}\label{sec:Device Types / Socket Device / Device Operation / Datagram Sockets}
>+
>+\drivernormative{\paragraph}{Device Operation: Packet Fragmentation}{Device Types / Socket Device / Datagram Sockets / Fragmentation}
>+
>+Drivers MAY disassemble packets into smaller fragments. If drivers fragment a
>+packet, they MUST follow the fragmentation rules described in section
>+\ref{sec:Device Types / Socket Device / Device Operation / Datagram Sockets / Fragmentation}.
>+
>+Drivers MUST support assembly of received packet fragments according to the
>+fragmentation rules described in section
>+\ref{sec:Device Types / Socket Device / Device Operation / Datagram
>Sockets / Fragmentation}.
>+
>+\devicenormative{\paragraph}{Device Operation: Packet Fragmentation}{Device Types / Socket Device / Datagram Sockets / Fragmentation}
>+
>+Devices MAY disassemble packets into smaller fragments. If devices fragment a
>+packet, they MUST follow the fragmentation rules described in section
>+\ref{sec:Device Types / Socket Device / Device Operation / Datagram Sockets / Fragmentation}.
>+
>+Devices MUST support assembly of received packet fragments according to the
>+fragmentation rules described in section
>+\ref{sec:Device Types / Socket Device / Device Operation / Datagram Sockets / Fragmentation}.
>+
>+\drivernormative{\paragraph}{Device Operation: Packet Dropping}{Device Types / Socket Device / Datagram Sockets / Dropping}
>+
>+The driver MAY drop received packets with no notification to the device. This
>+can happen if, for example, there are insufficient resources or no socket
>+exists for the destination address.
>+
>+\devicenormative{\paragraph}{Device Operation: Packet Dropping}{Device Types / Socket Device / Datagram Sockets / Dropping}
>+
>+The device MAY drop received packets with no notification to the driver. This
>+can happen if, for example, there are insufficient resources or no socket
>+exists for the destination address.

Should we provide some notification if the socket does not exist at the
destination?

>+
>+\paragraph{Datagram Fragmentation}\label{sec:Device Types / Socket Device / Device Operation / Datagram Sockets / Fragmentation}
>+
>+\field{flags} may have the following bit set:
>+
>+\begin{lstlisting}
>+#define VIRTIO_VSOCK_DGRAM_EOM (1 << 0)
>+\end{lstlisting}
>+
>+When the header \field{flags} field bit VIRTIO_VSOCK_DGRAM_EOM (bit 0) is set,
>+it indicates that the current payload is the end of a datagram
>fragment OR that
>+the current payload is an entire datagram packet.

In the destination, if we discard some fragments, then could we
reconstruct a different datagram from the one sent?

Is that anything acceptable?

Thanks,
Stefano

>+
>+Datagram fragmentation is subject to the following rules:
>+
>+The fragments for a datagram packet MUST be added to the virtqueue in
>+sequential order.
>+
>+If a packet is not a fragment, then the VIRTIO_VSOCK_DGRAM_EOM bit (bit 0) of
>+\field{flags} MUST be set.
>+
>+If a packet is the last fragment of a fragment sequence, then the
>+VIRTIO_VSOCK_DGRAM_EOM bit (bit 0) of \field{flags} MUST be set.
>+
>+If a packet is any fragment except the last fragment of a fragment sequence,
>+then the VIRTIO_VSOCK_DGRAM_EOM bit (bit 0) of \field{flags} MUST NOT be set.
>+
>+If a driver or device has already placed a fragment of a packet on the
>+virtqueue, it MUST add the remaining fragments of the packet to the virtqueue
>+before adding any additional packets or fragments to the virtqueue.
>+
>+If fragments for a packet are only partially received after an
>+implementation-specific amount of time, then the destination device or driver
>+MAY drop the fragments.
>+
>+Each buffer containing a fragment MUST begin with a valid struct
>+virtio_vsock_hdr. \field{len} MUST equal the length of the fragment payload
>+only.
>+
> \subsubsection{Device Events}\label{sec:Device Types / Socket Device / Device Operation / Device Events}
>
> Certain events are communicated by the device to the driver using the event
>--
>2.20.1