On Mon, Feb 16, 2026 at 02:44:57PM +0000, Kohei Enju wrote:
> On Mon, 16 Feb 2026 13:19:09 +0000, Simon Horman wrote:
>
> > > @@ -345,19 +344,19 @@ static void iavf_get_ethtool_stats(struct
> > > net_device *netdev,
> > > iavf_add_ethtool_stats(&data, adapter, iavf_gstrings_stats);
> > >
> > > rcu_read_lock();
> > > - /* As num_active_queues describe both tx and rx queues, we can use
> > > - * it to iterate over rings' stats.
> > > + /* Use num_tx_queues to report stats for the maximum number of queues.
> > > + * Queues beyond num_active_queues will report zero.
> > > */
> > > - for (i = 0; i < adapter->num_active_queues; i++) {
> > > - struct iavf_ring *ring;
> > > + for (i = 0; i < netdev->num_tx_queues; i++) {
> > > + struct iavf_ring *tx_ring = NULL, *rx_ring = NULL;
> > >
> > > - /* Tx rings stats */
> > > - ring = &adapter->tx_rings[i];
> > > - iavf_add_queue_stats(&data, ring);
> > > + if (i < adapter->num_active_queues) {
> >
> > Hi Enju-san,
>
> Hi Horman-san, thank you for reviewing!
>
> >
> > If I understand things correctly, in the scenario described in the patch
> > description, num_active_queues will be 8 here.
>
> Yes.
>
> >
> > Won't that result in an overflow?
>
> I think it won't overflow.
>
> In Thread 1, iavf_set_channels(), which allocates {tx,rx}_rings and
> updates num_active_queues, is executed under netdev lock. Therefore
> Thread 3, which is also executed under the netdev lock, sees updated
> num_active_queues and {tx,rx}_rings.
>
> The scenario flow lacked netdev_(un)lock, my bad.
>
> Thread 1 (ethtool -L) Thread 2 (work) Thread 3 (ethtool -S)
> netdev_lock()
> iavf_set_channels()
> ...
> iavf_alloc_queues()
> -> alloc {tx,rx}_rings
> -> num_active_queues = 8
> iavf_schedule_finish_config()
> netdev_unlock()
> netdev_lock()
> iavf_get_sset_count()
> real_num_tx_queues: 1
> -> buffer for 1 queue
> iavf_get_ethtool_stats()
> num_active_queues: 8
> -> out-of-bounds!
> netdev_unlock()
> iavf_finish_config()
> netdev_lock()
> -> real_num_tx_queues = 8
> netdev_unlock()
Thanks, and sorry for missing that the first time around.
With that clarified in my mind this looks good to me.
Reviewed-by: Simon Horman <[email protected]>
