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

[Parav Pandit <parav@nvidia.com>]

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