Re: [dpdk-dev] [PATCH v5 1/5] net/virtio: add initial RSS support

[Maxime Coquelin <maxime.coquelin@redhat.com>]

Hi Yuri,

On 10/27/21 16:45, Yuri Benditovich wrote:
> 
> 
> On Wed, Oct 27, 2021 at 1:55 PM Maxime Coquelin 
> <maxime.coquelin@redhat.com <mailto:maxime.coquelin@redhat.com>> wrote:
> 
>     Hi,
> 
>     On 10/19/21 11:37, Andrew Rybchenko wrote:
>      > Hi Maxime,
>      >
>      > On 10/19/21 12:22 PM, Maxime Coquelin wrote:
>      >> Hi Andrew,
>      >>
>      >> On 10/19/21 09:30, Andrew Rybchenko wrote:
>      >>> On 10/18/21 1:20 PM, Maxime Coquelin wrote:
>      >>>> Provide the capability to update the hash key, hash types
>      >>>> and RETA table on the fly (without needing to stop/start
>      >>>> the device). However, the key length and the number of RETA
>      >>>> entries are fixed to 40B and 128 entries respectively. This
>      >>>> is done in order to simplify the design, but may be
>      >>>> revisited later as the Virtio spec provides this
>      >>>> flexibility.
>      >>>>
>      >>>> Note that only VIRTIO_NET_F_RSS support is implemented,
>      >>>> VIRTIO_NET_F_HASH_REPORT, which would enable reporting the
>      >>>> packet RSS hash calculated by the device into mbuf.rss, is
>      >>>> not yet supported.
>      >>>>
>      >>>> Regarding the default RSS configuration, it has been
>      >>>> chosen to use the default Intel ixgbe key as default key,
>      >>>> and default RETA is a simple modulo between the hash and
>      >>>> the number of Rx queues.
>      >>>>
>      >>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com
>     <mailto:maxime.coquelin@redhat.com>>
>      >
>      > [snip]
>      >
>      >>>> +    rss.unclassified_queue = 0;
>      >>>> +    memcpy(rss.indirection_table, hw->rss_reta,
>      >>>> VIRTIO_NET_RSS_RETA_SIZE * sizeof(uint16_t));
>      >>>> +    rss.max_tx_vq = nb_queues;
>      >>>
>      >>> Is it guaranteed that driver is configured with equal number
>      >>> of Rx and Tx queues? Or is it not a problem otherwise?
>      >>
>      >> Virtio networking devices works with queue pairs.
>      >
>      > But it seems to me that I still can configure just 1 Rx queue
>      > and many Tx queues. Basically just non equal.
>      > The line is suspicious since I'd expect nb_queues to be
>      > a number of Rx queues in the function, but we set max_tx_vq
>      > count here.
> 
>     The Virtio spec says:
>     "
>     A driver sets max_tx_vq to inform a device how many transmit
>     virtqueues it may use (transmitq1. . .transmitq max_tx_vq).
>     "
> 
>     But looking at Qemu side, its value is interpreted as the number of
>     queue pairs setup by the driver, same as is handled virtqueue_pairs of
>     struct virtio_net_ctrl_mq when RSS is not supported.
> 
>     In this patch we are compatible with what is done in Qemu, and what is
>     done for multiqueue when RSS is not enabled.
> 
>     I don't get why the spec talks about transmit queues, Yan & Yuri, any
>     idea?
> 
> 
> Indeed QEMU reference code uses the max_tx_vq as a number of queue pairs 
> the same way it uses a parameter of _MQ command.
> Mainly this is related to vhost start flow which assumes that there is 
> some number of ready vq pairs.
> When the driver sets max_tx_vq it guarantees that it does not use more 
> than max_tx_vq TX queues.
> Actual RX queues that will be used can be taken from the indirection 
> table which contains indices of RX queues.

Thanks for the quick reply.
Then setting to MAX(nb_rx_queue, nb_tx_queue) is compliant with both the
spec and the Qemu implementation.

Maxime