[virtio-comment] [PATCH v3 0/8] Rename queue index to queue number

[virtio-comment] [PATCH v3 0/8] Rename queue index to queue number

[Parav Pandit]

1. Currently, virtqueue is identified between driver and device
interchangeably using either number of index terminology.

2. Between PCI and MMIO transport the queue size (depth) is
defined as queue_size and QueueNum respectively.

To avoid confusion and to have consistency, unify them to use as Number.

Solution:
a. Use virtqueue number description, and rename MMIO register as QueueSize.
b. Replace virtqueue index to virtqueue number
c. RSS area of virtio net has inherited some logic, describe it
using abstract rq_handle.

Patch summary:
patch-1 introduce vq number as generic term
patch-2 renames index to number for pci transport
patch-3 renames mmio register from Num to Size
patch-4 renames index to number for mmio transport
patch-5 renames num field to size for ccw transport
patch-6 renames index field to queue_select for ccw transport
patch-7 for virtio-net removes duplicate example from requirements
patch-8 for virtio-net updates rss description to use vq number

This series only improves the documentation, it does not change any
transport or device functionality.

Please review.
This series fixes the issue [1].

[1] https://github.com/oasis-tcs/virtio-spec/issues/163

---
changelog:
v2->v3:
- addressed comments from Michael
- added previous definitions for ccw fields
- moved rq_handle definition before using it
- added first patch to describe vq number
- updated pci for available buffer notification section
v1->v2:
- added patches for virtio net for rss area
- added patches for covering ccw transport
- added missing entries to refer in mmio transport


Parav Pandit (8):
  content: Add vq number text
  transport-pci: Refer to the vq by its number
  transport-mmio: Rename QueueNum register
  transport-mmio: Refer to the vq by its number
  transport-ccw: Rename queue depth/size to other transports
  transport-ccw: Refer to the vq by its number
  virtio-net: Avoid duplicate receive queue example
  virtio-net: Describe RSS using receive queue handle

 content.tex                      |  3 ++
 device-types/net/description.tex | 24 ++++++++++----
 transport-ccw.tex                | 51 ++++++++++++++++++++++-------
 transport-mmio.tex               | 56 +++++++++++++++++++-------------
 transport-pci.tex                | 14 ++++----
 5 files changed, 102 insertions(+), 46 deletions(-)

-- 
2.26.2


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/

[virtio-comment] [PATCH v3 1/8] content: Add vq number text

[Parav Pandit]

Introduce vq number and its range so that subsequent patches can refer
to it.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
changelog:
v2->v3:
- new patch
---
 content.tex | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/content.tex b/content.tex
index cff548a..1ab7794 100644
--- a/content.tex
+++ b/content.tex
@@ -321,6 +321,9 @@ \section{Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Virtqueues}
 VIRTIO_F_IN_ORDER feature. If negotiated, this knowledge
 might allow optimizations or simplify driver and/or device code.
 
+Each virtqueue is identified as vq number or also referred
+to as a virtqueue number; vq number range is from 0 to 65535.
+
 Each virtqueue can consist of up to 3 parts:
 \begin{itemize}
 \item Descriptor Area - used for describing buffers
-- 
2.26.2


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/

[virtio-comment] [PATCH v3 2/8] transport-pci: Refer to the vq by its number

[Parav Pandit]

Currently specification uses virtqueue index and
number interchangeably to refer to the virtqueue.

Instead refer to it by its number.

This patch is on top of [1].

[1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00527.html

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
changelog:
v2->v3:
- addressed comments from Michael
- changed vqn to virtqueue number in the Note
- refer to vqn field instead of virtqueue number
---
 content.tex       |  2 +-
 transport-pci.tex | 14 ++++++++------
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/content.tex b/content.tex
index 1ab7794..0f634ac 100644
--- a/content.tex
+++ b/content.tex
@@ -321,7 +321,7 @@ \section{Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Virtqueues}
 VIRTIO_F_IN_ORDER feature. If negotiated, this knowledge
 might allow optimizations or simplify driver and/or device code.
 
-Each virtqueue is identified as vq number or also referred
+Each virtqueue is identified by a vq number or also referred
 to as a virtqueue number; vq number range is from 0 to 65535.
 
 Each virtqueue can consist of up to 3 parts:
diff --git a/transport-pci.tex b/transport-pci.tex
index b07a822..044c085 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -390,13 +390,15 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
 
 \item[\field{queue_notify_data}]
         This field exists only if VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated.
-        The driver will use this value to put it in the 'virtqueue number' field
-        in the available buffer notification structure.
+        The driver will use this value to put it in the field
+        \field{vqn} in the available buffer notification
+        structure.
         See section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Available Buffer Notifications}.
         \begin{note}
         This field provides the device with flexibility to determine how virtqueues
         will be referred to in available buffer notifications.
-        In a trivial case the device can set \field{queue_notify_data}=vqn. Some devices
+        In a trivial case the device can set
+        \field{queue_notify_data}=vq number. Some devices
         may benefit from providing another value, for example an internal virtqueue
         identifier, or an internal offset related to the virtqueue number.
         \end{note}
@@ -1005,7 +1007,7 @@ \subsubsection{Device Initialization}\label{sec:Virtio Transport Options / Virti
 The driver typically does this as follows, for each virtqueue a device has:
 
 \begin{enumerate}
-\item Write the virtqueue index (first queue is 0) to \field{queue_select}.
+\item Write the virtqueue number (first queue is 0) to \field{queue_select}.
 
 \item Read the virtqueue size from \field{queue_size}. This controls how big the virtqueue is
   (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues}~\nameref{sec:Basic Facilities of a Virtio Device / Virtqueues}). If this field is 0, the virtqueue does not exist.
