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

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

[Parav Pandit]

1. Currently, virtqueue is identified between driver and device
interchangeably using either number or 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 Number.

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

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 vqn 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:
v9->v10:
- added virtqueue number part in content in braces
- replaced queue_select to vqn in ccw
- avoided aggrasive alignment of 65 chars
- updated commit log to drop reference to already merged patches
- added review-by tag for already reviewed patches
v8->v9:
- addressed comments from David
- few corrections with article
- renaming 'virtqueue number' to 'vq number'
- improving text and wording for rss_rq_id, avail notification
- commit log of specific change in individual patches
v7->v8:
- remove note about first virtqueue number
- skipped Max's comment to put word 'structure' in same line as its
  crosses 65 chars limit per line
- reworded queue_notification data set line, as '=' and vq number
  wording was odd
v6->v7:
- remove text around first vq as it is already covered in the basic
  virtqueues facility section
v5->v6:
- moved the vq number description from middle of vq operation
  to beginning of vq introduction
v4->v5:
- fixed accidental removal of "unclassifed packets".
- simplfied text around indirection_table mask
- removed rss_rq_id references as indirection table and
  unclassified_queue data type is self explanatory
v3->v4:
- moved note to comment for ccw
- renamed rq_handle to rss_rq_id
- moved rss_rq_id next to rss_config structure
- define rss_config structure using rss_rq_id
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 rss rq id

 content.tex                      |  4 ++
 device-types/net/description.tex | 29 ++++++++++----
 transport-ccw.tex                | 26 +++++++------
 transport-mmio.tex               | 65 ++++++++++++++++++--------------
 transport-pci.tex                | 13 ++++---
 5 files changed, 84 insertions(+), 53 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 v10 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:
v9->v10:
- added braces around vq number wording
- added vqn as another term for vq number
v8->v9:
- added inclusive when describing the vq number range
- skipped comment to put virtqueue number wording first because we
  prefer to use shorter vq number as much as possible
v5->v6:
- moved description close to introduction, it was in middle of
  queue data transfer description
v2->v3:
- new patch
---
 content.tex | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/content.tex b/content.tex
index cff548a..7bac167 100644
--- a/content.tex
+++ b/content.tex
@@ -298,6 +298,10 @@ \section{Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Virtqueues}
 virtqueues\footnote{For example, the simplest network device has one virtqueue for
 transmit and one for receive.}.
 
+Each virtqueue is identified by a vq number (also referred
+to as a virtqueue number or vqn); vq number range is from 0 to 65535
+inclusive.
+
 Driver makes requests available to device by adding
 an available buffer to the queue, i.e., adding a buffer
 describing the request to a virtqueue, and optionally triggering
-- 
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 v10 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.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
changelog:
v9->v10:
- updated commit log to drop reference to old patch
v8->v9:
- reword the sentence to avoid future tense, like rest of the other
  fields description
- reword the sentence to avoid multiple verbs use and put -> uses
- use shorter name 'vq number' instead of 'virtqueue number'
v7->v8:
- remove note about first virtqueue number
- skipped Max's comment to put word 'structure' in same line as its
  crosses 65 chars limit per line
- reworded queue_notification data set line, as '=' and vq number
  wording was odd
v2->v3:
- addressed comments from Michael
- changed vqn to virtqueue number in the Note
- refer to vqn field instead of virtqueue number
---
 transport-pci.tex | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/transport-pci.tex b/transport-pci.tex
index b07a822..0f3a48b 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -390,13 +390,14 @@ \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
+        The driver uses this value 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} to the 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 +1006,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 vq number 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,8 +1036,8 @@ \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
-of this virtqueue to the Queue Notify address.
+the 16-bit vq number of this virtqueue to the Queue Notify
+address.
 
 When VIRTIO_F_NOTIFICATION_DATA has been negotiated,
 the driver sends an available buffer notification to the device by writing
@@ -1053,7 +1054,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 vq 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 v10 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: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Parav Pandit <parav@nvidia.com>

---
changelog:
v8->v9:
- added field tag to indicate field name instead of English word
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..164e640 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 \field{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 \field{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 \field{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 \field{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 v10 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: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Parav Pandit <parav@nvidia.com>

---
changelog:
v8->v9:
- added 'by' at two places
- replaced 'queue number' with 'vq number'

v6->v7:
- remove text around first vq as it is already covered in the basic
  virtqueues facility section
---
 transport-mmio.tex | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/transport-mmio.tex b/transport-mmio.tex
index 164e640..1350b02 100644
--- a/transport-mmio.tex
+++ b/transport-mmio.tex
@@ -108,13 +108,12 @@ \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},
     \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).
+    \field{QueueDeviceLow}, \field{QueueDeviceHigh} and \field{QueueReset} apply to.
   }
   \hline
   \mmioreg{QueueSizeMax}{Maximum virtual queue size}{0x034}{R}{%
@@ -149,7 +148,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 vq number.
 
     When VIRTIO_F_NOTIFICATION_DATA has been negotiated,
     the \field{Notification data} value has the following format:
@@ -363,8 +362,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 by 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,8 +390,8 @@ \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
-of the queue to be notified to \field{QueueNotify}.
+the 16-bit vq number of the queue to be notified to
+\field{QueueNotify}.
 
 When VIRTIO_F_NOTIFICATION_DATA has been negotiated,
 the driver sends an available buffer notification to the device by writing
@@ -470,13 +468,11 @@ \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}
-    and \field{QueuePFN} registers apply to. The index
-    number of the first queue is zero (0x0).
-.
+    and \field{QueuePFN} registers apply to.
   }
   \hline
   \mmioreg{QueueSizeMax}{Maximum virtual queue size}{0x034}{R}{%
@@ -550,8 +546,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 by 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 v10 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>

---
changelog:
v9>v10:
- used lower case letter for comment first word
v8->v9:
- replaced 'named' as 'known'
v3->v4:
- moved note to comment
---
 transport-ccw.tex | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/transport-ccw.tex b/transport-ccw.tex
index c492cb9..3ce3f9f 100644
--- a/transport-ccw.tex
+++ b/transport-ccw.tex
@@ -237,12 +237,12 @@ \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; /* previously known as max_num */
 };
 \end{lstlisting}
 
 The requested number of buffers for queue \field{index} is returned in
-\field{max_num}.
+\field{max_queue_size}.
 
 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 +253,7 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
         be64 desc;
         be32 res0;
         be16 index;
-        be16 num;
+        be16 size; /* previously known as num */
         be64 driver;
         be64 device;
 };
@@ -262,7 +262,8 @@ \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}.
 
 \devicenormative{\paragraph}{Configuring a Virtqueue}{Virtio Transport Options / Virtio over channel I/O / Device Initialization / Configuring a Virtqueue}
 
@@ -278,11 +279,12 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
         be64 queue;
         be32 align;
         be16 index;
-        be16 num;
+        be16 size; /* previously known as num */
 };
 \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}.
 
 \subsubsection{Communicating Status Information}\label{sec:Virtio Transport Options / Virtio over channel I/O / Device Initialization / Communicating Status Information}
-- 
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 v10 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:
v9->v10:
- replaced queue_select with vqn
- used lower case letter for first word in comment
v8->v9:
- replaced 'named' with 'known'
- replaced 'queue number' with 'vq number'
v3->v4:
- moved note to comment
v2->v3:
- added comment note for queue_select similar to max_queue_size
v0->v1:
- new patch
---
 transport-ccw.tex | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/transport-ccw.tex b/transport-ccw.tex
index 3ce3f9f..23a6483 100644
--- a/transport-ccw.tex
+++ b/transport-ccw.tex
@@ -237,11 +237,12 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
 \begin{lstlisting}
 struct vq_config_block {
         be16 index;
+        be16 vqn; /* previously known as index */
         be16 max_queue_size; /* previously known as max_num */
 };
 \end{lstlisting}
 
-The requested number of buffers for queue \field{index} is returned in
+The requested number of buffers for queue \field{vqn} is returned in
 \field{max_queue_size}.
 
 Afterwards, CCW_CMD_SET_VQ is issued by the driver to inform the
