On Tue, Mar 15, 2022 at 10:41:21PM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> > > add new net.netfilter.nf_conntrack_events default mode: 2, autodetect.
> >
> > Probably the sysctl entry does not make any sense anymore if you can
> > autodetect when there is a listener?
>
> Hmmm, did not consider that. I *think* we still want to allow to
> disable the feature because of xt_CT/nft_ct.
>
> Someone might have nf_conntrack_events=0 and tehy could be using
> explicit configuration via templates (and then expect that only
> those flows that matched a '-j CT' rule generate events.
Maybe could you bump the ctnetlink_listeners counter when -j CT is
used with event filtering?
> > > in nfnetlink bind, inc pernet counter when event group is bound.
> > > in nfnetlink unbind, dec pernet counter when event group is unbound.
> >
> > So you keep one counter per netlink group in netns area?
>
> Rough sketch (doesn't compile/apply):
>
> +++ b/net/netfilter/nfnetlink.c
> @@ -45,6 +45,7 @@ MODULE_DESCRIPTION("Netfilter messages via netlink socket");
> static unsigned int nfnetlink_pernet_id __read_mostly;
>
> struct nfnl_net {
> + unsigned int ctnetlink_listeners;
> struct sock *nfnl;
>
> static int nfnetlink_bind(struct net *net, int group)
> {
> const struct nfnetlink_subsystem *ss;
> @@ -691,11 +691,47 @@ static int nfnetlink_bind(struct net *net, int group)
> if (!ss)
> request_module_nowait("nfnetlink-subsys-%d", type);
> +
> + if (type == NFNL_SUBSYS_CTNETLINK) {
> + struct nfnl_net *nfnlnet = nfnl_pernet(net);
> +
> + nfnl_lock(NFNL_SUBSYS_CTNETLINK);
> + nfnlnet->ctnetlink_listeners++;
> + if (nfnlnet->ctnetlink_listeners == 1)
> + net->ct.ctnetlink_has_listener = true;
> + nfnl_unlock(NFNL_SUBSYS_CTNETLINK);
>
> and then check 'net->ct.ctnetlink_has_listener' when allocating
> a new conntrack.
LGTM.
> > > a prototype.
> >
> > There is also setsockopt() to subscribe to netlink groups, you might
> > need to extend netlink_kernel_cfg to deal with this case too?
>
> afaics the netlink_bind callback is invoked for subscriptions via setsockopt too,
> so that angle shouw be covered.
good.
