On Fri, Mar 21, 2025 at 11:56 AM Akihiko Odaki <[email protected]> wrote:
>
> virtio-net uses the max_tx_vq field of struct virtio_net_rss_config to
> determine the number of queue pairs and emits an error message saying
> "Can't get queue_pairs". However, the field tells only about tx.
>
> Examine the indirection table to determine the number of queues required
> for rx, and correct the name of field in the error message, clarifying
> its correct semantics.
>
> Signed-off-by: Akihiko Odaki <[email protected]>
> ---
> hw/net/virtio-net.c | 25 +++++++++++++++----------
> 1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index de87cfadffe1..d9ab9e1eb74d 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1450,23 +1450,28 @@ static uint16_t virtio_net_handle_rss(VirtIONet *n,
> err_value = (uint32_t)s;
> goto error;
> }
> - for (i = 0; i < n->rss_data.indirections_len; ++i) {
> - uint16_t val = n->rss_data.indirections_table[i];
> - n->rss_data.indirections_table[i] = virtio_lduw_p(vdev, &val);
> - }
> offset += size_get;
> size_get = sizeof(temp);
> s = iov_to_buf(iov, iov_cnt, offset, &temp, size_get);
> if (s != size_get) {
> - err_msg = "Can't get queue_pairs";
> + err_msg = "Can't get max_tx_vq";
> err_value = (uint32_t)s;
> goto error;
> }
> - queue_pairs = do_rss ? virtio_lduw_p(vdev, &temp.us) :
> n->curr_queue_pairs;
> - if (queue_pairs == 0 || queue_pairs > n->max_queue_pairs) {
> - err_msg = "Invalid number of queue_pairs";
> - err_value = queue_pairs;
> - goto error;
> + if (do_rss) {
> + queue_pairs = virtio_lduw_p(vdev, &temp.us);
> + for (i = 0; i < n->rss_data.indirections_len; ++i) {
> + uint16_t val = n->rss_data.indirections_table[i];
> + n->rss_data.indirections_table[i] = virtio_lduw_p(vdev, &val);
> + queue_pairs = MAX(queue_pairs,
> n->rss_data.indirections_table[i]);
> + }
I think this change will create a problem rather than improve something.
curr_queue_pairs is used in virtio_net_can_receive.
Let's say the device has 4 queues and 4 CPUs.
If the OS currently configures the RSS for first 2 queues only the
curr_queue_pairs becomes 2.
The packets that come to queues 2 and 3 will be dropped, see
virtio_net_receive_rcu and virtio_net_can_receive
IMO the curr_queue_pairs should not be changed after it was set by
VIRTIO_NET_CTRL_MQ / VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET
> + if (queue_pairs == 0 || queue_pairs > n->max_queue_pairs) {
> + err_msg = "Invalid number of queue_pairs";
> + err_value = queue_pairs;
> + goto error;
> + }
> + } else {
> + queue_pairs = n->curr_queue_pairs;
> }
> if (temp.b > VIRTIO_NET_RSS_MAX_KEY_SIZE) {
> err_msg = "Invalid key size";
>
> ---
> base-commit: 825b96dbcee23d134b691fc75618b59c5f53da32
> change-id: 20250321-vq-87aff4f531bf
>
> Best regards,
> --
> Akihiko Odaki <[email protected]>
>