@@ -252,7 +253,7 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
 struct vq_info_block {
         be64 desc;
         be32 res0;
-        be16 index;
+        be16 vqn; /* previously known as index */
         be16 size; /* previously known as num */
         be64 driver;
         be64 device;
@@ -261,7 +262,7 @@ \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 vq number \field{vqn}, respectively. The actual
 virtqueue size (number of allocated buffers) is transmitted in
 \field{size}.
 
@@ -278,12 +279,13 @@ \subsubsection{Configuring a Virtqueue}\label{sec:Virtio Transport Options / Vir
 struct vq_info_block_legacy {
         be64 queue;
         be32 align;
-        be16 index;
+        be16 vqn; /* previously known as index */
         be16 size; /* previously known as num */
 };
 \end{lstlisting}
 
-\field{queue} contains the guest address for queue \field{index},
+\field{queue} contains the guest address for vq number
+\field{vqn},
 \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}.
 
@@ -571,7 +573,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 v10 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 v10 8/8] virtio-net: Describe RSS using rss rq id

[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:
v8->v9:
- reword rss_rq_id and unclassified_packets description
- use article
- use 'vq number' instead of 'virtqueue number'
v4->v5:
- change subject to reflect rss_rq_id
- fixed accidental removal of "unclassifed packets".
- simplfied text around indirection_table mask to use the term
  destination receive virtqueue. This aligns with next line about queue
  reset.
- removed rss_rq_id references as indirection table and
  unclassified_queue data type is self explanatory
v3->v4:
- renamed rq_handle to rss_rq_id
- moved rss_rq_id definition close to its usage in rss_config struct
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 | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 435c1fc..c64335d 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -1423,11 +1423,16 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 
 Driver sends a VIRTIO_NET_CTRL_MQ_RSS_CONFIG command using the following format for \field{command-specific-data}:
 \begin{lstlisting}
+struct rss_rq_id {
+   le16 vqn_1_16: 15; /* Bits 1 to 16 of the vq number */
+   le16 reserved: 1; /* Set to zero */
+};
+
 struct virtio_net_rss_config {
     le32 hash_types;
     le16 indirection_table_mask;
-    le16 unclassified_queue;
-    le16 indirection_table[indirection_table_length];
+    struct rss_rq_id unclassified_queue;
+    struct rss_rq_id indirection_table[indirection_table_length];
     le16 max_tx_vq;
     u8 hash_key_length;
     u8 hash_key_data[hash_key_length];
@@ -1442,10 +1447,15 @@ \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.
+\field{rss_rq_id} is a receive virtqueue id. \field{vqn_1_16}
+consists of bits 1 to 16 of a vq number. For example, a
+\field{vqn_1_16} value of 3 corresponds to vq number 6,
+which maps to receiveq4.
+
+Field \field{unclassified_queue} contains the receive virtqueue
+in which to place unclassified packets.
 
-Field \field{indirection_table} contains an array of 0-based indices of receive virtqueues. Index 0 corresponds to receiveq1.
+Field \field{indirection_table} is an array of receive virtqueues.
 
 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 +1465,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
+enabled receive virtqueues.
 
 The number of entries in \field{indirection_table} (\field{indirection_table_mask} + 1) MUST be a power of two.
 
@@ -1468,7 +1479,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
+the destination receive virtqueue.
 \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 v10 1/8] content: Add vq number text

[Cornelia Huck]

On Thu, Mar 30 2023, Parav Pandit <parav@nvidia.com> wrote:

> 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:
> v9->v10:
> - added braces around vq number wording
> - added vqn as another term for vq number
> v8->v9:
> - added inclusive when describing the vq number range
> - skipped comment to put virtqueue number wording first because we
>   prefer to use shorter vq number as much as possible
> v5->v6:
> - moved description close to introduction, it was in middle of
>   queue data transfer description
> v2->v3:
> - new patch
> ---
>  content.tex | 4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>


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 v10 6/8] transport-ccw: Refer to the vq by its number

[Cornelia Huck]

On Thu, Mar 30 2023, Parav Pandit <parav@nvidia.com> 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:
> v9->v10:
> - replaced queue_select with vqn
> - used lower case letter for first word in comment
> v8->v9:
> - replaced 'named' with 'known'
> - replaced 'queue number' with 'vq number'
> v3->v4:
> - moved note to comment
> v2->v3:
> - added comment note for queue_select similar to max_queue_size
> v0->v1:
> - new patch
> ---
>  transport-ccw.tex | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>


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 v10 7/8] virtio-net: Avoid duplicate receive queue example

[Cornelia Huck]

On Thu, Mar 30 2023, Parav Pandit <parav@nvidia.com> wrote:

> 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.

As you touch this sentence anyway, maybe make it "the 0-based number"?

>  \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}
>  

Otherwise,

Reviewed-by: Cornelia Huck <cohuck@redhat.com>


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 v10 8/8] virtio-net: Describe RSS using rss rq id

[Cornelia Huck]

On Thu, Mar 30 2023, Parav Pandit <parav@nvidia.com> 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.

This description is a bit confusing (and you renamed rq_handle as well.)
Does

"The content of the indirection table and unclassified_queue were
originally described based on mathematical operations. In order to make
it easier to understand and to avoid intermixing the array index with
the virtqueue number, introduce a structure rss_rq_id (RSS receive queue
ID) and use it to describe the unclassified_queue and indirection_table
fields."

capture the intent correctly?

>
> As part of it, have the example that uses non zero virtqueue

"have the example use a non-zero..." ?

> 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:
> v8->v9:
> - reword rss_rq_id and unclassified_packets description
> - use article
> - use 'vq number' instead of 'virtqueue number'
> v4->v5:
> - change subject to reflect rss_rq_id
> - fixed accidental removal of "unclassifed packets".
> - simplfied text around indirection_table mask to use the term
>   destination receive virtqueue. This aligns with next line about queue
>   reset.
> - removed rss_rq_id references as indirection table and
>   unclassified_queue data type is self explanatory
> v3->v4:
> - renamed rq_handle to rss_rq_id
> - moved rss_rq_id definition close to its usage in rss_config struct
> 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 | 27 ++++++++++++++++++++-------
>  1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> index 435c1fc..c64335d 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -1423,11 +1423,16 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  
>  Driver sends a VIRTIO_NET_CTRL_MQ_RSS_CONFIG command using the following format for \field{command-specific-data}:
>  \begin{lstlisting}
> +struct rss_rq_id {
> +   le16 vqn_1_16: 15; /* Bits 1 to 16 of the vq number */
> +   le16 reserved: 1; /* Set to zero */
> +};
> +
>  struct virtio_net_rss_config {
>      le32 hash_types;
>      le16 indirection_table_mask;
> -    le16 unclassified_queue;
> -    le16 indirection_table[indirection_table_length];
> +    struct rss_rq_id unclassified_queue;
> +    struct rss_rq_id indirection_table[indirection_table_length];
>      le16 max_tx_vq;
>      u8 hash_key_length;
>      u8 hash_key_data[hash_key_length];
> @@ -1442,10 +1447,15 @@ \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.
> +\field{rss_rq_id} is a receive virtqueue id. \field{vqn_1_16}
> +consists of bits 1 to 16 of a vq number. For example, a
> +\field{vqn_1_16} value of 3 corresponds to vq number 6,
> +which maps to receiveq4.
> +
> +Field \field{unclassified_queue} contains the receive virtqueue
> +in which to place unclassified packets.
>  
> -Field \field{indirection_table} contains an array of 0-based indices of receive virtqueues. Index 0 corresponds to receiveq1.
> +Field \field{indirection_table} is an array of receive virtqueues.

"an array of receive virtqueues identified via their rss_rq_id" ?

>  
>  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 +1465,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
> +enabled receive virtqueues.

"only with rss_rq_id references to enabled receive virtqueues" ?

>  
>  The number of entries in \field{indirection_table} (\field{indirection_table_mask} + 1) MUST be a power of two.
>  
> @@ -1468,7 +1479,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
> +the destination receive virtqueue.

Ok, you remove the line here anyway, so please just ignore my suggestion
for the previous patch.

>  \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}


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/

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

