netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* PROBLEM: nf_conntrack_events autodetect mode invalidates NETLINK_LISTEN_ALL_NSID netlink socket option
@ 2023-02-14 23:08 Bryce Kahle
  2023-02-15 10:02 ` Florian Westphal
  0 siblings, 1 reply; 8+ messages in thread
From: Bryce Kahle @ 2023-02-14 23:08 UTC (permalink / raw)
  To: fw; +Cc: netfilter-devel

nf_conntrack_events auto mode invalidates NETLINK_LISTEN_ALL_NSID
netlink socket option

commit 90d1daa45849f272b701f29d6ca88b24743c7553 introduced a
nf_conntrack_events=2 mode sysctl intended to avoid an allocation "as
long as no event listener is active in
the namespace".

The netlink socket option NETLINK_LISTEN_ALL_NSID allows a socket to
listen to events "from all network namespaces that have an nsid
assigned into the network namespace where the socket has been opened".

The effect of the above commit is that sockets in other network
namespaces (including the root network namespace) with
NETLINK_LISTEN_ALL_NSID, no longer receive events from any other
network namespace. Once you create a netlink socket in the same
network namespace as the event, then events from that network
namespace flow to all netlink sockets in all namespaces.

I attempted a workaround by setting nf_conntrack_events=1, but that
only applies in the current namespace. I believe this workaround has
no effect, because the default has been changed to 2 for all new
namespaces.

This affects kernels 5.19+. I have git bisected the kernel with a
reproducer to identify the commit above. I can publish the reproducer
on request.

Thanks,
Bryce Kahle
Datadog

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: PROBLEM: nf_conntrack_events autodetect mode invalidates NETLINK_LISTEN_ALL_NSID netlink socket option
  2023-02-14 23:08 PROBLEM: nf_conntrack_events autodetect mode invalidates NETLINK_LISTEN_ALL_NSID netlink socket option Bryce Kahle
@ 2023-02-15 10:02 ` Florian Westphal
  2023-02-16  1:05   ` Bryce Kahle
  0 siblings, 1 reply; 8+ messages in thread
From: Florian Westphal @ 2023-02-15 10:02 UTC (permalink / raw)
  To: Bryce Kahle; +Cc: fw, netfilter-devel

Bryce Kahle <bryce.kahle@datadoghq.com> wrote:
> This affects kernels 5.19+. I have git bisected the kernel with a
> reproducer to identify the commit above. I can publish the reproducer
> on request.

Reproducer would help, thanks.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: PROBLEM: nf_conntrack_events autodetect mode invalidates NETLINK_LISTEN_ALL_NSID netlink socket option
  2023-02-15 10:02 ` Florian Westphal
@ 2023-02-16  1:05   ` Bryce Kahle
  2023-02-16 15:18     ` Florian Westphal
  0 siblings, 1 reply; 8+ messages in thread
From: Bryce Kahle @ 2023-02-16  1:05 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

I have posted the reproducer at
https://github.com/brycekahle/netfilter-reproducer

On Wed, Feb 15, 2023 at 2:02 AM Florian Westphal <fw@strlen.de> wrote:
>
> Bryce Kahle <bryce.kahle@datadoghq.com> wrote:
> > This affects kernels 5.19+. I have git bisected the kernel with a
> > reproducer to identify the commit above. I can publish the reproducer
> > on request.
>
> Reproducer would help, thanks.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: PROBLEM: nf_conntrack_events autodetect mode invalidates NETLINK_LISTEN_ALL_NSID netlink socket option
  2023-02-16  1:05   ` Bryce Kahle
@ 2023-02-16 15:18     ` Florian Westphal
  2023-02-17  1:07       ` Bryce Kahle
  2023-02-20 14:43       ` Pablo Neira Ayuso
  0 siblings, 2 replies; 8+ messages in thread
From: Florian Westphal @ 2023-02-16 15:18 UTC (permalink / raw)
  To: Bryce Kahle; +Cc: Florian Westphal, netfilter-devel

Bryce Kahle <bryce.kahle@datadoghq.com> wrote:
> I have posted the reproducer at
> https://github.com/brycekahle/netfilter-reproducer

Thanks.  Resolving this in a backwards compatible way is
intrusive because at the time the ctnetlink subscription happens
nsid isn't set yet.

We'd need a new callback in netlink_kernel_cfg so that ctnetlink
can be informed about activation of 'allnet' option on an existing
socket.

We'd also need a new flag in netfilter/core.c for that and not in
ctnetlink because else we'd create an unwanted module dependency in
nf_conntrack.

I can think of 3 alternative solutions:
1. revert back to 'default 1'.
   I don't want to do that because for almost all conntrack use
   cases the extension allocation is unecessary.

