On Mon, Mar 16, 2026 at 05:27:57PM +0530, Vishwanath Seshagiri wrote: > On 3/16/26 4:13 PM, Michael S. Tsirkin wrote: > > On Mon, Mar 16, 2026 at 05:56:20AM -0400, Michael S. Tsirkin wrote: > > > On Tue, Mar 10, 2026 at 11:31:04AM -0700, Vishwanath Seshagiri wrote: > > > > @@ -5857,7 +5863,7 @@ static int virtnet_xsk_pool_enable(struct > > > > net_device *dev, > > > > /* In big_packets mode, xdp cannot work, so there is no need to > > > > * initialize xsk of rq. > > > > */ > > > > - if (vi->big_packets && !vi->mergeable_rx_bufs) > > > > + if (!vi->rq[qid].page_pool) > > > > return -ENOENT; > > > > if (qid >= vi->curr_queue_pairs) > > > > > > > > > > > > It seems that a qid that exceeds curr_queue_pairs would previously get > > > -EINVAL and now gets -ENOENT. > > > > Or maybe this if (qid >= vi->curr_queue_pairs) is dead code? > > I looked at it some more and I can't find a path where this > > triggers. > > > > > Maybe reorder the checks: > > > > > > if (qid >= vi->curr_queue_pairs) > > > return -EINVAL; > > > > > > /* In big_packets mode, xdp cannot work, so there is no need to > > > * initialize xsk of rq. > > > */ > > > if (!vi->rq[qid].page_pool) > > > return -ENOENT; > > > > > > > > > Alternatively I think we can completely drop this chunk: we already seem > > > to init page_pull at all times except for big packets mode, so the > > > current code is fine I think. > > Yes, I agree qid >= curr_queue_pairs appears to be dead code. > xsk_reg_pool_at_qid() in the XSK core already validates > queue_id < max(real_num_rx_queues, real_num_tx_queues) before > ndo_bpf is called, and real_num_rx_queues == curr_queue_pairs is an > invariant in virtio_net. Both paths hold rtnl_lock so no race is > possible. That said, I'll adopt your suggested reorder for v12 to ensure > vi->rq[qid] isn't accessed before a bounds check, even if the check > is currently redundant.
I think we can just take this as-is, and do more cleanups on top. Acked-by: Michael S. Tsirkin <[email protected]> > > > > > > > > > -- > > > MST > >
