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

[Suchit K]

On Fri, 25 Jul 2025 at 23:17, Cong Wang <xiyou.wangcong@xxxxxxxxx> wrote:
>
> 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

Hi, Thanks for the feedback. I'll try to dig more into it and will
post a patch if I find a solution. Thanks once again.