2. Switch netns creation behaviour to enable the extensions if
   init_net has nf_conntrack_events=1.
   This would require user intervention, but probably fine.
   Downside is that this will be different from all the other
   settings.

3. Add a module param to conntrack to override the default
   setting.  We already have such params for accounting and timestamps.

I'd go with 3).  Bryce, would that work for you?

Pablo, whats your take on this?
If you prefer I can work on the new netlink_kernel_cfg callback
to see how intrusive it really is.

Breakage scenario is:

1. Parent netns opens ctnetlink event sk
2. Parent netns sets ALL_NSID flag
3. No events from child netns, because no ctnetlink
   event sockets were created in this netns and thus
   conntrack objects get no event extension.

Extending the existing bind callback doesn't work because
ALL_NSID flag is set after event subscription.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: PROBLEM: nf_conntrack_events autodetect mode invalidates NETLINK_LISTEN_ALL_NSID netlink socket option
  2023-02-16 15:18     ` Florian Westphal
@ 2023-02-17  1:07       ` Bryce Kahle
  2023-02-20 14:43       ` Pablo Neira Ayuso
  1 sibling, 0 replies; 8+ messages in thread
From: Bryce Kahle @ 2023-02-17  1:07 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

I'd prefer whichever option doesn't require changes on our end. I'm
not familiar with the ins/outs of a kernel module param, but we are
running on customer hosts so we often start far later than when the
kernel module was loaded.

On Thu, Feb 16, 2023 at 7:18 AM Florian Westphal <fw@strlen.de> wrote:
>
> Bryce Kahle <bryce.kahle@datadoghq.com> wrote:
> > I have posted the reproducer at
> > https://github.com/brycekahle/netfilter-reproducer
>
> Thanks.  Resolving this in a backwards compatible way is
> intrusive because at the time the ctnetlink subscription happens
> nsid isn't set yet.
>
> We'd need a new callback in netlink_kernel_cfg so that ctnetlink
> can be informed about activation of 'allnet' option on an existing
> socket.
>
> We'd also need a new flag in netfilter/core.c for that and not in
> ctnetlink because else we'd create an unwanted module dependency in
> nf_conntrack.
>
> I can think of 3 alternative solutions:
> 1. revert back to 'default 1'.
>    I don't want to do that because for almost all conntrack use
>    cases the extension allocation is unecessary.
>
> 2. Switch netns creation behaviour to enable the extensions if
>    init_net has nf_conntrack_events=1.
>    This would require user intervention, but probably fine.
>    Downside is that this will be different from all the other
>    settings.
>
> 3. Add a module param to conntrack to override the default
>    setting.  We already have such params for accounting and timestamps.
>
> I'd go with 3).  Bryce, would that work for you?
>
> Pablo, whats your take on this?
> If you prefer I can work on the new netlink_kernel_cfg callback
> to see how intrusive it really is.
>
> Breakage scenario is:
>
> 1. Parent netns opens ctnetlink event sk
> 2. Parent netns sets ALL_NSID flag
> 3. No events from child netns, because no ctnetlink
>    event sockets were created in this netns and thus
>    conntrack objects get no event extension.
>
> Extending the existing bind callback doesn't work because
> ALL_NSID flag is set after event subscription.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: PROBLEM: nf_conntrack_events autodetect mode invalidates NETLINK_LISTEN_ALL_NSID netlink socket option
  2023-02-16 15:18     ` Florian Westphal
  2023-02-17  1:07       ` Bryce Kahle
@ 2023-02-20 14:43       ` Pablo Neira Ayuso
  2023-02-27 16:07         ` Bryce Kahle
  1 sibling, 1 reply; 8+ messages in thread
From: Pablo Neira Ayuso @ 2023-02-20 14:43 UTC (permalink / raw)
  To: Florian Westphal; +Cc: Bryce Kahle, netfilter-devel

On Thu, Feb 16, 2023 at 04:18:22PM +0100, Florian Westphal wrote:
> Bryce Kahle <bryce.kahle@datadoghq.com> wrote:
> > I have posted the reproducer at
> > https://github.com/brycekahle/netfilter-reproducer
> 
> Thanks.  Resolving this in a backwards compatible way is
> intrusive because at the time the ctnetlink subscription happens
> nsid isn't set yet.
> 
> We'd need a new callback in netlink_kernel_cfg so that ctnetlink
> can be informed about activation of 'allnet' option on an existing
> socket.
> 
> We'd also need a new flag in netfilter/core.c for that and not in
> ctnetlink because else we'd create an unwanted module dependency in
> nf_conntrack.
> 
> I can think of 3 alternative solutions:
> 1. revert back to 'default 1'.
>    I don't want to do that because for almost all conntrack use
>    cases the extension allocation is unecessary.
> 
> 2. Switch netns creation behaviour to enable the extensions if
>    init_net has nf_conntrack_events=1.
>    This would require user intervention, but probably fine.
>    Downside is that this will be different from all the other
>    settings.
> 
> 3. Add a module param to conntrack to override the default
>    setting.  We already have such params for accounting and timestamps.
> 
> I'd go with 3).  Bryce, would that work for you?
>
> Pablo, whats your take on this?
>
> If you prefer I can work on the new netlink_kernel_cfg callback
> to see how intrusive it really is.
>
> Breakage scenario is:
> 
> 1. Parent netns opens ctnetlink event sk
> 2. Parent netns sets ALL_NSID flag
> 3. No events from child netns, because no ctnetlink
>    event sockets were created in this netns and thus
>    conntrack objects get no event extension.
> 
> Extending the existing bind callback doesn't work because
> ALL_NSID flag is set after event subscription.

I would prefer netlink_kernel_cfg .setsockopt callback as you suggest.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: PROBLEM: nf_conntrack_events autodetect mode invalidates NETLINK_LISTEN_ALL_NSID netlink socket option
  2023-02-20 14:43       ` Pablo Neira Ayuso
