Re: [PATCH v2 03/11] transport-fabircs: introduce Segment Descriptor Definition

[Parav Pandit <parav@nvidia.com>]

On 5/4/2023 4:19 AM, zhenwei pi wrote:
> Introduce segment descriptor to describe the Virtio device buffer
> segments.
>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
>   transport-fabrics.tex | 43 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 43 insertions(+)
>
> diff --git a/transport-fabrics.tex b/transport-fabrics.tex
> index 26b0192..b88acfd 100644
> --- a/transport-fabrics.tex
> +++ b/transport-fabrics.tex
> @@ -45,3 +45,46 @@ \subsection{Virtio Qualified Name}\label{sec:Virtio Transport Options / Virtio O
>   \item The string is null terminated.
>   \item There is no strict style limitation.
>   \end{itemize}
> +
> +\subsection{Transmission Protocol}\label{sec:Virtio Transport Options / Virtio Over Fabrics / Transmission Protocol}
> +This section defines transmission protocol for Virtio Over Fabrics. All the
> +fields use little endian format.
> +
> +\subsubsection{Segment Descriptor Definition}\label{sec:Virtio Transport Options / Virtio Over Fabrics / Transmission Protocol / Segment Descriptor Definition}
> +Virtio Over Fabrics uses the following structure to describe data segment:
> +
> +\begin{lstlisting}
> +struct virtio_of_vring_desc {
vring doesnt seem necessary here.

> +        le64 addr;
> +        le32 length;
> +        /* This marks the unique ID within a command, no limitation among inflight commands */
> +        le16 id;
id and key both seems redundant.
Not sure the need of id in each descriptor.
Yet to read full...

> +        /* This marks a buffer as keyed transmission (otherwise stream transmission) */
> +#define VIRTIO_OF_DESC_F_KEYED     1
> +        /* This marks a buffer as device write-only (otherwise device read-only). */
> +#define VIRTIO_OF_DESC_F_WRITE     2
> +        le16 flags;
> +        le32 key;
> +};
> +\end{lstlisting}
> +
> +The structure virtio_of_vring_desc is used for both keyed transmission
> +(i.e. RDMA) and stream transmission(i.e. TCP). The fields is described as follows:
> +
> +\begin{tabular}{ |l|l|l| }
> +\hline
> +Field & keyed transmission & stream transmission \\
> +\hline \hline
> +addr & Start address of remote memory buffer & Start address within the stream buffer \\
> +\hline
> +length & The length of remote memory buffer & The length of buffer within the stream \\
> +\hline
> +id & The ID of this descriptor & The ID of this descriptor \\
> +\hline
> +flags & both keyed transmission and stream transmission supported & stream transmission only \\
> +\hline
This is probably transport specific, don't see the need to transport 
this bit when the transport type is already known as keyed.

> +key & Key of the remote Memory Region & Ignore \\
> +\hline
> +\end{tabular}
> +
> +Depending on the opcode, a Command contains zero or more structure virtio_of_vring_desc.
Please move above opcode line patch to different patch where opcode is 
introduced.