On Tue, Oct 27, 2026 at 01:06:41PM +0100, Paolo Abeni wrote:
> On 10/22/26 20:26, Lianjie Wang wrote:
> > [...]
> > @@ -4976,6 +4977,10 @@ static int __virtnet_get_hw_stats(struct
> > virtnet_info *vi,
> > for (p = reply; p - reply < res_size; p += le16_to_cpu(hdr->size)) {
> > hdr = p;
> > qid = le16_to_cpu(hdr->vq_index);
> > + if ((qid < first_vq || qid > last_vq) &&
> > + !(enable_cvq && qid == vi->max_queue_pairs * 2))
> > + return -EINVAL;
>
> I think the above check could be simplified to:
> if (qid < first_vq || qid > last_vq)
> return -EINVAL;
>
> squashing the following in virtnet_get_hw_stats():
>
> - if (enable_cvq)
> + if (enable_cvq) {
> virtnet_make_stat_req(vi, ctx, req, vi->max_queue_pairs * 2,
> &j);
> + last_vq++;
> + }
>
> (likely with some meaningful comment).
Thanks for the review!
I looked into this, and I think last_vq + 1 matches the control vq
only when all queue pairs are in use. The control vq index is
max_queue_pairs * 2, while last_vq is curr_queue_pairs * 2 - 1 for
ethtool -S. After ethtool -L lowers the queue count, or when probe caps
curr_queue_pairs at num_online_cpus(), last_vq + 1 is the receive vq at
index curr_queue_pairs * 2, which is not in the request. The simplified
check would accept a record for it. Since the ethtool -S array has
slots for only curr_queue_pairs queue pairs, with the transmit slots at
its end, virtnet_fill_stats() would write the counters of that record
into the transmit slots, and can write past the end of the array.
Would you like me to keep the current check or change it in some
other way?
As for the comment, I would add the following above the check in v2:
/* The control vq comes after all max_queue_pairs pairs, so it
* exceeds last_vq + 1 when fewer pairs are in use.
*/
Thanks,
Lianjie Wang