@@ -1035,7 +1037,7 @@ \subsubsection{Available Buffer Notifications}\label{sec:Virtio Transport Option
 
 When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
 the driver sends an available buffer notification to the device by writing
-the 16-bit virtqueue index
+the 16-bit virtqueue number
 of this virtqueue to the Queue Notify address.
 
 When VIRTIO_F_NOTIFICATION_DATA has been negotiated,
@@ -1053,7 +1055,7 @@ \subsubsection{Available Buffer Notifications}\label{sec:Virtio Transport Option
 If VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated:
 \begin{itemize}
 \item If VIRTIO_F_NOTIFICATION_DATA has not been negotiated, the driver MUST use the
-\field{queue_notify_data} value instead of the virtqueue index.
+\field{queue_notify_data} value instead of the virtqueue number.
 \item If VIRTIO_F_NOTIFICATION_DATA has been negotiated, the driver MUST set the
 \field{vqn} field to the \field{queue_notify_data} value.
 \end{itemize}
-- 
2.26.2


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/

[virtio-comment] [PATCH v3 3/8] transport-mmio: Rename QueueNum register

[Parav Pandit]

Currently specification uses virtqueue index and number
interchangeably to refer to the virtqueue.

It is better to always refer to it the virtqueue in consistent manner.

Two registers QueueNumMax and QueueNum actually reflects the queue size
or queue depth indicating max and actual number of entries in the queue.

These are further named differently between pci and mmio transport.
PCI transport indicates queue size as queue_size.

To bring consistency between pci and mmio transport, and to avoid
confusion between number and index, rename the QueueNumMax and QueueNum
registers to QueueSizeMax and QueueSize respectively.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Parav Pandit <parav@nvidia.com>

---
changelog:
v0->v1:
- replaced references of QueueNumMax to QueueSizeMax
- replaced references of QueueNum to QueueSize
- added note for renamed fields old name suggested by @Michael Tsirkin
---
 transport-mmio.tex | 42 ++++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/transport-mmio.tex b/transport-mmio.tex
index f884a2c..3047633 100644
--- a/transport-mmio.tex
+++ b/transport-mmio.tex
@@ -110,24 +110,31 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
   \hline
   \mmioreg{QueueSel}{Virtual queue index}{0x030}{W}{%
     Writing to this register selects the virtual queue that the
-    following operations on \field{QueueNumMax}, \field{QueueNum}, \field{QueueReady},
+    following operations on \field{QueueSizeMax},
+    \field{QueueSize}, \field{QueueReady},
     \field{QueueDescLow}, \field{QueueDescHigh}, \field{QueueDriverlLow}, \field{QueueDriverHigh},
     \field{QueueDeviceLow}, \field{QueueDeviceHigh} and \field{QueueReset} apply to. The index
     number of the first queue is zero (0x0).
   }
   \hline
-  \mmioreg{QueueNumMax}{Maximum virtual queue size}{0x034}{R}{%
+  \mmioreg{QueueSizeMax}{Maximum virtual queue size}{0x034}{R}{%
     Reading from the register returns the maximum size (number of
     elements) of the queue the device is ready to process or
     zero (0x0) if the queue is not available. This applies to the
     queue selected by writing to \field{QueueSel}.
+    \begin{note}
+    \field{QueueSizeMax} was previously known as QueueNumMax.
+    \end{note}
   }
   \hline
-  \mmioreg{QueueNum}{Virtual queue size}{0x038}{W}{%
+  \mmioreg{QueueSize}{Virtual queue size}{0x038}{W}{%
     Queue size is the number of elements in the queue.
     Writing to this register notifies the device what size of the
     queue the driver will use. This applies to the queue selected by
     writing to \field{QueueSel}.
+    \begin{note}
+    \field{QueueSize} was previously known as QueueNum.
+    \end{note}
   }
   \hline
   \mmioreg{QueueReady}{Virtual queue ready bit}{0x044}{RW}{%
@@ -308,11 +315,11 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
 
 Before writing to the \field{DriverFeatures} register, the driver MUST write a value to the \field{DriverFeaturesSel} register.
 
-The driver MUST write a value to \field{QueueNum} which is less than
-or equal to the value presented by the device in \field{QueueNumMax}.
+The driver MUST write a value to \field{QueueSize} which is less than
+or equal to the value presented by the device in \field{QueueSizeMax}.
 
 When \field{QueueReady} is not zero, the driver MUST NOT access
-\field{QueueNum}, \field{QueueDescLow}, \field{QueueDescHigh},
+\field{QueueSize}, \field{QueueDescLow}, \field{QueueDescHigh},
 \field{QueueDriverLow}, \field{QueueDriverHigh}, \field{QueueDeviceLow}, \field{QueueDeviceHigh}.
 
 To stop using the queue the driver MUST write zero (0x0) to this
@@ -363,14 +370,14 @@ \subsubsection{Virtqueue Configuration}\label{sec:Virtio Transport Options / Vir
    and expect a returned value of zero (0x0).
 
 \item Read maximum queue size (number of elements) from
-   \field{QueueNumMax}. If the returned value is zero (0x0) the
+   \field{QueueSizeMax}. If the returned value is zero (0x0) the
    queue is not available.
 
 \item Allocate and zero the queue memory, making sure the memory
    is physically contiguous.
 
 \item Notify the device about the queue size by writing the size to
-   \field{QueueNum}.
+   \field{QueueSize}.
 
 \item Write physical addresses of the queue's Descriptor Area,
    Driver Area and Device Area to (respectively) the
@@ -465,25 +472,32 @@ \subsection{Legacy interface}\label{sec:Virtio Transport Options / Virtio Over M
   \hline
   \mmioreg{QueueSel}{Virtual queue index}{0x030}{W}{%
     Writing to this register selects the virtual queue that the
-    following operations on the \field{QueueNumMax}, \field{QueueNum}, \field{QueueAlign}
+    following operations on the \field{QueueSizeMax},
+    \field{QueueSize}, \field{QueueAlign}
     and \field{QueuePFN} registers apply to. The index
     number of the first queue is zero (0x0).
 .
   }
   \hline
-  \mmioreg{QueueNumMax}{Maximum virtual queue size}{0x034}{R}{%
+  \mmioreg{QueueSizeMax}{Maximum virtual queue size}{0x034}{R}{%
     Reading from the register returns the maximum size of the queue
     the device is ready to process or zero (0x0) if the queue is not
     available. This applies to the queue selected by writing to
     \field{QueueSel} and is allowed only when \field{QueuePFN} is set to zero
     (0x0), so when the queue is not actively used.
+    \begin{note}
+    \field{QueueSizeMax} was previously known as QueueNumMax.
+    \end{note}
   }
   \hline
-  \mmioreg{QueueNum}{Virtual queue size}{0x038}{W}{%
+  \mmioreg{QueueSize}{Virtual queue size}{0x038}{W}{%
     Queue size is the number of elements in the queue.
     Writing to this register notifies the device what size of the
     queue the driver will use. This applies to the queue selected by
     writing to \field{QueueSel}.
+    \begin{note}
+    \field{QueueSize} was previously known as QueueNum.
+    \end{note}
   }
   \hline
   \mmioreg{QueueAlign}{Used Ring alignment in the virtual queue}{0x03c}{W}{%
@@ -543,16 +557,16 @@ \subsection{Legacy interface}\label{sec:Virtio Transport Options / Virtio Over M
    expecting a returned value of zero (0x0).
 
 \item Read maximum queue size (number of elements) from
-   \field{QueueNumMax}. If the returned value is zero (0x0) the
+   \field{QueueSizeMax}. If the returned value is zero (0x0) the
    queue is not available.
 
 \item Allocate and zero the queue pages in contiguous virtual
    memory, aligning the Used Ring to an optimal boundary (usually
    page size). The driver should choose a queue size smaller than or
-   equal to \field{QueueNumMax}.
+   equal to \field{QueueSizeMax}.
 
 \item Notify the device about the queue size by writing the size to
-   \field{QueueNum}.
+   \field{QueueSize}.
 
 \item Notify the device about the used alignment by writing its value
    in bytes to \field{QueueAlign}.
-- 
2.26.2


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/

[virtio-comment] [PATCH v3 4/8] transport-mmio: Refer to the vq by its number

[Parav Pandit]

Currently specification uses virtqueue index and
number interchangeably to refer to the virtqueue.

Instead refer to it by its number.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
 transport-mmio.tex | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/transport-mmio.tex b/transport-mmio.tex
index 3047633..f9b3a34 100644
--- a/transport-mmio.tex
+++ b/transport-mmio.tex
@@ -108,7 +108,7 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
     bits accessible by writing to \field{DriverFeatures}.
   }
   \hline
-  \mmioreg{QueueSel}{Virtual queue index}{0x030}{W}{%
+  \mmioreg{QueueSel}{Virtual queue number}{0x030}{W}{%
     Writing to this register selects the virtual queue that the
     following operations on \field{QueueSizeMax},
     \field{QueueSize}, \field{QueueReady},
@@ -149,7 +149,7 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
     there are new buffers to process in a queue.
 
     When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
-    the value written is the queue index.
+    the value written is the queue number.
 
     When VIRTIO_F_NOTIFICATION_DATA has been negotiated,
     the \field{Notification data} value has the following format:
@@ -363,8 +363,7 @@ \subsubsection{Virtqueue Configuration}\label{sec:Virtio Transport Options / Vir
 The driver will typically initialize the virtual queue in the following way:
 
 \begin{enumerate}
-\item Select the queue writing its index (first queue is 0) to
-   \field{QueueSel}.
+\item Select the queue writing its number to \field{QueueSel}.
 
 \item Check if the queue is not already in use: read \field{QueueReady},
    and expect a returned value of zero (0x0).
@@ -392,7 +391,7 @@ \subsubsection{Available Buffer Notifications}\label{sec:Virtio Transport Option
 
 When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
 the driver sends an available buffer notification to the device by writing
-the 16-bit virtqueue index
+the 16-bit virtqueue number
 of the queue to be notified to \field{QueueNotify}.
 
 When VIRTIO_F_NOTIFICATION_DATA has been negotiated,
@@ -470,7 +469,7 @@ \subsection{Legacy interface}\label{sec:Virtio Transport Options / Virtio Over M
     (see QueuePFN).
   }
   \hline
-  \mmioreg{QueueSel}{Virtual queue index}{0x030}{W}{%
+  \mmioreg{QueueSel}{Virtual queue number}{0x030}{W}{%
     Writing to this register selects the virtual queue that the
     following operations on the \field{QueueSizeMax},
     \field{QueueSize}, \field{QueueAlign}
@@ -550,8 +549,7 @@ \subsection{Legacy interface}\label{sec:Virtio Transport Options / Virtio Over M
 
 The virtual queue is configured as follows:
 \begin{enumerate}
-\item Select the queue writing its index (first queue is 0) to
-   \field{QueueSel}.
+\item Select the queue writing its number to \field{QueueSel}.
 
 \item Check if the queue is not already in use: read \field{QueuePFN},
    expecting a returned value of zero (0x0).
-- 
2.26.2


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/

[virtio-comment] [PATCH v3 5/8] transport-ccw: Rename queue depth/size to other transports

[Parav Pandit]

max_num field reflects the maximum queue size/depth. Hence align name of
this field with similar field in PCI and MMIO transport to
max_queue_size.
Similarly rename 'num' to 'size'.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
 transport-ccw.tex | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/transport-ccw.tex b/transport-ccw.tex
index c492cb9..c2e60b6 100644
--- a/transport-ccw.tex
+++ b/transport-ccw.tex
@@ -237,12 +237,16 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
 \begin{lstlisting}
 struct vq_config_block {
         be16 index;
-        be16 max_num;
+        be16 max_queue_size;
 };
 \end{lstlisting}
 
 The requested number of buffers for queue \field{index} is returned in
-\field{max_num}.
+\field{max_queue_size}.
+
+\begin{note}
+\field{max_queue_size} was previously named as max_num.
+\end{note}
 
 Afterwards, CCW_CMD_SET_VQ is issued by the driver to inform the
 device about the location used for its queue. The transmitted
@@ -253,7 +257,7 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
         be64 desc;
         be32 res0;
         be16 index;
-        be16 num;
+        be16 size;
         be64 driver;
         be64 device;
 };
@@ -262,7 +266,12 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
 \field{desc}, \field{driver} and \field{device} contain the guest
 addresses for the descriptor area,
 available area and used area for queue \field{index}, respectively. The actual
-virtqueue size (number of allocated buffers) is transmitted in \field{num}.
+virtqueue size (number of allocated buffers) is transmitted in
+\field{size}.
+
+\begin{note}
+\field{size} was previously named as num.
+\end{note}
 
 \devicenormative{\paragraph}{Configuring a Virtqueue}{Virtio Transport Options / Virtio over channel I/O / Device Initialization / Configuring a Virtqueue}
 
@@ -278,13 +287,18 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
         be64 queue;
         be32 align;
         be16 index;
-        be16 num;
+        be16 size;
 };
 \end{lstlisting}
 
-\field{queue} contains the guest address for queue \field{index}, \field{num} the number of buffers
+\field{queue} contains the guest address for queue \field{index},
+\field{size} the number of buffers
 and \field{align} the alignment. The queue layout follows \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Legacy Interfaces: A Note on Virtqueue Layout}~\nameref{sec:Basic Facilities of a Virtio Device / Virtqueues / Legacy Interfaces: A Note on Virtqueue Layout}.
 
+\begin{note}
+\field{size} was previously named as num.
+\end{note}
+
 \subsubsection{Communicating Status Information}\label{sec:Virtio Transport Options / Virtio over channel I/O / Device Initialization / Communicating Status Information}
 
 The driver changes the status of a device via the
-- 
2.26.2


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/

[virtio-comment] [PATCH v3 6/8] transport-ccw: Refer to the vq by its number

[Parav Pandit]

Currently specification uses virtqueue index and
number interchangeably to refer to the virtqueue.

Instead refer to it by its number.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Signed-off-by: Parav Pandit <parav@nvidia.com>

---
changelog:
v2->v3:
- added comment note for queue_select similar to max_queue_size
v0->v1:
- new patch
---
 transport-ccw.tex | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/transport-ccw.tex b/transport-ccw.tex
index c2e60b6..880b87f 100644
--- a/transport-ccw.tex
+++ b/transport-ccw.tex
@@ -236,14 +236,18 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
 
 \begin{lstlisting}
 struct vq_config_block {
-        be16 index;
+        be16 queue_select;
         be16 max_queue_size;
 };
 \end{lstlisting}
 
-The requested number of buffers for queue \field{index} is returned in
+The requested number of buffers for queue \field{queue_select} is returned in
 \field{max_queue_size}.
 
+\begin{note}
+\field{queue_select} was previously named as index.
+\end{note}
+
 \begin{note}
 \field{max_queue_size} was previously named as max_num.
 \end{note}
@@ -256,7 +260,7 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
 struct vq_info_block {
         be64 desc;
         be32 res0;
-        be16 index;
+        be16 queue_select;
         be16 size;
         be64 driver;
         be64 device;
@@ -265,10 +269,14 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
 
 \field{desc}, \field{driver} and \field{device} contain the guest
 addresses for the descriptor area,
-available area and used area for queue \field{index}, respectively. The actual
+available area and used area for queue number \field{queue_select}, respectively. The actual
 virtqueue size (number of allocated buffers) is transmitted in
 \field{size}.
 
+\begin{note}
+\field{queue_select} was previously named as index.
+\end{note}
+
 \begin{note}
 \field{size} was previously named as num.
 \end{note}
@@ -286,15 +294,20 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
 struct vq_info_block_legacy {
         be64 queue;
         be32 align;
-        be16 index;
+        be16 queue_select;
         be16 size;
 };
 \end{lstlisting}
 
-\field{queue} contains the guest address for queue \field{index},
+\field{queue} contains the guest address for queue number
+\field{queue_select},
 \field{size} the number of buffers
 and \field{align} the alignment. The queue layout follows \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Legacy Interfaces: A Note on Virtqueue Layout}~\nameref{sec:Basic Facilities of a Virtio Device / Virtqueues / Legacy Interfaces: A Note on Virtqueue Layout}.
 
+\begin{note}
+\field{queue_select} was previously named as index.
+\end{note}
+
 \begin{note}
 \field{size} was previously named as num.
 \end{note}
@@ -583,7 +596,7 @@ \subsubsection{Guest->Host Notification}\label{sec:Virtio Transport Options / Vi
 For example:
 \begin{lstlisting}
 info->cookie = do_notify(schid,
-                         virtqueue_get_queue_index(vq),
+                         virtqueue_get_queue_number(vq),
                          info->cookie);
 \end{lstlisting}
 \end{note}
-- 
2.26.2


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/

[virtio-comment] [PATCH v3 7/8] virtio-net: Avoid duplicate receive queue example

[Parav Pandit]

Receive queue number/index example is duplicate which is already defined
in the Setting RSS parameters section.

Hence, avoid such duplicate example and prepare it for the subsequent
patch to describe using receive queue handle.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
 device-types/net/description.tex | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index d7c8b1b..435c1fc 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -1467,8 +1467,8 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 The device MUST determine the destination queue for a network packet as follows:
 \begin{itemize}
 \item Calculate the hash of the packet as defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets}.
-\item If the device did not calculate the hash for the specific packet, the device directs the packet to the receiveq specified by \field{unclassified_queue} of virtio_net_rss_config structure (value of 0 corresponds to receiveq1).
-\item Apply \field{indirection_table_mask} to the calculated hash and use the result as the index in the indirection table to get 0-based number of destination receiveq (value of 0 corresponds to receiveq1).
+\item If the device did not calculate the hash for the specific packet, the device directs the packet to the receiveq specified by \field{unclassified_queue} of virtio_net_rss_config structure.
+\item Apply \field{indirection_table_mask} to the calculated hash and use the result as the index in the indirection table to get 0-based number of destination receiveq.
 \item If the destination receive queue is being reset (See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}), the device MUST drop the packet.
 \end{itemize}
 
-- 
2.26.2


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/

[virtio-comment] [PATCH v3 8/8] virtio-net: Describe RSS using receive queue handle

[Parav Pandit]

The content of indirection table and unclassified_queue which are
based on math calculation historically. To better describe this, to
avoid intermixing array index with virtqueue index and to use virtqueue
number

introduce a field rq_handle (receive queue handle) and refer them
to describe unclassified_queue and indirection_table fields.

As part of it, have the example that uses non zero virtqueue
number which helps to have better mapping between receiveX
object with virtqueue number and the actual value in the
indirection table.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Signed-off-by: Parav Pandit <parav@nvidia.com>

---
changelog:
v2->v3:
- moved rq_handle definition before using it
- removed duplicate example as rq_handle is now described first
v0->v1:
- new patch suggested by Michael Tsirkin
---
 device-types/net/description.tex | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 435c1fc..3de8994 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -1442,10 +1442,19 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 \field{indirection_table} array.
 Number of entries in \field{indirection_table} is (\field{indirection_table_mask} + 1).
 
-Field \field{unclassified_queue} contains the 0-based index of
-the receive virtqueue to place unclassified packets in. Index 0 corresponds to receiveq1.
+\begin{lstlisting}
+le16 rq_handle;
+\end{lstlisting}
+
+\field{rq_handle} is a receive virtqueue handle. It is calculated as
+virtqueue number divided by two. For example a receive virtqueue handle
+value of 3 corresponds to virtqueue number 6 maps to receiveq4.
+
+Field \field{unclassified_queue} contains the receive queue
+handle \field{rq_handle} described above.
 
-Field \field{indirection_table} contains an array of 0-based indices of receive virtqueues. Index 0 corresponds to receiveq1.
+Field \field{indirection_table} contains an array of receive
+queue handles described above in \field{rq_handle}.
 
 A driver sets \field{max_tx_vq} to inform a device how many transmit virtqueues it may use (transmitq1\ldots transmitq \field{max_tx_vq}).
 
@@ -1455,7 +1464,8 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 
 A driver MUST NOT send the VIRTIO_NET_CTRL_MQ_RSS_CONFIG command if the feature VIRTIO_NET_F_RSS has not been negotiated.
 
-A driver MUST fill the \field{indirection_table} array only with indices of enabled queues. Index 0 corresponds to receiveq1.
+A driver MUST fill the \field{indirection_table} array only with
+the \field{rq_handle} corresponding to the enabled receive virtqueues.
 
 The number of entries in \field{indirection_table} (\field{indirection_table_mask} + 1) MUST be a power of two.
 
@@ -1468,7 +1478,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 \begin{itemize}
 \item Calculate the hash of the packet as defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets}.
 \item If the device did not calculate the hash for the specific packet, the device directs the packet to the receiveq specified by \field{unclassified_queue} of virtio_net_rss_config structure.
-\item Apply \field{indirection_table_mask} to the calculated hash and use the result as the index in the indirection table to get 0-based number of destination receiveq.
+\item Apply \field{indirection_table_mask} to the calculated hash
+and use the result as the index in the indirection table to get
+\field{rq_handle}.
 \item If the destination receive queue is being reset (See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}), the device MUST drop the packet.
 \end{itemize}
 
-- 
2.26.2


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/

[virtio-comment] Re: [PATCH v3 8/8] virtio-net: Describe RSS using receive queue handle

[Michael S. Tsirkin]

On Tue, Mar 21, 2023 at 11:58:34PM +0200, Parav Pandit wrote:
> The content of indirection table and unclassified_queue which are
> based on math calculation historically. To better describe this, to
> avoid intermixing array index with virtqueue index and to use virtqueue
> number
> 
> introduce a field rq_handle (receive queue handle) and refer them
> to describe unclassified_queue and indirection_table fields.
> 
> As part of it, have the example that uses non zero virtqueue
> number which helps to have better mapping between receiveX
> object with virtqueue number and the actual value in the
> indirection table.
> 
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> 
> ---
> changelog:
> v2->v3:
> - moved rq_handle definition before using it
> - removed duplicate example as rq_handle is now described first
> v0->v1:
> - new patch suggested by Michael Tsirkin
> ---
>  device-types/net/description.tex | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> index 435c1fc..3de8994 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -1442,10 +1442,19 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  \field{indirection_table} array.
>  Number of entries in \field{indirection_table} is (\field{indirection_table_mask} + 1).
>  
> -Field \field{unclassified_queue} contains the 0-based index of
> -the receive virtqueue to place unclassified packets in. Index 0 corresponds to receiveq1.
> +\begin{lstlisting}
> +le16 rq_handle;
> +\end{lstlisting}
> +
> +\field{rq_handle} is a receive virtqueue handle. It is calculated as
> +virtqueue number divided by two. For example a receive virtqueue handle
> +value of 3 corresponds to virtqueue number 6 maps to receiveq4.
> +
> +Field \field{unclassified_queue} contains the receive queue
> +handle \field{rq_handle} described above.

You dropped *which* queue this refers to.

And it's still kind of complex and non-standard. E.g. what does 
\begin{lstlisting}
le16 rq_handle;
\end{lstlisting}
mean exactly? Apparently nothing ...

I feel what we keep there is really the virtqueue number itself.
Just stored in this strage format.

And all this talk about handles kind of seems to add yet another
term to learn. Where in fact all it is, is just a different way
to store vqn.

So my idea was this: we say something like:


\field{unclassified_queue} contains the virtqueue number of the
receive queue to place unclassified packets in.
\field{indirection_table} contains an array of virtqueue numbers of
receive queues.

Both \field{unclassified_queue} and \field{indirection_table} use the
following format for the virtqueue numbers:
\begin{lstlisting}
struct rss_virtqueue_number {
	le16 vqn_16_1 : 15; /* Bits 16 to 1 of the virtqueue number */
	le16 reserved : 1; /* Set to 0 */
}
\end{lstlisting}
for example, a value of 3 corresponds to virtqueue number 6 and maps to receiveq4.



and then everywhere else we just say it keeps a vq number, we
already explained it is using this format once no need to repeat that.

WDYT?


>  
> -Field \field{indirection_table} contains an array of 0-based indices of receive virtqueues. Index 0 corresponds to receiveq1.
> +Field \field{indirection_table} contains an array of receive
> +queue handles described above in \field{rq_handle}.
>  
>  A driver sets \field{max_tx_vq} to inform a device how many transmit virtqueues it may use (transmitq1\ldots transmitq \field{max_tx_vq}).
>  
> @@ -1455,7 +1464,8 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  
>  A driver MUST NOT send the VIRTIO_NET_CTRL_MQ_RSS_CONFIG command if the feature VIRTIO_NET_F_RSS has not been negotiated.
>  
> -A driver MUST fill the \field{indirection_table} array only with indices of enabled queues. Index 0 corresponds to receiveq1.
> +A driver MUST fill the \field{indirection_table} array only with
> +the \field{rq_handle} corresponding to the enabled receive virtqueues.
>  
>  The number of entries in \field{indirection_table} (\field{indirection_table_mask} + 1) MUST be a power of two.
>  
> @@ -1468,7 +1478,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  \begin{itemize}
>  \item Calculate the hash of the packet as defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets}.
>  \item If the device did not calculate the hash for the specific packet, the device directs the packet to the receiveq specified by \field{unclassified_queue} of virtio_net_rss_config structure.
> -\item Apply \field{indirection_table_mask} to the calculated hash and use the result as the index in the indirection table to get 0-based number of destination receiveq.
> +\item Apply \field{indirection_table_mask} to the calculated hash
> +and use the result as the index in the indirection table to get
> +\field{rq_handle}.
>  \item If the destination receive queue is being reset (See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}), the device MUST drop the packet.
>  \end{itemize}
>  
> -- 
> 2.26.2


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/

[virtio-comment] Re: [PATCH v3 6/8] transport-ccw: Refer to the vq by its number

[Michael S. Tsirkin]

On Tue, Mar 21, 2023 at 11:58:32PM +0200, Parav Pandit wrote:
> Currently specification uses virtqueue index and
> number interchangeably to refer to the virtqueue.
> 
> Instead refer to it by its number.
> 
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> 
> ---
> changelog:
> v2->v3:
> - added comment note for queue_select similar to max_queue_size
> v0->v1:
> - new patch
> ---
>  transport-ccw.tex | 27 ++++++++++++++++++++-------
>  1 file changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/transport-ccw.tex b/transport-ccw.tex
> index c2e60b6..880b87f 100644
> --- a/transport-ccw.tex
> +++ b/transport-ccw.tex
> @@ -236,14 +236,18 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
>  
>  \begin{lstlisting}
>  struct vq_config_block {
> -        be16 index;
> +        be16 queue_select;
>          be16 max_queue_size;
>  };
>  \end{lstlisting}
>  
> -The requested number of buffers for queue \field{index} is returned in
> +The requested number of buffers for queue \field{queue_select} is returned in
>  \field{max_queue_size}.
>  
> +\begin{note}
> +\field{queue_select} was previously named as index.

sounds a bit strange, and \field{} is missing. If you insist I'd say
	in previous versions of this specification,
	field{queue_select} was also called queue \field{index}


e.g. in blk we have this:
  In the legacy interface, VIRTIO_BLK_F_FLUSH was also 
  called VIRTIO_BLK_F_WCE.


but I really feel this misses the point, the compat is needed
in the struct definition, not split out after usage
is described. This is why I proposed just making this a comment in the
struct. Why not?



> +\end{note}
> +
>  \begin{note}
>  \field{max_queue_size} was previously named as max_num.
>  \end{note}
> @@ -256,7 +260,7 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
>  struct vq_info_block {
>          be64 desc;
>          be32 res0;
> -        be16 index;
> +        be16 queue_select;
>          be16 size;
>          be64 driver;
>          be64 device;
> @@ -265,10 +269,14 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
>  
>  \field{desc}, \field{driver} and \field{device} contain the guest
>  addresses for the descriptor area,
> -available area and used area for queue \field{index}, respectively. The actual
> +available area and used area for queue number \field{queue_select}, respectively. The actual
>  virtqueue size (number of allocated buffers) is transmitted in
>  \field{size}.
>  
> +\begin{note}
> +\field{queue_select} was previously named as index.
> +\end{note}
> +
>  \begin{note}
>  \field{size} was previously named as num.
>  \end{note}
> @@ -286,15 +294,20 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
>  struct vq_info_block_legacy {
>          be64 queue;
>          be32 align;
> -        be16 index;
> +        be16 queue_select;
>          be16 size;
>  };
>  \end{lstlisting}
>  
> -\field{queue} contains the guest address for queue \field{index},
> +\field{queue} contains the guest address for queue number
> +\field{queue_select},
>  \field{size} the number of buffers
>  and \field{align} the alignment. The queue layout follows \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Legacy Interfaces: A Note on Virtqueue Layout}~\nameref{sec:Basic Facilities of a Virtio Device / Virtqueues / Legacy Interfaces: A Note on Virtqueue Layout}.
>  
> +\begin{note}
> +\field{queue_select} was previously named as index.
> +\end{note}
> +
>  \begin{note}
>  \field{size} was previously named as num.
>  \end{note}
> @@ -583,7 +596,7 @@ \subsubsection{Guest->Host Notification}\label{sec:Virtio Transport Options / Vi
>  For example:
>  \begin{lstlisting}
>  info->cookie = do_notify(schid,
> -                         virtqueue_get_queue_index(vq),
> +                         virtqueue_get_queue_number(vq),
>                           info->cookie);
>  \end{lstlisting}
>  \end{note}
> -- 
> 2.26.2


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/

[virtio-comment] RE: [PATCH v3 8/8] virtio-net: Describe RSS using receive queue handle

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Tuesday, March 21, 2023 6:17 PM

> > -Field \field{unclassified_queue} contains the 0-based index of -the
> > receive virtqueue to place unclassified packets in. Index 0 corresponds to
> receiveq1.
> > +\begin{lstlisting}
> > +le16 rq_handle;
> > +\end{lstlisting}
> > +
> > +\field{rq_handle} is a receive virtqueue handle. It is calculated as
> > +virtqueue number divided by two. For example a receive virtqueue
> > +handle value of 3 corresponds to virtqueue number 6 maps to receiveq4.
> > +
> > +Field \field{unclassified_queue} contains the receive queue handle
> > +\field{rq_handle} described above.
> 
> You dropped *which* queue this refers to.
>
Will add back.
 
> And it's still kind of complex and non-standard. E.g. what does \begin{lstlisting}
> le16 rq_handle;
> \end{lstlisting}
> mean exactly? Apparently nothing ...
> 
Why nothing, it is referenced further down.
Did you suggest moving before using it?
It was just fine to provide forward reference.

> I feel what we keep there is really the virtqueue number itself.
> Just stored in this strage format.
>
> And all this talk about handles kind of seems to add yet another term to learn.
> Where in fact all it is, is just a different way to store vqn.
> 
> So my idea was this: we say something like:
> 
> 
> \field{unclassified_queue} contains the virtqueue number of the receive queue
> to place unclassified packets in.
> \field{indirection_table} contains an array of virtqueue numbers of receive
> queues.
>
Above two lines are clearly confusing where virtqueue number describe in rest of the spec and above doesn't align to same notion.
So better to say field A contains the rq_handle and

struct rq_handle {
	le16 vqn_16_1: 15;
	le16 reserved : 1;
};

 
> Both \field{unclassified_queue} and \field{indirection_table} use the following
> format for the virtqueue numbers:
> \begin{lstlisting}
> struct rss_virtqueue_number {
It is really not any superior in term of cost of learning.

> 	le16 vqn_16_1 : 15; /* Bits 16 to 1 of the virtqueue number */
> 	le16 reserved : 1; /* Set to 0 */
I like the structure and reserved bit that enables to claim one bit for some unknown future use.
> }
> \end{lstlisting}
> for example, a value of 3 corresponds to virtqueue number 6 and maps to
> receiveq4.
> 
> 
> 
> and then everywhere else we just say it keeps a vq number, we already
> explained it is using this format once no need to repeat that.
>
I just prefer to rename it to rq_handle ( or at least other than virtqueue number) to distinguish it from rest of the virtqueue number.
 
> WDYT?

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/

[virtio-comment] RE: [PATCH v3 6/8] transport-ccw: Refer to the vq by its number

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Tuesday, March 21, 2023 6:22 PM

> > +\begin{note}
> > +\field{queue_select} was previously named as index.
> 
> sounds a bit strange, and \field{} is missing. If you insist I'd say
> 	in previous versions of this specification,
> 	field{queue_select} was also called queue \field{index}
> 
The field is missing because field index is no longer there.
> 
> e.g. in blk we have this:
>   In the legacy interface, VIRTIO_BLK_F_FLUSH was also
>   called VIRTIO_BLK_F_WCE.
> 
> 
> but I really feel this misses the point, the compat is needed
> in the struct definition, not split out after usage
> is described. This is why I proposed just making this a comment in the
> struct. Why not?
> 
Sure comment is good to me too.
In v0 you specifically asked to add note with example.
You said "like "Note: this was previously known as QueueNumMax""
It is hard to guess to write a comment when you mean "Note". :)

I will change to comment format.

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/

[virtio-comment] Re: [PATCH v3 8/8] virtio-net: Describe RSS using receive queue handle

[Michael S. Tsirkin]

On Wed, Mar 22, 2023 at 02:37:48AM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Tuesday, March 21, 2023 6:17 PM
> 
> > > -Field \field{unclassified_queue} contains the 0-based index of -the
> > > receive virtqueue to place unclassified packets in. Index 0 corresponds to
> > receiveq1.
> > > +\begin{lstlisting}
> > > +le16 rq_handle;
> > > +\end{lstlisting}
> > > +
> > > +\field{rq_handle} is a receive virtqueue handle. It is calculated as
> > > +virtqueue number divided by two. For example a receive virtqueue
> > > +handle value of 3 corresponds to virtqueue number 6 maps to receiveq4.
> > > +
> > > +Field \field{unclassified_queue} contains the receive queue handle
> > > +\field{rq_handle} described above.
> > 
> > You dropped *which* queue this refers to.
> >
> Will add back.
>  
> > And it's still kind of complex and non-standard. E.g. what does \begin{lstlisting}
> > le16 rq_handle;
> > \end{lstlisting}
> > mean exactly? Apparently nothing ...
> > 
> Why nothing, it is referenced further down.
> Did you suggest moving before using it?
> It was just fine to provide forward reference.

because it does not say anything about the contents or the format. just some kind
of integer.

> > I feel what we keep there is really the virtqueue number itself.
> > Just stored in this strage format.
> >
> > And all this talk about handles kind of seems to add yet another term to learn.
> > Where in fact all it is, is just a different way to store vqn.
> > 
> > So my idea was this: we say something like:
> > 
> > 
> > \field{unclassified_queue} contains the virtqueue number of the receive queue
> > to place unclassified packets in.
> > \field{indirection_table} contains an array of virtqueue numbers of receive
> > queues.
> >
> Above two lines are clearly confusing where virtqueue number describe in rest of the spec and above doesn't align to same notion.

That's true.

> So better to say field A contains the rq_handle and
> 
> struct rq_handle {
> 	le16 vqn_16_1: 15;
> 	le16 reserved : 1;
> };



>  
> > Both \field{unclassified_queue} and \field{indirection_table} use the following
> > format for the virtqueue numbers:
> > \begin{lstlisting}
> > struct rss_virtqueue_number {
> It is really not any superior in term of cost of learning.
> 
> > 	le16 vqn_16_1 : 15; /* Bits 16 to 1 of the virtqueue number */
> > 	le16 reserved : 1; /* Set to 0 */
> I like the structure and reserved bit that enables to claim one bit for some unknown future use.
> > }
> > \end{lstlisting}
> > for example, a value of 3 corresponds to virtqueue number 6 and maps to
> > receiveq4.
> > 
> > 
> > 
> > and then everywhere else we just say it keeps a vq number, we already
> > explained it is using this format once no need to repeat that.
> >
> I just prefer to rename it to rq_handle ( or at least other than virtqueue number) to distinguish it from rest of the virtqueue number.

Well first of all I really want to make it clear it's specific to
RSS at least for now. So let's prefix with rss_.
Maybe I'm wrong but I feel using up a completely
new term for something very specific to RSS is a waste.
We won't be able to use handle for something else without confusion.
So how about just

	struct rss_rq {
		le16 vqn_16_1 : 15; /* Bits 16 to 1 of the virtqueue number */
		le16 reserved : 1; /* Set to 0 */
	};

hmm?



> > WDYT?


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/

[virtio-comment] Re: [PATCH v3 6/8] transport-ccw: Refer to the vq by its number

[Michael S. Tsirkin]

On Wed, Mar 22, 2023 at 02:45:00AM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Tuesday, March 21, 2023 6:22 PM
> 
> > > +\begin{note}
> > > +\field{queue_select} was previously named as index.
> > 
> > sounds a bit strange, and \field{} is missing. If you insist I'd say
> > 	in previous versions of this specification,
> > 	field{queue_select} was also called queue \field{index}
> > 
> The field is missing because field index is no longer there.

But it was there. Look what it does, it formats in italics so
it stands out from rest of text, making it clear it is
field name (former one) and not the word "index" in english.

> > 
> > e.g. in blk we have this:
> >   In the legacy interface, VIRTIO_BLK_F_FLUSH was also
> >   called VIRTIO_BLK_F_WCE.
> > 
> > 
> > but I really feel this misses the point, the compat is needed
> > in the struct definition, not split out after usage
> > is described. This is why I proposed just making this a comment in the
> > struct. Why not?
> > 
> Sure comment is good to me too.
> In v0 you specifically asked to add note with example.
> You said "like "Note: this was previously known as QueueNumMax""
> It is hard to guess to write a comment when you mean "Note". :)

That referred to MMIO where it's a table not a listing.
I did not check the generated PDF the point is to make
the note appear near the field and also not damage the layout.
Pls take a look at how it looks in PDF - another option is
a footnote though it's a bit harder for readers to find these
and bad for accessibility. Again I don't think these work in listings,
there we are kind of limited to code comments.

> I will change to comment format.


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/

[virtio-comment] RE: [PATCH v3 6/8] transport-ccw: Refer to the vq by its number

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Tuesday, March 21, 2023 11:53 PM
> 
> On Wed, Mar 22, 2023 at 02:45:00AM +0000, Parav Pandit wrote:
> >
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Tuesday, March 21, 2023 6:22 PM
> >
> > > > +\begin{note}
> > > > +\field{queue_select} was previously named as index.
> > >
> > > sounds a bit strange, and \field{} is missing. If you insist I'd say
> > > 	in previous versions of this specification,
> > > 	field{queue_select} was also called queue \field{index}
> > >
> > The field is missing because field index is no longer there.
> 
> But it was there. Look what it does, it formats in italics so it stands out from
> rest of text, making it clear it is field name (former one) and not the word
> "index" in english.
True, but since that field index was written what would it refer to.
Anyway, not important once its part of the structure comment.
> 
> > >
> > > e.g. in blk we have this:
> > >   In the legacy interface, VIRTIO_BLK_F_FLUSH was also
> > >   called VIRTIO_BLK_F_WCE.
> > >
> > >
> > > but I really feel this misses the point, the compat is needed in the
> > > struct definition, not split out after usage is described. This is
> > > why I proposed just making this a comment in the struct. Why not?
> > >
> > Sure comment is good to me too.
> > In v0 you specifically asked to add note with example.
> > You said "like "Note: this was previously known as QueueNumMax""
> > It is hard to guess to write a comment when you mean "Note". :)
> 
> That referred to MMIO where it's a table not a listing.
> I did not check the generated PDF the point is to make the note appear near the
> field and also not damage the layout.
Yes, the note is next to the field. I looked in the PDF.
> Pls take a look at how it looks in PDF - another option is a footnote though it's a
> bit harder for readers to find these and bad for accessibility. Again I don't think
> these work in listings, there we are kind of limited to code comments.
> 
I was trying to have uniform note for mmio and ccw regardless off struct vs table.
But comment is fine too.
I will change to comment format for the struct.

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/

[virtio-comment] Re: [PATCH v3 6/8] transport-ccw: Refer to the vq by its number

[Michael S. Tsirkin]

On Wed, Mar 22, 2023 at 04:52:36PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Tuesday, March 21, 2023 11:53 PM
> > 
> > On Wed, Mar 22, 2023 at 02:45:00AM +0000, Parav Pandit wrote:
> > >
> > > > From: Michael S. Tsirkin <mst@redhat.com>
> > > > Sent: Tuesday, March 21, 2023 6:22 PM
> > >
> > > > > +\begin{note}
> > > > > +\field{queue_select} was previously named as index.
> > > >
> > > > sounds a bit strange, and \field{} is missing. If you insist I'd say
> > > > 	in previous versions of this specification,
> > > > 	field{queue_select} was also called queue \field{index}
> > > >
> > > The field is missing because field index is no longer there.
> > 
> > But it was there. Look what it does, it formats in italics so it stands out from
> > rest of text, making it clear it is field name (former one) and not the word
> > "index" in english.
> True, but since that field index was written what would it refer to.

Refer how?  \field{} is not a cross reference. It's a way to make
field names stand out to make sure readers do not think
this is plain english.

> Anyway, not important once its part of the structure comment.
> > 
> > > >
> > > > e.g. in blk we have this:
> > > >   In the legacy interface, VIRTIO_BLK_F_FLUSH was also
> > > >   called VIRTIO_BLK_F_WCE.
> > > >
> > > >
> > > > but I really feel this misses the point, the compat is needed in the
> > > > struct definition, not split out after usage is described. This is
> > > > why I proposed just making this a comment in the struct. Why not?
> > > >
> > > Sure comment is good to me too.
> > > In v0 you specifically asked to add note with example.
> > > You said "like "Note: this was previously known as QueueNumMax""
> > > It is hard to guess to write a comment when you mean "Note". :)
> > 
> > That referred to MMIO where it's a table not a listing.
> > I did not check the generated PDF the point is to make the note appear near the
> > field and also not damage the layout.
> Yes, the note is next to the field. I looked in the PDF.
> > Pls take a look at how it looks in PDF - another option is a footnote though it's a
> > bit harder for readers to find these and bad for accessibility. Again I don't think
> > these work in listings, there we are kind of limited to code comments.
> > 
> I was trying to have uniform note for mmio and ccw regardless off struct vs table.

That would involve rewriting MMIO description to match ccw and pci
using structs and not a table :)

> But comment is fine too.
> I will change to comment format for the struct.


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/

[virtio-comment] RE: [PATCH v3 6/8] transport-ccw: Refer to the vq by its number

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, March 22, 2023 12:56 PM
> 
> Refer how?  \field{} is not a cross reference. It's a way to make field names
> stand out to make sure readers do not think this is plain english.
> 
Ok. will change.

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/

[virtio-comment] RE: [PATCH v3 8/8] virtio-net: Describe RSS using receive queue handle

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Tuesday, March 21, 2023 11:46 PM
> >
> > > And it's still kind of complex and non-standard. E.g. what does
> > > \begin{lstlisting}
> > > le16 rq_handle;
> > > \end{lstlisting}
> > > mean exactly? Apparently nothing ...
> > >
> > Why nothing, it is referenced further down.
> > Did you suggest moving before using it?
> > It was just fine to provide a forward reference.
> 
> because it does not say anything about the contents or the format. just some
> kind of integer.
> 
Because it is an integer there is no need of a special format.
It does say about the content very clearly = vqn / 2;
But more below.

> > > I feel what we keep there is really the virtqueue number itself.
> > > Just stored in this strage format.
> > >
> > > And all this talk about handles kind of seems to add yet another term to
> learn.
> > > Where in fact all it is, is just a different way to store vqn.
> > >
> > > So my idea was this: we say something like:
> > >
> > >
> > > \field{unclassified_queue} contains the virtqueue number of the
> > > receive queue to place unclassified packets in.
> > > \field{indirection_table} contains an array of virtqueue numbers of
> > > receive queues.
> > >
> > Above two lines are clearly confusing where virtqueue number describe in
> rest of the spec and above doesn't align to same notion.
> 
> That's true.
> 
> > So better to say field A contains the rq_handle and
> >
> > struct rq_handle {
> > 	le16 vqn_16_1: 15;
> > 	le16 reserved : 1;
> > };
> 
> 
> 
> >
> > > Both \field{unclassified_queue} and \field{indirection_table} use
> > > the following format for the virtqueue numbers:
> > > \begin{lstlisting}
> > > struct rss_virtqueue_number {
> > It is really not any superior in term of cost of learning.
> >
> > > 	le16 vqn_16_1 : 15; /* Bits 16 to 1 of the virtqueue number */
> > > 	le16 reserved : 1; /* Set to 0 */
> > I like the structure and reserved bit that enables to claim one bit for some
> unknown future use.
> > > }
> > > \end{lstlisting}
> > > for example, a value of 3 corresponds to virtqueue number 6 and maps
> > > to receiveq4.
> > >
> > >
> > >
> > > and then everywhere else we just say it keeps a vq number, we
> > > already explained it is using this format once no need to repeat that.
> > >
> > I just prefer to rename it to rq_handle ( or at least other than virtqueue
> number) to distinguish it from rest of the virtqueue number.
> 
> Well first of all I really want to make it clear it's specific to RSS at least for now.
> So let's prefix with rss_.
Yes, rss_ prefix is good.

> Maybe I'm wrong but I feel using up a completely new term for something very
> specific to RSS is a waste.
> We won't be able to use handle for something else without confusion.
> So how about just
> 
> 	struct rss_rq {
> 		le16 vqn_16_1 : 15; /* Bits 16 to 1 of the virtqueue number */
> 		le16 reserved : 1; /* Set to 0 */
> 	};
Rq is usually an object and here we want to just refer to its id/vqn/handle.

Hence, I prefer rss_rq_handle {} or rss_rq_id{} for the structure name.
WDYT?


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/

[virtio-comment] Re: [PATCH v3 8/8] virtio-net: Describe RSS using receive queue handle

[Michael S. Tsirkin]

On Wed, Mar 22, 2023 at 05:07:51PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Tuesday, March 21, 2023 11:46 PM
> > >
> > > > And it's still kind of complex and non-standard. E.g. what does
> > > > \begin{lstlisting}
> > > > le16 rq_handle;
> > > > \end{lstlisting}
> > > > mean exactly? Apparently nothing ...
> > > >
> > > Why nothing, it is referenced further down.
> > > Did you suggest moving before using it?
> > > It was just fine to provide a forward reference.
> > 
> > because it does not say anything about the contents or the format. just some
> > kind of integer.
> > 
> Because it is an integer there is no need of a special format.
> It does say about the content very clearly = vqn / 2;
> But more below.
> 
> > > > I feel what we keep there is really the virtqueue number itself.
> > > > Just stored in this strage format.
> > > >
> > > > And all this talk about handles kind of seems to add yet another term to
> > learn.
> > > > Where in fact all it is, is just a different way to store vqn.
> > > >
> > > > So my idea was this: we say something like:
> > > >
> > > >
> > > > \field{unclassified_queue} contains the virtqueue number of the
> > > > receive queue to place unclassified packets in.
> > > > \field{indirection_table} contains an array of virtqueue numbers of
> > > > receive queues.
> > > >
> > > Above two lines are clearly confusing where virtqueue number describe in
> > rest of the spec and above doesn't align to same notion.
> > 
> > That's true.
> > 
> > > So better to say field A contains the rq_handle and
> > >
> > > struct rq_handle {
> > > 	le16 vqn_16_1: 15;
> > > 	le16 reserved : 1;
> > > };
> > 
> > 
> > 
> > >
> > > > Both \field{unclassified_queue} and \field{indirection_table} use
> > > > the following format for the virtqueue numbers:
> > > > \begin{lstlisting}
> > > > struct rss_virtqueue_number {
> > > It is really not any superior in term of cost of learning.
> > >
> > > > 	le16 vqn_16_1 : 15; /* Bits 16 to 1 of the virtqueue number */
> > > > 	le16 reserved : 1; /* Set to 0 */
> > > I like the structure and reserved bit that enables to claim one bit for some
> > unknown future use.
> > > > }
> > > > \end{lstlisting}
> > > > for example, a value of 3 corresponds to virtqueue number 6 and maps
> > > > to receiveq4.
> > > >
> > > >
> > > >
> > > > and then everywhere else we just say it keeps a vq number, we
> > > > already explained it is using this format once no need to repeat that.
> > > >
> > > I just prefer to rename it to rq_handle ( or at least other than virtqueue
> > number) to distinguish it from rest of the virtqueue number.
> > 
> > Well first of all I really want to make it clear it's specific to RSS at least for now.
> > So let's prefix with rss_.
> Yes, rss_ prefix is good.
> 
> > Maybe I'm wrong but I feel using up a completely new term for something very
> > specific to RSS is a waste.
> > We won't be able to use handle for something else without confusion.
> > So how about just
> > 
> > 	struct rss_rq {
> > 		le16 vqn_16_1 : 15; /* Bits 16 to 1 of the virtqueue number */
> > 		le16 reserved : 1; /* Set to 0 */
> > 	};
> Rq is usually an object and here we want to just refer to its id/vqn/handle.
> 
> Hence, I prefer rss_rq_handle {} or rss_rq_id{} for the structure name.
> WDYT?

_id is ok. But don't define it separately as a special object, it's just
the format of indirection_table and unclassified_queue.
IOW define at point of 1st use.

-- 
MST


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/

[virtio-comment] RE: [PATCH v3 8/8] virtio-net: Describe RSS using receive queue handle

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, March 22, 2023 4:57 PM


> _id is ok. But don't define it separately as a special object, it's just the format of
> indirection_table and unclassified_queue.
> IOW define at point of 1st use.

Sounds good. Sent v4 with this change with rss_id next to rss_config struct.

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/