netdev - Re: [PATCH net 0/6] net/sched: Fix packet loops in mirred and netem

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAM0EoMkFMURPj3+gNOaqO60D4deeht2F3EZWZbmShjO+B4wJBA@mail.gmail.com>
Date: Thu, 15 Jan 2026 09:32:37 -0500
From: Jamal Hadi Salim <jhs@...atatu.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, 
	horms@...nel.org, andrew+netdev@...n.ch, netdev@...r.kernel.org, 
	xiyou.wangcong@...il.com, jiri@...nulli.us, victor@...atatu.com, 
	dcaratti@...hat.com, lariel@...dia.com, daniel@...earbox.net, 
	pablo@...filter.org, kadlec@...filter.org, fw@...len.de, phil@....cc, 
	netfilter-devel@...r.kernel.org, coreteam@...filter.org, 
	zyc199902@...omail.cn, lrGerlinde@...lfence.com, jschung2@...ton.me
Subject: Re: [PATCH net 0/6] net/sched: Fix packet loops in mirred and netem

On Thu, Jan 15, 2026 at 5:23 AM Paolo Abeni <pabeni@...hat.com> wrote:
>
> On 1/11/26 5:39 PM, Jamal Hadi Salim wrote:
> > We introduce a 2-bit global skb->ttl counter.Patch #1 describes how we puti
> > together those bits. Patches #2 and patch #5 use these bits.
> > I added Fixes tags to patch #1 in case it is useful for backporting.
> > Patch #3 and #4 revert William's earlier netem commits. Patch #6 introduces
> > tdc test cases.
>
> Generally speaking I think that a more self-encapsulated solution should
> be preferable.
>

I dont see a way to do that with mirred. I am more than happy if
someone else solves that issue or gives me an idea how to.

> I [mis?]understand that your main concern with Cong's series is the
> possible parent qlen corruption in case of duplication and the last
> iteration of such series includes a self-test for that, is there
> anything missing there?

i dont read the list when I am busy, but I will read emails when Cced
to me. I had not seen Cong's patches before yesterday.

But to answer your question, I presented a much simpler fix and more
importantly after looking at Cong's post i notice it changes a 20 year
old approach (which returned things to the root qdisc). William had
already pointed this to him. The simple change i posted doesn't
require that.
In any case if Stephen or you or Jakub want to push that change go
ahead - we'll wait to see what the bots find.

I am more interested in the mirred one because the current approach
has both loops and false positive(example claiming a loop when there
is none).

> The new sk_buff field looks a bit controversial. Adding such field
> opens/implies using it for other/all loop detection; a 2 bits counter
> will not be enough for that, and the struct sk_buff will increase for
> typical build otherwise.

My logic is: two bits is better than zero bits. More bits the better.
I am not sure i see sharing across the stack - and if we do hit that
situation, something will drop and we can debug.
At the moment I know of these two bugs - which are trivial to fix as
shown. I don't think it's fair to ask me to fix all potential (and
hypotheical) loops; i can fix them if someone shows an example setup.

> FTR I don't think that sk_buff the size increase for minimal config is
> very relevant, as most/all of the binary layout optimization and not
> thought for such build.

It is not really. Nobody turns off options that are ifdef just to say
"i need to save 1B".

cheers,
jamal