netdev - Re: [PATCH net v2 1/2] net/sched: act_mirred: use the backlog for mirred ingress

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: <789d5cc3c38b320d61867290115acafb060ca752.camel@redhat.com>
Date: Thu, 15 Feb 2024 13:56:12 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Jakub Kicinski <kuba@...nel.org>, Jiri Pirko <jiri@...nulli.us>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
 Marcelo Ricardo Leitner <marcelo.leitner@...il.com>, Davide Caratti
 <dcaratti@...hat.com>, jhs@...atatu.com,  xiyou.wangcong@...il.com,
 shmulik.ladkani@...il.com
Subject: Re: [PATCH net v2 1/2] net/sched: act_mirred: use the backlog for
 mirred ingress

On Wed, 2024-02-14 at 07:04 -0800, Jakub Kicinski wrote:
> On Wed, 14 Feb 2024 09:51:00 +0100 Jiri Pirko wrote:
> > Wed, Feb 14, 2024 at 04:38:47AM CET, kuba@...nel.org wrote:
> > > The test Davide added in commit ca22da2fbd69 ("act_mirred: use the backlog
> > > for nested calls to mirred ingress") hangs our testing VMs every 10 or so
> > > runs, with the familiar tcp_v4_rcv -> tcp_v4_rcv deadlock reported by
> > > lockdep.
> > > 
> > > In the past there was a concern that the backlog indirection will
> > > lead to loss of error reporting / less accurate stats. But the current
> > > workaround does not seem to address the issue.  
> > 
> > Okay, so what the patch actually should change to fix this?
> 
> Sorry I'm not sure what you're asking.
> 
> We can't redirect traffic back to ourselves because we can end up
> trying to take the socket lock for a socket that is generating
> the packet.
> 
> Or are you asking how we can get the stats from the packet
> asynchronously? We could build a local async scheme but I'd rather
> not go there unless someone actually cares about these stats.

I *guess* Jiri is suggesting to expand the commit message describing
how the fix implemented by this patch works.

@Jiri, feel free to provide the actual correct interpretation :)

Cheers,

Paolo