[Halil Pasic]

On Thu, 30 Mar 2023 00:23:35 +0300
Parav Pandit <parav@nvidia.com> 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
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> ---

[..]

>  transport-pci.tex | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/transport-pci.tex b/transport-pci.tex
> index b07a822..0f3a48b 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -390,13 +390,14 @@ \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
> +        The driver uses this value in the field \field{vqn}

This is one of the problems with this approach... 

>          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} to the vq number. Some devices

... the 'vq' number here is not the same vqn above which renders the usage
of vqn ambiguous. 

> @@ -1053,7 +1054,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 vq 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.

And here it is really obvious: if VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated
the field \field{vqn} does not hold a virtqueue number/vq  nuber/vqn but
a device supplied identifier which may or may not be same as the vqn.

So we went 
from:
if !VIRTIO_F_NOTIF_CONFIG_DATA then vqn is the virtqueue index
if !!VIRTIO_F_NOTIF_CONFIG_DATA then vqn is queue_notify_data which may
or may not be the same as the virtqueue index
to
if !VIRTIO_F_NOTIF_CONFIG_DATA then vqn is the vq number
if !!VIRTIO_F_NOTIF_CONFIG_DATA then vqn is queue_notify_data which may
or may not be the same as the vq number.

Which is my opinion less clear that what we had before. And in my opinion
calling the very same thing virtqueue und vq number interchangeably is
not helpful either -- makes grepping harder.

I don't see the benefit of replacing virtqueue index with virtqueue
number. AFAIR the supposed benefit was to:
a) harmonize the terminology in the notifications part with the rest of
the spec
b) to resolve the RSS problematic with its receive virtqueue indexes.

For b) we are going down a different route calling those receive queue
ids (AFAIR), and for the notifications part see my comments above.

I'm about to reply to the cover letter, and make my argument against
standardizing virtqueue nuber (and in favor of standardizing on the
on virtqueue index) along with a diff that is supposed to act as
a counter proposal. If that doesn't work I will give up and make peace
with vq number and vqn.

Regards,
Halil 


>  \end{itemize}


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: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number

[Halil Pasic]

On Thu, 30 Mar 2023 00:23:33 +0300
Parav Pandit <parav@nvidia.com> wrote:

> 1. Currently, virtqueue is identified between driver and device
> interchangeably using either number or 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 Number.
> 
> Solution:
> a. Use virtqueue number description, and rename MMIO register as QueueSize.

I'm in favor of replacing number with size where appropriate.

> b. Replace virtqueue index with virtqueue number

I don't see the benefit of replacing virtqueue index with virtqueue
number.

Currently virtqueue number is only used in the parts that describe
notifications (Guest->Host), the rest of the spec uses virtqueue index.

I argue that using a different term in that context than in the rest
of the specification makes sense, because in the context of notifications
the virtqueue isn't always identified by its index.

More precisely: if VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated in the
context of notifications the virtqueue is identified by the
so called "queue_notify_data"; if VIRTIO_F_NOTIF_CONFIG_DATA has been
negotiated in the context of notifications the virtqueue is identified by
the virtqueue index (as usual, for example in queue_select, or in
the ccws).

As I've pointed out in my comment to patch 2, I believe replacing
virtqueue index with virtqueue number is detrimental to clarity.

Thus please find a counter-proposal below. If there is interest
I can make a series out of it, and prettify it. If I can't convince
you guys, then I will have to get used to vqn and virtqueue number.

AFAIR the other problem with index was the RSS for virtio-net. But there
we are currently heading down a direction of introducing a new
abstraction. This approach avoids confusion around the term 'virtqueue
index' as much as it avoids confusion around the term 'virtqueue nuber'.


> c. RSS area of virtio net has inherited some logic, describe it
> using abstract rss_rq_id.

-------------------------8<--------------------------------------
From: Halil Pasic <pasic@linux.ibm.com>
Date: Thu, 30 Mar 2023 17:57:53 +0200
Subject: [PATCH 1/1] content: clarify how virtques are identified

Clarify how virtqueues are identified in the context of
available notifications and in the context of RSS for
virtio-net .

Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
---
 content.tex                      | 15 ++++++++++-----
 device-types/net/description.tex | 30 ++++++++++++++++++++++--------
 transport-ccw.tex                |  2 +-
 transport-pci.tex                |  7 ++++---
 4 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/content.tex b/content.tex
index cff548a..a376232 100644
--- a/content.tex
+++ b/content.tex
@@ -298,6 +298,9 @@ \section{Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Virtqueues}
 virtqueues\footnote{For example, the simplest network device has one virtqueue for
 transmit and one for receive.}.
 
+If not otherwise stated, each virtqueue is identified by a so called virtqueue
+index. Valid virtqueue indexes range from 0 up to 65535.
+
 Driver makes requests available to device by adding
 an available buffer to the queue, i.e., adding a buffer
 describing the request to a virtqueue, and optionally triggering
@@ -402,7 +405,7 @@ \section{Driver Notifications} \label{sec:Basic Facilities of a Virtio Device /
 
 When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
 this notification involves sending the
-virtqueue number to the device (method depending on the transport).
+virtqueue index to the device (method depending on the transport).
 
 However, some devices benefit from the ability to find out the
 amount of available data in the queue without accessing the virtqueue in memory:
@@ -413,7 +416,9 @@ \section{Driver Notifications} \label{sec:Basic Facilities of a Virtio Device /
 the following information:
 
 \begin{description}
-\item [vqn] VQ number to be notified.
+\item [vqn] Identifies the virtqueueue to be notified. If VIRTIO_F_NOTIF_CONFIG_DATA has been
+      negotiateted, the value of \field{vqn} is the notify data suplied by the device
+      (by transport specific means). Otherwise it is the virtqueue index.
 \item [next_off] Offset
       within the ring where the next available ring entry
       will be written.
@@ -786,13 +791,13 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
   buffer notifications.
   As mentioned in section \ref{sec:Basic Facilities of a Virtio Device / Driver notifications}, when the
   driver is required to send an available buffer notification to the device, it
-  sends the virtqueue number to be notified. The method of delivering
+  sends the virtqueue index of the virtqueue to be notified. The method of delivering
   notifications is transport specific.
   With the PCI transport, the device can optionally provide a per-virtqueue value
-  for the driver to use in driver notifications, instead of the virtqueue number.
+  for the driver to use in driver notifications, instead of the virtqueue index.
   Some devices may benefit from this flexibility by providing, for example,
   an internal virtqueue identifier, or an internal offset related to the
-  virtqueue number.
+  virtqueue index.
 
   This feature indicates the availability of such value. The definition of the
   data to be provided in driver notification and the delivery method is
diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index d7c8b1b..7678a57 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -1421,18 +1421,33 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 
 \subparagraph{Setting RSS parameters}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / Setting RSS parameters}
 
+
 Driver sends a VIRTIO_NET_CTRL_MQ_RSS_CONFIG command using the following format for \field{command-specific-data}:
 \begin{lstlisting}
+
+struct rss_rq_id {
+   le16 value; /* virtqueue index divided by two */
+};
+
 struct virtio_net_rss_config {
     le32 hash_types;
     le16 indirection_table_mask;
-    le16 unclassified_queue;
-    le16 indirection_table[indirection_table_length];
+    struct rss_rq_id unclassified_queue;
+    struct rss_rq_id indirection_table[indirection_table_length];
     le16 max_tx_vq;
     u8 hash_key_length;
     u8 hash_key_data[hash_key_length];
 };
 \end{lstlisting}
+
+The type struct rss\_rq\_id is introduced to better distinguish receive queue
+ids form other integral fields.
+
+A receive queue id is only defined for receive queues, as the virtqueue index
+of the receive virtqueue divided by two (the virtqueue index of a receive
+queue is always even). For example receiveq4 is identified by the virtqueue
+index 6 and the receive queue id 3.
+
 Field \field{hash_types} contains a bitmask of allowed hash types as
 defined in
 \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
@@ -1442,10 +1457,9 @@ \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.
+Field \field{unclassified_queue} contains the receive queue id of the receive virtqueue to place unclassified packets in.
 
-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 0-based indices of receive queue ids.
 
 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 +1469,7 @@ \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 receive queue ids of enabled queues.
 
 The number of entries in \field{indirection_table} (\field{indirection_table_mask} + 1) MUST be a power of two.
 
@@ -1467,8 +1481,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 receive queue id of the 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}
 
