Re: Re: [PATCH v4 4/7] transport-fabrics: introduce command set

[zhenwei pi <pizhenwei@bytedance.com>]

On 6/27/23 17:04, Jason Wang wrote:
> On Mon, Jun 26, 2023 at 3:28âPM zhenwei pi <pizhenwei@bytedance.com> wrote:
> > Introduce command set for Virtio-oF which includes:
> > - command id: a tag to distinguish in-flight commands
> > - status: indicate the result of a Virtio-oF command
> > - opcodes: the Virtio-oF transport layer commands use 0x0000-0x0fff,
> >            and the device layer commands use 0x1000-0xffff.
> >            get/set status/feature/ config use consecutive number.
> >
> > Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> > ---
> >   transport-fabrics.tex | 616 ++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 616 insertions(+)
> >
> > diff --git a/transport-fabrics.tex b/transport-fabrics.tex
> > index d562b2c..3fd9e8e 100644
> > --- a/transport-fabrics.tex
> > +++ b/transport-fabrics.tex
> > @@ -113,3 +113,619 @@ \subsubsection{Keyed Data Transfers}\label{sec:Virtio Transport Options / Virtio
> >        |COMP|     |COMP|     |COMP|
> >        +----+     +----+     +----+
> >   \end{lstlisting}
> > +
> > +
> > +\subsection{Commands Definition}\label{sec:Virtio Transport Options / Virtio Over Fabrics / Commands Definition}
> > +This section defines data structures, opcodes, and status for Virtio-oF.
> > +A Virtio-oF command is fixed to 16 bytes, and a Virtio-oF completion is fixed to 16 bytes.
> > +Note that the reserved fields of Virtio-oF commands and completions are filled with zero.
> > +
> > +\subsubsection{Command ID}\label{sec:Virtio Transport Options / Virtio Over Fabrics / Commands Definition / Command ID}
> > +The Command ID (le16 type) is used to uniquely describe a Virtio-oF PDU for identification.
> > +Generally, the Virtio-oF initiator allocates a Command ID that is unique for all in-flight commands,
> > +and the Virtio-oF target specifies the same Command ID for completion.
> > +
> > +Command IDs 0xff00 - 0xffff are reserved for the Virtio-oF control queue asynchronous events.
> > +The reserved Command IDs for the Virtio-oF control queue are as follows:
> > +
> > +\begin{tabular}{ |l|l| }
> > +\hline
> > +Command ID & Description \\
> > +\hline \hline
> > +0xfffe & Config change. Causes the initiator to generate a configuration change notification \\
> > +\hline
> > +0xffff & Keepalive. The initiator ignores this event \\
> > +\hline
> > +0xff00 - 0xfffd & Reserved \\
> > +\hline
> > +\end{tabular}
> > +
> > +\subsubsection{Status}\label{sec:Virtio Transport Options / Virtio Over Fabrics / Commands Definition / Status}
> > +The Status (le16 type) of a Virtio-oF completion is used to indicate the result of a Virtio-oF command in detail.
> > +The status values are defined as follows:
> > +
> > +\begin{lstlisting}
> > +/* command executed successfully */
> > +#define VIRTIO_OF_STATUS_SUCCESS       0x0000
> > +
> > +/* unrecognized command, or disabled command due to unsupported feature */
> > +#define VIRTIO_OF_STATUS_ENOCMD        0x0001
> > +/* in-flight commands exceeded queue size */
> > +#define VIRTIO_OF_STATUS_ECMDQUOT      0x0002
> > +
> > +/* no such target specified by TVQN */
> > +#define VIRTIO_OF_STATUS_ENOTGT        0x1001
> > +/* failed to create Virtio-oF device instance */
> > +#define VIRTIO_OF_STATUS_ENODEV        0x1002
> > +/* rejected due to access control */
> > +#define VIRTIO_OF_STATUS_EACLREJECTED  0x1003
> > +/* bad Virtio-oF device instance ID */
> > +#define VIRTIO_OF_STATUS_EBADDEV       0x1010
> > +/* mismatched TVQN or IVQN from Virtio-oF device instance */
> > +#define VIRTIO_OF_STATUS_EBADVQN       0x1011
> > +/* Virtio-oF virtqueue index exceeded */
> > +#define VIRTIO_OF_STATUS_EQUEUEQUOT    0x1020
> > +/* Virtio-oF virtqueue size exceeded */
> > +#define VIRTIO_OF_STATUS_EQSIZEQUOT    0x1021
> > +
> > +/* unsupported Virtio-oF feature */
> > +#define VIRTIO_OF_STATUS_EFEATURE      0x2000
> > +/* unsupported Virtio-oF device instance status */
> > +#define VIRTIO_OF_STATUS_ESTATUS       0x2010
> > +/* unsupported Virtio-oF device instance feature */
> > +#define VIRTIO_OF_STATUS_EDEVFEATURE   0x2020
> > +/* unsupported offset of device configuration space */
> > +#define VIRTIO_OF_STATUS_ECONFOFF      0x2030
> > +/* unsupported bytes of device configuration space */
> > +#define VIRTIO_OF_STATUS_ECONFBYTES    0x2031
> > +/* failed to read device-readable virtqueue buffers */
> > +#define VIRTIO_OF_STATUS_EOUTVQBUF     0x20f0
> > +/* failed to write device-writable virtqueue buffers */
> > +#define VIRTIO_OF_STATUS_EINVQBUF      0x20f1
> > +\end{lstlisting}
> > +
> > +\subsubsection{Opcodes}\label{sec:Virtio Transport Options / Virtio Over Fabrics / Commands Definition / Opcodes}
> > +Opcodes (u16 type) of Virtio-oF Commands are as follows:
> > +
> > +\begin{lstlisting}
> > +#define virtio_of_op_connect               0x0000
> > +#define virtio_of_op_disconnect            0x0001
> > +#define virtio_of_op_keepalive             0x0002
> > +#define virtio_of_op_get_feature           0x0004
> > +#define virtio_of_op_set_feature           0x0005
> > +#define virtio_of_op_get_keyed_num_descs   0x0100
> > +#define virtio_of_op_vq                    0x0fff
> > +#define virtio_of_op_get_vendor_id         0x1000
> > +#define virtio_of_op_get_device_id         0x1001
> > +#define virtio_of_op_reset_device          0x1003
> > +#define virtio_of_op_get_status            0x1004
> > +#define virtio_of_op_set_status            0x1005
> > +#define virtio_of_op_get_device_feature    0x1006
> > +#define virtio_of_op_set_driver_feature    0x1009
> > +#define virtio_of_op_get_vq_size           0x100a
> > +#define virtio_of_op_get_config            0x100c
> > +#define virtio_of_op_set_config            0x100d
>
> We had a proposal of transport virtqueue that allows using a
> virtqueue[1] for configuring a device. And Ling shan will rebase it on
> top of the admin commands.
>
> That is to say, admin commands could be used for transport which will
> be duplicated with the commands you invented here.
>
> I wonder if the virtio-OF can just reuse the admin commands (have a
> quick glance and I didn't find any blocker for this).
>
> Thanks
>
> [1] https://lists.oasis-open.org/archives/virtio-comment/202208/msg00003.html

From my understand, the virtio device related commands looks 
similar(set/get status/features, reset device ...), but not same:
1, the admin commands are executed upon the admin virtqueue, it uses 
virtqueue based transfer.
The virtio-of commands are executed upon a virtio-of queue(established 
reliable connection), not a virtqueue. A virtio-of device could support 
admin commands in theory.
I suppose a virtio-of device with admin vq supported like:
+---------------------+
|   +-------------+   |
|   |control queue|   |
|   +-------------+   |
+---------------------+
|   +-------------+   |
|   | virtqueue0  |   |
|   +-------------+   |
|   | virtqueue1  |   |
|   +-------------+   |
|   | virtqueue ..|   |
|   +-------------+   |
|   |  admin vq   |   |
|   +-------------+   |
+---------------------+

The virtio-of commands ares used by both virtio-of control queue and 
virtio-of virtqueues(include admin vq).
The admin commands are used by admin vq only.

2, the virtio-of commands are used over fabrics, they are defined in a 
fixed size, the admin commands are described by virtqueue descriptors.

3, the virtio-of commands need a command_id field to distinguish each 
other for command depth, but the admin commands don't need command_id 
field(supported by virtqueue descriptor heads).

4, connect/disconnect are fabrics specific, stream/keyed transfer 
related features/commands are fabrics specific too.
interrupt related commands are not used by virtio-of.

As far as I can see, they are quite different, defining them separately 
would be fine.

--
zhenwei pi