Re: [PATCH] net: Revert tx queue length on partial failure in dev_qdisc_change_tx_queue_len()

From: Cong Wang
Date: Fri Jul 25 2025 - 13:47:48 EST


Hi Suchit,

On Wed, Jul 23, 2025 at 11:47:09PM +0530, Suchit K wrote:
> >
> > WRITE_ONCE() is missing.
> >
> > > + while (i >= 0) {
> > > + qdisc_change_tx_queue_len(dev, &dev->_tx[i]);
> >
> > What happens if one of these calls fails ?
> >
> > I think a fix will be more complicated...
>
> Hi Eric,
> Given that pfifo_fast_change_tx_queue_len is currently the only
> implementation of change_tx_queue_len, would it be reasonable to
> handle partial failures solely within pfifo_fast_change_tx_queue_len
> (which in turn leads to skb_array_resize_multiple_bh)? In other words,
> is it sufficient to modify only the underlying low level
> implementation of pfifo_fast_change_tx_queue_len for partial failures,
> given that it's the sole implementation of change_tx_queue_len?

Thanks for your patch.

As you noticed it is tricky to handle the failure elegantly here, which
was also the reason why I didn't do it. Did you observe any real issue?

To answer your question above: I am not sure if we can do it in pfifo
fast implementation since struct netdev_queue is not explicitly exposed to
the lower Qdisc.

On the other hand, although dev_qdisc_change_tx_queue_len() is generic,
it is only called for this very specific code path, so changing it won't
impact other code paths, IMHO.

Regards,
Cong Wang