diff --git a/transport-ccw.tex b/transport-ccw.tex
index c492cb9..bc05639 100644
--- a/transport-ccw.tex
+++ b/transport-ccw.tex
@@ -544,7 +544,7 @@ \subsubsection{Guest->Host Notification}\label{sec:Virtio Transport Options / Vi
 \end{tabular}
 
 When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
-the \field{Notification data} contains the Virtqueue number.
+the \field{Notification data} contains the virtque index.
 
 When VIRTIO_F_NOTIFICATION_DATA has been negotiated,
 the value has the following format:
diff --git a/transport-pci.tex b/transport-pci.tex
index b07a822..9b71c05 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -390,15 +390,16 @@ \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
+        The driver will use this value to put it in the \field{vqn} field
         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} to the virtqueue index,
+        that is the value in \field{queue\_select}. Some devices
         may benefit from providing another value, for example an internal virtqueue
-        identifier, or an internal offset related to the virtqueue number.
+        identifier, or an internal offset related to the virtqueue index.
         \end{note}
 
 \item[\field{queue_reset}]
-- 
2.31.1




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/

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

[Parav Pandit]

> From: Halil Pasic <pasic@linux.ibm.com>
> Sent: Thursday, March 30, 2023 1:10 PM

> > -        The driver will use this value to put it in the 'virtqueue number' field
> > +        The driver uses this value in the field \field{vqn}
> 
> This is one of the problems with this approach...
> 
Instead of vqn, it is just q identifier.
Sometimes is vqn, sometimes its q config data.
Currently wording has taken care of it.
But a better field name can also do it in future.

> >          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} to the vq number. Some devices
> 
> ... the 'vq' number here is not the same vqn above which renders the usage of
> vqn ambiguous.
> 
Not sure I follow. It is the vqn of above case in trivial case.

> > @@ -1053,7 +1054,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 vq 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.
> 
> And here it is really obvious: if VIRTIO_F_NOTIF_CONFIG_DATA has been
> negotiated the field \field{vqn} does not hold a virtqueue number/vq
> nuber/vqn but a device supplied identifier which may or may not be same as
> the vqn.
> 
> So we went
> from:
> if !VIRTIO_F_NOTIF_CONFIG_DATA then vqn is the virtqueue index if
> !!VIRTIO_F_NOTIF_CONFIG_DATA then vqn is queue_notify_data which may or
> may not be the same as the virtqueue index to if
> !VIRTIO_F_NOTIF_CONFIG_DATA then vqn is the vq number if
> !!VIRTIO_F_NOTIF_CONFIG_DATA then vqn is queue_notify_data which may or
> may not be the same as the vq number.
> 
The current wording is written bit simpler than above multiple if-else. :)

> Which is my opinion less clear that what we had before. And in my opinion
> calling the very same thing virtqueue und vq number interchangeably is not
> helpful either -- makes grepping harder.
> 
> I don't see the benefit of replacing virtqueue index with virtqueue number.
> AFAIR the supposed benefit was to:
> a) harmonize the terminology in the notifications part with the rest of the spec
> b) to resolve the RSS problematic with its receive virtqueue indexes.
> 
And also, the upcoming patches to use either one of them that uses vqn at multiple places.
And you have already voted for vqn in the new patches ballot yesterday.

> For b) we are going down a different route calling those receive queue ids
> (AFAIR), and for the notifications part see my comments above.
> 
This would be done anyway regardless of picking "index" vs "number".

> I'm about to reply to the cover letter, and make my argument against
> standardizing virtqueue nuber (and in favor of standardizing on the on
> virtqueue index) along with a diff that is supposed to act as a counter proposal.
> If that doesn't work I will give up and make peace with vq number and vqn.

I think the filed name "vqn" in the notification area can be improved.
The main reason I didn't touch it much, because CONFIG_DATA has rare usage in most virtualized world, real devices will not implement it.
After 2+ years of introduction in the spec, no open-source user has find it useful yet.
May be some use it there.
So if there is motivation, we can rename the vqn field and further improve the language.

I will surely read through other proposal anyway.
Thanks for taking time into this.

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: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number

[Parav Pandit]

> From: Halil Pasic <pasic@linux.ibm.com>
> Sent: Thursday, March 30, 2023 1:11 PM

> I argue that using a different term in that context than in the rest of the
> specification makes sense, because in the context of notifications the virtqueue
> isn't always identified by its index.
Using single term as number is possible, so lets use single term. 

> 
> More precisely: if VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated in the
> context of notifications the virtqueue is identified by the so called
> "queue_notify_data"; 
> if VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated in

You missed "not" before negotiation. :)

> the context of notifications the virtqueue is identified by the virtqueue index (as
> usual, for example in queue_select, or in the ccws).
> 
> As I've pointed out in my comment to patch 2, I believe replacing virtqueue
> index with virtqueue number is detrimental to clarity.
> 
> Thus please find a counter-proposal below. If there is interest I can make a
> series out of it, and prettify it. If I can't convince you guys, then I will have to
> get used to vqn and virtqueue number.
> 
> AFAIR the other problem with index was the RSS for virtio-net. But there we are
> currently heading down a direction of introducing a new abstraction. This
> approach avoids confusion around the term 'virtqueue index' as much as it
> avoids confusion around the term 'virtqueue nuber'.
> 
> 
> > c. RSS area of virtio net has inherited some logic, describe it using
> > abstract rss_rq_id.
> 
> -------------------------8<--------------------------------------
> From: Halil Pasic <pasic@linux.ibm.com>
> Date: Thu, 30 Mar 2023 17:57:53 +0200
> Subject: [PATCH 1/1] content: clarify how virtques are identified
> 
> Clarify how virtqueues are identified in the context of available notifications
> and in the context of RSS for virtio-net .
> 
> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> ---
>  content.tex                      | 15 ++++++++++-----
>  device-types/net/description.tex | 30 ++++++++++++++++++++++--------
>  transport-ccw.tex                |  2 +-
>  transport-pci.tex                |  7 ++++---
>  4 files changed, 37 insertions(+), 17 deletions(-)
> 
> diff --git a/content.tex b/content.tex
> index cff548a..a376232 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -298,6 +298,9 @@ \section{Virtqueues}\label{sec:Basic Facilities of a
> Virtio Device / Virtqueues}  virtqueues\footnote{For example, the simplest
> network device has one virtqueue for  transmit and one for receive.}.
> 
> +If not otherwise stated, each virtqueue is identified by a so called
> +virtqueue index. Valid virtqueue indexes range from 0 up to 65535.
> +
Missing inclusive after 65535.
Covered in v10.