@ 2023-02-27 16:07         ` Bryce Kahle
  2023-02-27 16:13           ` Florian Westphal
  0 siblings, 1 reply; 8+ messages in thread
From: Bryce Kahle @ 2023-02-27 16:07 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel

I see there was a patch applied. Is there any chance of getting this
backported to the affected versions 5.19+, since it broke existing
functionality?

On Mon, Feb 20, 2023 at 6:43 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> On Thu, Feb 16, 2023 at 04:18:22PM +0100, Florian Westphal wrote:
> > Bryce Kahle <bryce.kahle@datadoghq.com> wrote:
> > > I have posted the reproducer at
> > > https://github.com/brycekahle/netfilter-reproducer
> >
> > Thanks.  Resolving this in a backwards compatible way is
> > intrusive because at the time the ctnetlink subscription happens
> > nsid isn't set yet.
> >
> > We'd need a new callback in netlink_kernel_cfg so that ctnetlink
> > can be informed about activation of 'allnet' option on an existing
> > socket.
> >
> > We'd also need a new flag in netfilter/core.c for that and not in
> > ctnetlink because else we'd create an unwanted module dependency in
> > nf_conntrack.
> >
> > I can think of 3 alternative solutions:
> > 1. revert back to 'default 1'.
> >    I don't want to do that because for almost all conntrack use
> >    cases the extension allocation is unecessary.
> >
> > 2. Switch netns creation behaviour to enable the extensions if
> >    init_net has nf_conntrack_events=1.
> >    This would require user intervention, but probably fine.
> >    Downside is that this will be different from all the other
> >    settings.
> >
> > 3. Add a module param to conntrack to override the default
> >    setting.  We already have such params for accounting and timestamps.
> >
> > I'd go with 3).  Bryce, would that work for you?
> >
> > Pablo, whats your take on this?
> >
> > If you prefer I can work on the new netlink_kernel_cfg callback
> > to see how intrusive it really is.
> >
> > Breakage scenario is:
> >
> > 1. Parent netns opens ctnetlink event sk
> > 2. Parent netns sets ALL_NSID flag
> > 3. No events from child netns, because no ctnetlink
> >    event sockets were created in this netns and thus
> >    conntrack objects get no event extension.
> >
> > Extending the existing bind callback doesn't work because
> > ALL_NSID flag is set after event subscription.
>
> I would prefer netlink_kernel_cfg .setsockopt callback as you suggest.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: PROBLEM: nf_conntrack_events autodetect mode invalidates NETLINK_LISTEN_ALL_NSID netlink socket option
  2023-02-27 16:07         ` Bryce Kahle
@ 2023-02-27 16:13           ` Florian Westphal
  0 siblings, 0 replies; 8+ messages in thread
From: Florian Westphal @ 2023-02-27 16:13 UTC (permalink / raw)
  To: Bryce Kahle; +Cc: Pablo Neira Ayuso, Florian Westphal, netfilter-devel

Bryce Kahle <bryce.kahle@datadoghq.com> wrote:
> I see there was a patch applied. Is there any chance of getting this
> backported to the affected versions 5.19+, since it broke existing
> functionality?

stable should pick this up automatically due to 'Fixes' tag.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-02-27 16:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-14 23:08 PROBLEM: nf_conntrack_events autodetect mode invalidates NETLINK_LISTEN_ALL_NSID netlink socket option Bryce Kahle
2023-02-15 10:02 ` Florian Westphal
2023-02-16  1:05   ` Bryce Kahle
2023-02-16 15:18     ` Florian Westphal
2023-02-17  1:07       ` Bryce Kahle
2023-02-20 14:43       ` Pablo Neira Ayuso
2023-02-27 16:07         ` Bryce Kahle
2023-02-27 16:13           ` Florian Westphal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).