netdev - Re: [PATCH] net: Revert tx queue length on partial failure in dev_qdisc_change_tx_queue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAO9wTFhC=cRvJQro1g2SXiq4Zx-Oiu3Q3c8stbfHSXCA2fi7Bw@mail.gmail.com>
Date: Sat, 26 Jul 2025 16:32:33 +0530
From: Suchit K <suchitkarunakaran@...il.com>
To: Cong Wang <xiyou.wangcong@...il.com>
Cc: Eric Dumazet <edumazet@...gle.com>, davem@...emloft.net, kuba@...nel.org, 
	pabeni@...hat.com, horms@...nel.org, jhs@...atatu.com, jiri@...nulli.us, 
	sdf@...ichev.me, kuniyu@...gle.com, aleksander.lobakin@...el.com, 
	netdev@...r.kernel.org, skhan@...uxfoundation.org, 
	linux-kernel-mentees@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: Revert tx queue length on partial failure in dev_qdisc_change_tx_queue_len()

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