>  Driver makes requests available to device by adding  an available buffer to the
> queue, i.e., adding a buffer  describing the request to a virtqueue, and
> optionally triggering @@ -402,7 +405,7 @@ \section{Driver Notifications}
> \label{sec:Basic Facilities of a Virtio Device /
> 
>  When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,  this
> notification involves sending the -virtqueue number to the device (method
> depending on the transport).
> +virtqueue index to the device (method depending on the transport).
> 
>  However, some devices benefit from the ability to find out the  amount of
> available data in the queue without accessing the virtqueue in memory:
> @@ -413,7 +416,9 @@ \section{Driver Notifications} \label{sec:Basic Facilities
> of a Virtio Device /  the following information:
> 
>  \begin{description}
> -\item [vqn] VQ number to be notified.
> +\item [vqn] Identifies the virtqueueue to be notified. If
> VIRTIO_F_NOTIF_CONFIG_DATA has been
> +      negotiateted, the value of \field{vqn} is the notify data suplied by the
> device
There is still vqn here.  So no better than vq number.
To be truly clear, it should be renamed to vq_identifier, that either contains vqn or vq_notification_data.

As I replied I can take up that cleanup at later point if we find some motivation for CONFIG_DATA use.

> +      (by transport specific means). Otherwise it is the virtqueue index.
>  \item [next_off] Offset
>        within the ring where the next available ring entry
>        will be written.
> @@ -786,13 +791,13 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved
> Feature Bits}
>    buffer notifications.
>    As mentioned in section \ref{sec:Basic Facilities of a Virtio Device / Driver
> notifications}, when the
>    driver is required to send an available buffer notification to the device, it
> -  sends the virtqueue number to be notified. The method of delivering
> +  sends the virtqueue index of the virtqueue to be notified. The method
> + of delivering
>    notifications is transport specific.
>    With the PCI transport, the device can optionally provide a per-virtqueue
> value
> -  for the driver to use in driver notifications, instead of the virtqueue number.
> +  for the driver to use in driver notifications, instead of the virtqueue index.
>    Some devices may benefit from this flexibility by providing, for example,
>    an internal virtqueue identifier, or an internal offset related to the
> -  virtqueue number.
> +  virtqueue index.
> 
>    This feature indicates the availability of such value. The definition of the
>    data to be provided in driver notification and the delivery method is diff --git
> a/device-types/net/description.tex b/device-types/net/description.tex
> index d7c8b1b..7678a57 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -1421,18 +1421,33 @@ \subsubsection{Control
> Virtqueue}\label{sec:Device Types / Network Device / Devi
> 
>  \subparagraph{Setting RSS parameters}\label{sec:Device Types / Network
> Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) /
> Setting RSS parameters}
> 
> +
>  Driver sends a VIRTIO_NET_CTRL_MQ_RSS_CONFIG command using the
> following format for \field{command-specific-data}:
>  \begin{lstlisting}
> +
> +struct rss_rq_id {
> +   le16 value; /* virtqueue index divided by two */ };
> +
This misses the freeing up the last bit of the value that Michael suggested.
Once that is done, it will start looking like v10.

>  struct virtio_net_rss_config {
>      le32 hash_types;
>      le16 indirection_table_mask;
> -    le16 unclassified_queue;
> -    le16 indirection_table[indirection_table_length];
> +    struct rss_rq_id unclassified_queue;
> +    struct rss_rq_id indirection_table[indirection_table_length];
>      le16 max_tx_vq;
>      u8 hash_key_length;
>      u8 hash_key_data[hash_key_length];
>  };
>  \end{lstlisting}
> +
> +The type struct rss\_rq\_id is introduced to better distinguish receive
> +queue ids form other integral fields.
> +
> +A receive queue id is only defined for receive queues, as the virtqueue
> +index of the receive virtqueue divided by two (the virtqueue index of a
> +receive queue is always even). For example receiveq4 is identified by
> +the virtqueue index 6 and the receive queue id 3.
> +
>  Field \field{hash_types} contains a bitmask of allowed hash types as  defined in
> \ref{sec:Device Types / Network Device / Device Operation / Processing of
> Incoming Packets / Hash calculation for incoming packets / Supported/enabled
> hash types}.
> @@ -1442,10 +1457,9 @@ \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.
> +Field \field{unclassified_queue} contains the receive queue id of the receive
> virtqueue to place unclassified packets in.
> 
> -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 0-based indices of receive
> queue ids.
> 
>  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 +1469,7 @@ \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 receive queue
> ids of enabled queues.
> 
>  The number of entries in \field{indirection_table}
> (\field{indirection_table_mask} + 1) MUST be a power of two.
> 
> @@ -1467,8 +1481,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 receive queue id of the 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}
> 
> diff --git a/transport-ccw.tex b/transport-ccw.tex index c492cb9..bc05639
> 100644
> --- a/transport-ccw.tex
> +++ b/transport-ccw.tex
> @@ -544,7 +544,7 @@ \subsubsection{Guest->Host
> Notification}\label{sec:Virtio Transport Options / Vi  \end{tabular}
> 
>  When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, -the
> \field{Notification data} contains the Virtqueue number.
> +the \field{Notification data} contains the virtque index.
> 
>  When VIRTIO_F_NOTIFICATION_DATA has been negotiated,  the value has the
> following format:
> diff --git a/transport-pci.tex b/transport-pci.tex index b07a822..9b71c05 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -390,15 +390,16 @@ \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
> +        The driver will use this value to put it in the \field{vqn}
> + field
>          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} to the
> virtqueue index,
> +        that is the value in \field{queue\_select}. Some devices
>          may benefit from providing another value, for example an internal
> virtqueue
> -        identifier, or an internal offset related to the virtqueue number.
> +        identifier, or an internal offset related to the virtqueue index.
>          \end{note}
> 
>  \item[\field{queue_reset}]
> --
> 2.31.1
> 
> 
Most field changes are using index instead of number here in one patch.
No different than "number" patch.

And it still uses the vqn field name in the notification.
vqn is at least correct field name when CONFIG_DATA is not negotiated.
With 'index' usage, vqn field name is incorrect regardless of CONFIG_DATA, because its intents to hold index or something else. :)

So v10 has same or higher score as index.

Given everyone have come this far and also other patches using vqn, I think we should proceed to use "number".

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: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number

[Cornelia Huck]

On Thu, Mar 30 2023, Halil Pasic <pasic@linux.ibm.com> wrote:

> On Thu, 30 Mar 2023 00:23:33 +0300
> Parav Pandit <parav@nvidia.com> wrote:
>
>> 1. Currently, virtqueue is identified between driver and device
>> interchangeably using either number or 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 Number.
>> 
>> Solution:
>> a. Use virtqueue number description, and rename MMIO register as QueueSize.
>
> I'm in favor of replacing number with size where appropriate.
>
>> b. Replace virtqueue index with virtqueue number
>
> I don't see the benefit of replacing virtqueue index with virtqueue
> number.
>
> Currently virtqueue number is only used in the parts that describe
> notifications (Guest->Host), the rest of the spec uses virtqueue index.
>
> I argue that using a different term in that context than in the rest
> of the specification makes sense, because in the context of notifications
> the virtqueue isn't always identified by its index.
>
> More precisely: if VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated in the
> context of notifications the virtqueue is identified by the
> so called "queue_notify_data"; if VIRTIO_F_NOTIF_CONFIG_DATA has been
> negotiated in the context of notifications the virtqueue is identified by
> the virtqueue index (as usual, for example in queue_select, or in
> the ccws).
>
> As I've pointed out in my comment to patch 2, I believe replacing
> virtqueue index with virtqueue number is detrimental to clarity.
>
> Thus please find a counter-proposal below. If there is interest
> I can make a series out of it, and prettify it. If I can't convince
> you guys, then I will have to get used to vqn and virtqueue number.

I would generally prefer "index" as well, but there seemed to be a
strong sentiment that we should go with "number"... so, what *is* the
actual general sentiment? It's hard to say, but maybe most people are
fine with either?

>
> AFAIR the other problem with index was the RSS for virtio-net. But there
> we are currently heading down a direction of introducing a new
> abstraction. This approach avoids confusion around the term 'virtqueue
> index' as much as it avoids confusion around the term 'virtqueue nuber'.
>
>
>> c. RSS area of virtio net has inherited some logic, describe it
>> using abstract rss_rq_id.
>
> -------------------------8<--------------------------------------
> From: Halil Pasic <pasic@linux.ibm.com>
> Date: Thu, 30 Mar 2023 17:57:53 +0200
> Subject: [PATCH 1/1] content: clarify how virtques are identified
>
> Clarify how virtqueues are identified in the context of
> available notifications and in the context of RSS for
> virtio-net .
>
> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> ---
>  content.tex                      | 15 ++++++++++-----
>  device-types/net/description.tex | 30 ++++++++++++++++++++++--------
>  transport-ccw.tex                |  2 +-
>  transport-pci.tex                |  7 ++++---
>  4 files changed, 37 insertions(+), 17 deletions(-)

