[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANn89iJiapfb3OULLv8FxQET4e-c7Kei_wyx2EYb7Wt_0qaAtw@mail.gmail.com>
Date: Wed, 26 Nov 2025 08:40:51 -0800
From: Eric Dumazet <edumazet@...gle.com>
To: Jamal Hadi Salim <jhs@...atatu.com>
Cc: Jakub Kicinski <kuba@...nel.org>, davem@...emloft.net, pabeni@...hat.com,
jiri@...nulli.us, xiyou.wangcong@...il.com, netdev@...r.kernel.org,
dcaratti@...hat.com
Subject: Re: [PATCH net-next 1/2] net/sched: act_mirred: Fix infinite loop
On Wed, Nov 26, 2025 at 8:26 AM Jamal Hadi Salim <jhs@...atatu.com> wrote:
>
> On Tue, Nov 25, 2025 at 11:20 AM Jamal Hadi Salim <jhs@...atatu.com> wrote:
> >
> > On Mon, Nov 24, 2025 at 5:51 PM Jakub Kicinski <kuba@...nel.org> wrote:
> > >
> > > On Mon, 24 Nov 2025 15:08:24 -0500 Jamal Hadi Salim wrote:
> > > > When doing multiport mirroring we dont detect infinite loops.
> > > >
> > > > Example (see the first accompanying tdc test):
> > > > packet showing up on port0 ingress mirred redirect --> port1 egress
> > > > packet showing up on port1 egress mirred redirect --> port0 ingress
> > > >
> > > > Example 2 (see the second accompanying tdc test)
> > > > port0 egress --> port1 ingress --> port0 egress
> > > >
> > > > Fix this by remembering the source dev where mirred ran as opposed to
> > > > destination/target dev
> > > >
> > > > Fixes: fe946a751d9b ("net/sched: act_mirred: add loop detection")
> > > > Signed-off-by: Jamal Hadi Salim <jhs@...atatu.com>
> > >
> > > Hm, this breaks net/fib_tests.sh:
> > >
> > > # 23.80 [+0.00] IPv4 rp_filter tests
> > > # 25.63 [+1.84] TEST: rp_filter passes local packets [FAIL]
> > > # 26.65 [+1.02] TEST: rp_filter passes loopback packets [FAIL]
> > >
> > > https://netdev-3.bots.linux.dev/vmksft-net/results/400301/10-fib-tests-sh/stdout
> > >
> > > Not making a statement on whether the fix itself is acceptable
> > > but if it is we gotta fix that test too..
> >
> > Sigh. I will look into it later.
> > Note: Fixing this (and the netem loop issue) would have been trivial
> > if we had those two skb ttl fields that were taken away.
> > The human hours spent trying to detect and prevent infinite loops!
> >
>
> Ok, I spent time on this and frankly cant find a way to fix the
> infinite loop that avoids adding _a lot more_ complexity.
> We need loop state to be associated with the skb. I will restore the
> two skb bits and test. From inspection, i see one bit free but i may
> be able to steal a bit from somewhere. I will post an RFC and at
> minimal that will start a discussion and maybe someone will come up
> with a better way of solving this.
>
> Eric, there's another issue as well involving example
> port0:egress->port0:egress - I have a patch but will post it later
> after some testing.
Adding bits for mirred in skb would be quite unfortunate.
Argument of 'we have available space, let's use/waste it' is not very appealing.
Do we really need to accept more than one mirred ?
What is a legitimate/realistic use for MIRRED_NEST_LIMIT == 4 ?
Powered by blists - more mailing lists