On Thu, Jun 10, 2021 at 06:12:03PM +0000, Jiang Wang wrote: > Add supports for datagram type for virtio-vsock. Datagram > sockets are connectionless and unreliable. To avoid contention > with stream and other sockets, add two more virtqueues and > a new feature bit to identify if those two new queues exist or not. > > Also add descriptions for resource management of datagram, which > does not use the existing credit update mechanism associated with > stream sockets. > > Signed-off-by: Jiang Wang <[email protected]> > ---
Overall this looks good. The tricky thing will be implementing dgram
sockets in a way that minimizes dropped packets and provides some degree
of fairness between senders. Those are implementation issues though and
not visible at the device specification level.
> diff --git a/virtio-vsock.tex b/virtio-vsock.tex
> index da7e641..26a62ac 100644
> --- a/virtio-vsock.tex
> +++ b/virtio-vsock.tex
> @@ -9,14 +9,37 @@ \subsection{Device ID}\label{sec:Device Types / Socket
> Device / Device ID}
>
> \subsection{Virtqueues}\label{sec:Device Types / Socket Device / Virtqueues}
> \begin{description}
> -\item[0] rx
> -\item[1] tx
> +\item[0] stream rx
> +\item[1] stream tx
> +\item[2] datagram rx
> +\item[3] datagram tx
> +\item[4] event
> +\end{description}
> +The virtio socket device uses 5 queues if feature bit VIRTIO_VSOCK_F_DRGAM
> is set. Otherwise, it
> +only uses 3 queues, as the following.
s/as the following/as follows:/
> +
> +\begin{description}
> +\item[0] stream rx
> +\item[1] stream tx
> \item[2] event
> \end{description}
>
> +When behavior differs between stream and datagram rx/tx virtqueues
> +their full names are used. Common behavior is simply described in
> +terms of rx/tx virtqueues and applies to both stream and datagram
> +virtqueues.
> +
> \subsection{Feature bits}\label{sec:Device Types / Socket Device / Feature
> bits}
>
> -There are currently no feature bits defined for this device.
> +\begin{description}
> +\item[VIRTIO_VSOCK_F_STREAM (0)] Device has support for stream socket type.
> +\end{description}
> +
> +\begin{description}
> +\item[VIRTIO_VSOCK_F_DGRAM (2)] Device has support for datagram socket type.
Is this really bit 2 or did you mean bit 1 (value 0x2)?
What happens to the virtqueue layout when VIRTIO_VSOCK_F_DGRAM is
present and VIRTIO_VSOCK_F_STREAM is absent? The virtqueue section above
implies that VIRTIO_VSOCK_F_STREAM is always present.
> +\end{description}
> +
> +If no feature bits are defined, assume device only supports stream socket
> type.
It's cleaner to define VIRTIO_VSOCK_F_NO_STREAM (0) instead. When the
bit is set the stream socket type is not available and the stream_rx/tx
virtqueues are absent.
This way it's not necessary to define special behavior depending on
certain combinations of feature bits.
> \subsubsection{Receive and Transmit}\label{sec:Device Types / Socket Device
> / Device Operation / Receive and Transmit}
> -The driver queues outgoing packets on the tx virtqueue and incoming packet
> +The driver queues outgoing packets on the tx virtqueue and allocates
> incoming packet
> receive buffers on the rx virtqueue. Packets are of the following form:
This change seems unrelated to dgram sockets. I don't think adding the
word "allocates" makes things clearer or more precise. The driver may
reuse receive buffers rather than allocating fresh buffers. I suggest
dropping this change.
>
> \begin{lstlisting}
> @@ -195,6 +235,7 @@ \subsubsection{Receive and Transmit}\label{sec:Device
> Types / Socket Device / De
> };
> \end{lstlisting}
>
> +
> Virtqueue buffers for outgoing packets are read-only. Virtqueue buffers for
> incoming packets are write-only.
>
Unnecessary whitespace change. Please drop.
signature.asc
Description: PGP signature
_______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