(...)

> +struct rss_rq_id {
> +   le16 value; /* virtqueue index divided by two */
> +};
> +
>  struct virtio_net_rss_config {
>      le32 hash_types;
>      le16 indirection_table_mask;
> -    le16 unclassified_queue;
> -    le16 indirection_table[indirection_table_length];
> +    struct rss_rq_id unclassified_queue;
> +    struct rss_rq_id indirection_table[indirection_table_length];
>      le16 max_tx_vq;
>      u8 hash_key_length;
>      u8 hash_key_data[hash_key_length];
>  };
>  \end{lstlisting}
> +
> +The type struct rss\_rq\_id is introduced to better distinguish receive queue
> +ids form other integral fields.
> +
> +A receive queue id is only defined for receive queues, as the virtqueue index
> +of the receive virtqueue divided by two (the virtqueue index of a receive
> +queue is always even). For example receiveq4 is identified by the virtqueue
> +index 6 and the receive queue id 3.

FWIW, I think this is much easier to understand.


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 v10 8/8] virtio-net: Describe RSS using rss rq id

[Parav Pandit]

> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Thursday, March 30, 2023 5:17 AM
> 
> On Thu, Mar 30 2023, Parav Pandit <parav@nvidia.com> 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.
> 
> This description is a bit confusing (and you renamed rq_handle as well.) Does
> 
> "The content of the indirection table and unclassified_queue were originally
> described based on mathematical operations. In order to make it easier to
> understand and to avoid intermixing the array index with the virtqueue
> number, introduce a structure rss_rq_id (RSS receive queue
> ID) and use it to describe the unclassified_queue and indirection_table fields."
> 
> capture the intent correctly?
> 
Yep. Will use it.
I missed to update the commit log.
Will roll v11.

> >
> > As part of it, have the example that uses non zero virtqueue
> 
> "have the example use a non-zero..." ?
:)
Ok.

> > -Field \field{unclassified_queue} contains the 0-based index of -the
> > receive virtqueue to place unclassified packets in. Index 0 corresponds to
> receiveq1.
> > +\field{rss_rq_id} is a receive virtqueue id. \field{vqn_1_16}
> > +consists of bits 1 to 16 of a vq number. For example, a
> > +\field{vqn_1_16} value of 3 corresponds to vq number 6, which maps to
> > +receiveq4.
> > +
> > +Field \field{unclassified_queue} contains the receive virtqueue in
> > +which to place unclassified packets.
> >
> > -Field \field{indirection_table} contains an array of 0-based indices of receive
> virtqueues. Index 0 corresponds to receiveq1.
> > +Field \field{indirection_table} is an array of receive virtqueues.
> 
> "an array of receive virtqueues identified via their rss_rq_id" ?
No need to make it this verbose. It is evident from the definition itself.

> 
> >
> >  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 +1465,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
> > +enabled receive virtqueues.
> 
> "only with rss_rq_id references to enabled receive virtqueues" ?
> 
In this field and other fields, we just refer to the receive virtqueues as rss_rq_id parent structure itself is describing what it is.
Hence, we omit re-iterating rss_rq_id at multiple places.

> >
> >  The number of entries in \field{indirection_table}
> (\field{indirection_table_mask} + 1) MUST be a power of two.
> >
> > @@ -1468,7 +1479,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 the
> > +destination receive virtqueue.
> 
> Ok, you remove the line here anyway, so please just ignore my suggestion for
> the previous patch.
> 
Ok.
Will send v11 with updated commit log.

> >  \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}


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: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number

[Halil Pasic]

On Thu, 30 Mar 2023 19:13:47 +0000
Parav Pandit <parav@nvidia.com> wrote:

> > From: Halil Pasic <pasic@linux.ibm.com>
> > Sent: Thursday, March 30, 2023 1:11 PM  
> 
> > Currently virtqueue number is only used in the parts that describe
> > notifications (Guest->Host), the rest of the spec uses virtqueue index.
> >
> > I argue that using a different term in that context than in the rest of the
> > specification makes sense, because in the context of notifications the virtqueue
> > isn't always identified by its index.  
> Using single term as number is possible, so lets use single term. 
> 

I don't understand. In my opinion we should use a single term for
a particular thing, but we should avoid using the same term for
two different things.

One thing is the zero based index of the virtqueue, for that
unfortunately we currently have multiple terms: virtqueue index
and virtqueue nubmer. To remove ourselves from the index vs number
discussion let us call it "X".

Another thing is the what you below propose to call vq_identifier, but
for the same reason as above let us briefly call it Y. "Y" may hold
an "X" or a "vq_notification_data" to use your term -- we both agree
on that so, I believe we should both agree that "X" and "Y" are distinct
things an need distinct and easy to distinguish names.

What we currently have is:
* when we mean "Y" we write "virtqueue number" or "vqn", 
* and when we mean "X" we write either "virtqueue index" or "virtqueue
  number"

Now let me pull the historic argument for consolidation on "virtqueue
index" rather than "virtqueue number".

