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

[Parav Pandit <parav@nvidia.com>]

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

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

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

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