Most of the occurrences of "virtqueue number" were introduced by
commit 4ca1311 ("VIRTIO_F_NOTIFICATION_DATA: extra data to devices"
and commit 2ff0d5c ("virtio-net: Add support for the flexible driver
notification structure.")

And I argue most of them stand where we mean "Y". Some of them
don't and that is bad.

Please have a look at the v1.0 version of the specification:
http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-370007
and search for "queue number" and "queue index". For "queue number"
you should get 1 hit and for "queue index" 8. All of them mean "X",
because "Y" was introduced later.

I argue, it is more sane to assume that the single "queue number"
occurrence is a back then quite harmless mistake (all we had is "X", so
we didn't have the "queue number" means "Y"), that to say the 8
occurrences of "queue index" are all mistakes.

Your approach to resolving the problem is: 
* change all concurrences of "queue index" (all stand for "X") to
  "queue number" 
* change the occurrences of "queue number" that stand for "Y" (that
  is most of them) to "vq_identifier"
* keep the occurrences of "queue number" that stand for "X" as is

Yes that way we can reach an in itself consistent state.

I argue that the following approach is better:
* keep all occurrences of "queue index" as is (all stand for "X")
* change he occurrences of "queue number" that stand for "X"
  to "virtqueue index"
* if we can find a name for "Y" better than "queue number"
  replace all remaining occurrences of "queue number" with
  that new name (I agree "queue number" is not a very good
  name for "Y")

Both solutions are equally consistent in themselves, but I
argue the latter is better because:
* it is more consistent with historic usage
* in my subjective opinion "queue index" is a better
name for "X" compared to "queue number"
* it requires fewer changes.

@Michael: Do you agree? Disagree?

Parav, your work is very much appreciated. I know, the messy
current state is not your fault at all. In fact it is to a certain
degree my fault. 

> > 
> > More precisely: if VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated in the
> > context of notifications the virtqueue is identified by the so called
> > "queue_notify_data"; 
> > if VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated in  
> 
> You missed "not" before negotiation. :)

Right.

Regard,
Halil

> 
> > the context of notifications the virtqueue is identified by the virtqueue index (as
> > usual, for example in queue_select, or in the ccws).
> > 
> > As I've pointed out in my comment to patch 2, I believe replacing virtqueue
> > index with virtqueue number is detrimental to clarity.

[..]

> > @@ -413,7 +416,9 @@ \section{Driver Notifications} \label{sec:Basic Facilities
> > of a Virtio Device /  the following information:
> > 
> >  \begin{description}
> > -\item [vqn] VQ number to be notified.
> > +\item [vqn] Identifies the virtqueueue to be notified. If
> > VIRTIO_F_NOTIF_CONFIG_DATA has been
> > +      negotiateted, the value of \field{vqn} is the notify data suplied by the
> > device  
> There is still vqn here.  So no better than vq number.
> To be truly clear, it should be renamed to vq_identifier, that either contains vqn or vq_notification_data.
> 
> As I replied I can take up that cleanup at later point if we find some motivation for CONFIG_DATA use.


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: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number

[Parav Pandit]

On 4/3/2023 9:36 PM, Halil Pasic wrote:
> On Thu, 30 Mar 2023 19:13:47 +0000
> Parav Pandit <parav@nvidia.com> wrote:
> 
>>> From: Halil Pasic <pasic@linux.ibm.com>
>>> Sent: Thursday, March 30, 2023 1:11 PM
>>
>>> Currently virtqueue number is only used in the parts that describe
>>> notifications (Guest->Host), the rest of the spec uses virtqueue index.
>>>
>>> I argue that using a different term in that context than in the rest of the
>>> specification makes sense, because in the context of notifications the virtqueue
>>> isn't always identified by its index.
>> Using single term as number is possible, so lets use single term.
>>
> 
> I don't understand. In my opinion we should use a single term for
> a particular thing, but we should avoid using the same term for
> two different things.
> 
Your below description clarifies that we both are talking same.
so I will pause to comment for above point.

> One thing is the zero based index of the virtqueue, for that
> unfortunately we currently have multiple terms: virtqueue index
> and virtqueue nubmer. To remove ourselves from the index vs number
> discussion let us call it "X".
>
There is nothing like a zero based index. A VQ can be any u16 _number_ 
in range of 0 to 65534.
Number can also start from zero. So zero based index = zero based number.

> Another thing is the what you below propose to call vq_identifier, but
> for the same reason as above let us briefly call it Y. "Y" may hold
> an "X" or a "vq_notification_data" to use your term -- we both agree
> on that so, I believe we should both agree that "X" and "Y" are distinct
> things an need distinct and easy to distinguish names.
> 
Yes. I proposed Y = vq_notify_identifier.
(like how we well named rss_rq_id).
Y = vqn when NOTIF_DATA is not negotiated (usually default).
Y = device supplied vq_notify_identifier when NOTIF_DATA is negotiated.

We should better rename the NOTIF_DATA to NOTIF_ID too. But I park that 
proposal for later.

> What we currently have is:
> * when we mean "Y" we write "virtqueue number" or "vqn", > * and when we mean "X" we write either "virtqueue index" or "virtqueue
>    number"
> 
> Now let me pull the historic argument for consolidation on "virtqueue
> index" rather than "virtqueue number".
> 
> Most of the occurrences of "virtqueue number" were introduced by
> commit 4ca1311 ("VIRTIO_F_NOTIFICATION_DATA: extra data to devices"
> and commit 2ff0d5c ("virtio-net: Add support for the flexible driver
> notification structure.")
> 
> And I argue most of them stand where we mean "Y". Some of them
> don't and that is bad.
> 
> Please have a look at the v1.0 version of the specification:
> http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-370007
> and search for "queue number" and "queue index". For "queue number"
> you should get 1 hit and for "queue index" 8. All of them mean "X",
> because "Y" was introduced later.
> 
> I argue, it is more sane to assume that the single "queue number"
> occurrence is a back then quite harmless mistake (all we had is "X", so
> we didn't have the "queue number" means "Y"), that to say the 8
> occurrences of "queue index" are all mistakes.
> 
> Your approach to resolving the problem is:
> * change all concurrences of "queue index" (all stand for "X") to
>    "queue number"
> * change the occurrences of "queue number" that stand for "Y" (that
>    is most of them) to "vq_identifier"
> * keep the occurrences of "queue number" that stand for "X" as is
> 

Once we rename the field vqn to vq_notify_id in the notification 
structure, we are out of the mess.

> Yes that way we can reach an in itself consistent state.
> 
> I argue that the following approach is better:
> * keep all occurrences of "queue index" as is (all stand for "X")
> * change he occurrences of "queue number" that stand for "X"
>    to "virtqueue index"
> * if we can find a name for "Y" better than "queue number"
>    replace all remaining occurrences of "queue number" with
>    that new name (I agree "queue number" is not a very good
>    name for "Y")
> 
vq_notify_id is a good name that takes us in sane place.

> Both solutions are equally consistent in themselves, but I
> argue the latter is better because:
> * it is more consistent with historic usage
Well index/vqn has mixed up anyway now.
For historic reason, I agree that index is right.
But it is too late now.
Comments have not come on time.

> * in my subjective opinion "queue index" is a better
> name for "X" compared to "queue number"
> * it requires fewer changes.
>
Unfortunately not.
Based on past agreements more patches have progress to use vqn.
It includes this patch, other patch you voted yes for vqn.
(interrupt moderation), more work that Michael did with AQ in the series 
v11.

If you sum up, changing all of it requires more changes.

> @Michael: Do you agree? Disagree?
> 
> Parav, your work is very much appreciated. I know, the messy
> current state is not your fault at all. In fact it is to a certain
> degree my fault.
>
I was really fine either way for index and number, one month back when I 
asked to reach consensus.

Most agreed with vqn, and engineers wrote v11 of this series, and 
multiple people reviewed.
Followed by other two patches.

I see few options.

Option_1:
1. I will send v12 of this series to use index all the places instead of 
number.
2. driver notification structure vqn to rename to vq_notify_id.
(after merging this series as follow up cleanup).
3. Once interrupt moderation series is merged, rename vqn to vq_index. 
(really vqn reads better even though history say vq index).
Because votes is completed and voting period ended for the important 
feature.
4. Michael send v12 with index.

Option_2:
1. Continue with the v11 of this series as vq number.
2. Continue with interrupt moderation series voted
3. Continue with Michael's work of AQ
4. Will do #2 of option_1.

Please decide at earliest.








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: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number

[Michael S. Tsirkin]

On Mon, Apr 03, 2023 at 10:57:35PM -0400, Parav Pandit wrote:
> > Both solutions are equally consistent in themselves, but I
> > argue the latter is better because:
> > * it is more consistent with historic usage
> Well index/vqn has mixed up anyway now.
> For historic reason, I agree that index is right.
> But it is too late now.
> Comments have not come on time.

Why, is there a deadline? Is this blocking some other feature? For
something that is supposedly a cleanup we might as well get this right.
If you guys both agree index is better, and at this point it sounds
convincing, why not do it? It's more or less a machanical replacement.
It's not like there's a lot of word smithing going on here except for
8/8 which really works with index just as well.

-- 
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: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number

[Michael S. Tsirkin]

On Fri, Mar 31, 2023 at 10:13:51AM +0200, Cornelia Huck wrote:
> On Thu, Mar 30 2023, Halil Pasic <pasic@linux.ibm.com> wrote:
> 
> > On Thu, 30 Mar 2023 00:23:33 +0300
> > Parav Pandit <parav@nvidia.com> wrote:
> >
> >> 1. Currently, virtqueue is identified between driver and device
> >> interchangeably using either number or 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 Number.
> >> 
> >> Solution:
> >> a. Use virtqueue number description, and rename MMIO register as QueueSize.
> >
> > I'm in favor of replacing number with size where appropriate.
> >
> >> b. Replace virtqueue index with virtqueue number
> >
> > I don't see the benefit of replacing virtqueue index with virtqueue
> > number.
> >
> > Currently virtqueue number is only used in the parts that describe
> > notifications (Guest->Host), the rest of the spec uses virtqueue index.
> >
> > I argue that using a different term in that context than in the rest
> > of the specification makes sense, because in the context of notifications
> > the virtqueue isn't always identified by its index.
> >
> > More precisely: if VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated in the
> > context of notifications the virtqueue is identified by the
> > so called "queue_notify_data"; if VIRTIO_F_NOTIF_CONFIG_DATA has been
> > negotiated in the context of notifications the virtqueue is identified by
> > the virtqueue index (as usual, for example in queue_select, or in
> > the ccws).
> >
> > As I've pointed out in my comment to patch 2, I believe replacing
> > virtqueue index with virtqueue number is detrimental to clarity.
> >
> > Thus please find a counter-proposal below. If there is interest
> > I can make a series out of it, and prettify it. If I can't convince
> > you guys, then I will have to get used to vqn and virtqueue number.
> 
> I would generally prefer "index" as well, but there seemed to be a
> strong sentiment that we should go with "number"... so, what *is* the
> actual general sentiment? It's hard to say, but maybe most people are
> fine with either?

If we really can't decide one way or another then I can run a ballot,
it's not hard.


> >
> > AFAIR the other problem with index was the RSS for virtio-net. But there
> > we are currently heading down a direction of introducing a new
> > abstraction. This approach avoids confusion around the term 'virtqueue
> > index' as much as it avoids confusion around the term 'virtqueue nuber'.
> >
> >
> >> c. RSS area of virtio net has inherited some logic, describe it
> >> using abstract rss_rq_id.
> >
> > -------------------------8<--------------------------------------
> > From: Halil Pasic <pasic@linux.ibm.com>
> > Date: Thu, 30 Mar 2023 17:57:53 +0200
> > Subject: [PATCH 1/1] content: clarify how virtques are identified
> >
> > Clarify how virtqueues are identified in the context of
> > available notifications and in the context of RSS for
> > virtio-net .
> >
> > Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> > ---
> >  content.tex                      | 15 ++++++++++-----
> >  device-types/net/description.tex | 30 ++++++++++++++++++++++--------
> >  transport-ccw.tex                |  2 +-
> >  transport-pci.tex                |  7 ++++---
> >  4 files changed, 37 insertions(+), 17 deletions(-)
> 
> (...)
> 
> > +struct rss_rq_id {
> > +   le16 value; /* virtqueue index divided by two */
> > +};
> > +
> >  struct virtio_net_rss_config {
> >      le32 hash_types;
> >      le16 indirection_table_mask;
> > -    le16 unclassified_queue;
> > -    le16 indirection_table[indirection_table_length];
> > +    struct rss_rq_id unclassified_queue;
> > +    struct rss_rq_id indirection_table[indirection_table_length];
> >      le16 max_tx_vq;
> >      u8 hash_key_length;
> >      u8 hash_key_data[hash_key_length];
> >  };
> >  \end{lstlisting}
> > +
> > +The type struct rss\_rq\_id is introduced to better distinguish receive queue
> > +ids form other integral fields.
> > +
> > +A receive queue id is only defined for receive queues, as the virtqueue index
> > +of the receive virtqueue divided by two (the virtqueue index of a receive
> > +queue is always even). For example receiveq4 is identified by the virtqueue
> > +index 6 and the receive queue id 3.
> 
> FWIW, I think this is much easier to understand.


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: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number

[Michael S. Tsirkin]

On Mon, Apr 03, 2023 at 10:57:35PM -0400, Parav Pandit wrote:
> 3. Once interrupt moderation series is merged, rename vqn to vq_index.
> (really vqn reads better even though history say vq index).
> Because votes is completed and voting period ended for the important
> feature.

interrupt moderation voting is still ongoing, isn't it?

-- 
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: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number

[Michael S. Tsirkin]

On Mon, Apr 03, 2023 at 10:57:35PM -0400, Parav Pandit wrote:
> I see few options.
> 
> Option_1:
> 1. I will send v12 of this series to use index all the places instead of
> number.
> 2. driver notification structure vqn to rename to vq_notify_id.
> (after merging this series as follow up cleanup).

We can pretend that vqn means "VQ Notification".  Backronims are fun.
In fact it is not always a vq index, sometimes it's a different value.
So maybe keeping vqn there makes sense.


> 3. Once interrupt moderation series is merged, rename vqn to vq_index.
> (really vqn reads better even though history say vq index).
> Because votes is completed and voting period ended for the important
> feature.

It's ongoing but sure, we can do a fix up on top.

> 4. Michael send v12 with index.

Of AQ? Sure.

> Option_2:
> 1. Continue with the v11 of this series as vq number.
> 2. Continue with interrupt moderation series voted
> 3. Continue with Michael's work of AQ
> 4. Will do #2 of option_1.
> 
> Please decide at earliest.
>

It looks like the sentiment is now going more option 1.
If you like post v12 and we'll see which gets more acks.

-- 
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/

Re: [virtio-comment] RE: [PATCH v10 8/8] virtio-net: Describe RSS using rss rq id

[Cornelia Huck]

On Mon, Apr 03 2023, Parav Pandit <parav@nvidia.com> wrote:

>> From: Cornelia Huck <cohuck@redhat.com>
>> Sent: Thursday, March 30, 2023 5:17 AM
>> > +Field \field{indirection_table} is an array of receive virtqueues.
>> 
>> "an array of receive virtqueues identified via their rss_rq_id" ?
> No need to make it this verbose. It is evident from the definition itself.
>
>> 
>> >
>> >  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 +1465,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
>> > +enabled receive virtqueues.
>> 
>> "only with rss_rq_id references to enabled receive virtqueues" ?
>> 
> In this field and other fields, we just refer to the receive virtqueues as rss_rq_id parent structure itself is describing what it is.
> Hence, we omit re-iterating rss_rq_id at multiple places.

Personally, I'd prefer to spell it out, as it is a specific way to refer
to a virtqueue.


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: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number

[Halil Pasic]

On Tue, 4 Apr 2023 03:07:26 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> > 3. Once interrupt moderation series is merged, rename vqn to vq_index.
> > (really vqn reads better even though history say vq index).
> > Because votes is completed and voting period ended for the important
> > feature.  
> 
> interrupt moderation voting is still ongoing, isn't it?

Yes it is. But not for very long. Close date is today. I
won't change my vote because of the index vs number discussion. I believe
we can include that patch in the cleanup if push comes to shove. But
I wouldn't have anything against that ballot getting withdrawn either.
I'm Cc-ing Heng Qi.

Regards,
Halil

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/

Re: [virtio-comment] Re: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number

[Cornelia Huck]

On Tue, Apr 04 2023, Halil Pasic <pasic@linux.ibm.com> wrote:

> On Tue, 4 Apr 2023 03:07:26 -0400
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
>> > 3. Once interrupt moderation series is merged, rename vqn to vq_index.
>> > (really vqn reads better even though history say vq index).
>> > Because votes is completed and voting period ended for the important
>> > feature.  
>> 
>> interrupt moderation voting is still ongoing, isn't it?
>
> Yes it is. But not for very long. Close date is today. I
> won't change my vote because of the index vs number discussion. I believe
> we can include that patch in the cleanup if push comes to shove. But
> I wouldn't have anything against that ballot getting withdrawn either.
> I'm Cc-ing Heng Qi.

...it has actually closed right now. But I don't really see including it
as a show stopper.


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/

RE: [virtio-comment] RE: [PATCH v10 8/8] virtio-net: Describe RSS using rss rq id

[Parav Pandit]

> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Tuesday, April 4, 2023 4:15 AM

> > In this field and other fields, we just refer to the receive virtqueues as
> rss_rq_id parent structure itself is describing what it is.
> > Hence, we omit re-iterating rss_rq_id at multiple places.
> 
> Personally, I'd prefer to spell it out, as it is a specific way to refer to a
> virtqueue.
That specific way is already covered without making it more verbose. So I will keep it as is, unless you have strong objection to it.

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/

Re: [virtio-comment] Re: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number

[Halil Pasic]

On Tue, 4 Apr 2023 02:33:15 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Apr 03, 2023 at 10:57:35PM -0400, Parav Pandit wrote:
> > > Both solutions are equally consistent in themselves, but I
> > > argue the latter is better because:
> > > * it is more consistent with historic usage  
> > Well index/vqn has mixed up anyway now.
> > For historic reason, I agree that index is right.
> > But it is too late now.
> > Comments have not come on time.  
> 
> Why, is there a deadline? Is this blocking some other feature? For
> something that is supposedly a cleanup we might as well get this right.
> If you guys both agree index is better, and at this point it sounds
> convincing, why not do it? It's more or less a machanical replacement.
> It's not like there's a lot of word smithing going on here except for
> 8/8 which really works with index just as well.

I agree.

Regards,
Halil

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/