[V6] netfilter: netns nf_conntrack: per-netns net.netfilter.nf_conntrack_max sysctl

From: lvxiafei <lvxiafei@sensetime.com>

Support net.netfilter.nf_conntrack_max settings per
netns, net.netfilter.nf_conntrack_max is used to more
flexibly limit the ct_count in different netns. The
default value belongs to the init_net limit.

After net.netfilter.nf_conntrack_max is set in different
netns, it is not allowed to be greater than the init_net
limit when working.

Signed-off-by: lvxiafei <lvxiafei@sensetime.com>
---
 .../networking/nf_conntrack-sysctl.rst        | 29 +++++++++++++++----
 include/net/netfilter/nf_conntrack.h          | 12 +++++++-
 include/net/netns/conntrack.h                 |  1 +
 net/netfilter/nf_conntrack_core.c             | 19 ++++++------
 net/netfilter/nf_conntrack_netlink.c          |  2 +-
 net/netfilter/nf_conntrack_standalone.c       |  7 +++--
 6 files changed, 50 insertions(+), 20 deletions(-)

Hello!

My name is Xiafei Lv, and I sent patch v6 on April 15, 2025. As of today, it
has been nearly two weeks, and I have not received any feedback. However, the
patch has been tested successfully, and you can view the details at this link:
https://patchwork.kernel.org/project/netdevbpf/patch/20250415090834.24882-1-xiafei_xupt@163.com/

I understand that kernel development is busy, and maintainers may have many
things to deal with and may not have time to review my patch. With this in mind,
I would like to politely ask what steps I should take next? If you have any
questions about my patch or need additional information from me, please feel
free to let me know and I will cooperate as soon as possible.

Thank you very much for taking the time to deal with my request, and I look
forward to your response.

Best wishes!

Xiafei Lv

Hi!

On Sun, Apr 27, 2025 at 04:14:20PM +0800, lvxiafei wrote:
> Hello!
> 
> My name is Xiafei Lv, and I sent patch v6 on April 15, 2025. As of today, it
> has been nearly two weeks, and I have not received any feedback. However, the
> patch has been tested successfully, and you can view the details at this link:
> https://patchwork.kernel.org/project/netdevbpf/patch/20250415090834.24882-1-xiafei_xupt@163.com/
>
> I understand that kernel development is busy, and maintainers may have many
> things to deal with and may not have time to review my patch. With this in mind,
> I would like to politely ask what steps I should take next? If you have any
> questions about my patch or need additional information from me, please feel
> free to let me know and I will cooperate as soon as possible.

You just have to wait. We will get back to you with more questions if
needed.

> Thank you very much for taking the time to deal with my request, and I look
> forward to your response.

Thanks.

On Tue, Apr 15, 2025 at 05:08:34PM +0800, lvxiafei wrote:
> diff --git a/Documentation/networking/nf_conntrack-sysctl.rst b/Documentation/networking/nf_conntrack-sysctl.rst
> index 238b66d0e059..6e7f17f5959a 100644
> --- a/Documentation/networking/nf_conntrack-sysctl.rst
> +++ b/Documentation/networking/nf_conntrack-sysctl.rst
> @@ -93,12 +93,29 @@ nf_conntrack_log_invalid - INTEGER
>  	Log invalid packets of a type specified by value.
>  
>  nf_conntrack_max - INTEGER
> -        Maximum number of allowed connection tracking entries. This value is set
> -        to nf_conntrack_buckets by default.
> -        Note that connection tracking entries are added to the table twice -- once
> -        for the original direction and once for the reply direction (i.e., with
> -        the reversed address). This means that with default settings a maxed-out
> -        table will have a average hash chain length of 2, not 1.
> +    - 0 - disabled (unlimited)

unlimited is too much, and the number of buckets is also global, how
does this work?

Is your goal to allow a netns to have larger table than netns? There
should be a cap for this.

> +    - not 0 - enabled
> +
> +    Maximum number of allowed connection tracking entries per netns. This value
> +    is set to nf_conntrack_buckets by default.
> +
> +    Note that connection tracking entries are added to the table twice -- once
> +    for the original direction and once for the reply direction (i.e., with
> +    the reversed address). This means that with default settings a maxed-out
> +    table will have a average hash chain length of 2, not 1.
> +
> +    The limit of other netns cannot be greater than init_net netns.
> +    +----------------+-------------+----------------+
> +    | init_net netns | other netns | limit behavior |
> +    +----------------+-------------+----------------+
> +    | 0              | 0           | unlimited      |
> +    +----------------+-------------+----------------+
> +    | 0              | not 0       | other          |
> +    +----------------+-------------+----------------+
> +    | not 0          | 0           | init_net       |
> +    +----------------+-------------+----------------+
> +    | not 0          | not 0       | min            |
> +    +----------------+-------------+----------------+
>  
>  nf_conntrack_tcp_be_liberal - BOOLEAN
>  	- 0 - disabled (default)
> diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h
> index 3f02a45773e8..594439b2f5a1 100644
> --- a/include/net/netfilter/nf_conntrack.h
> +++ b/include/net/netfilter/nf_conntrack.h
> @@ -320,7 +320,6 @@ int nf_conntrack_hash_resize(unsigned int hashsize);
>  extern struct hlist_nulls_head *nf_conntrack_hash;
>  extern unsigned int nf_conntrack_htable_size;
>  extern seqcount_spinlock_t nf_conntrack_generation;
> -extern unsigned int nf_conntrack_max;
>  
>  /* must be called with rcu read lock held */
>  static inline void
> @@ -360,6 +359,17 @@ static inline struct nf_conntrack_net *nf_ct_pernet(const struct net *net)
>  	return net_generic(net, nf_conntrack_net_id);
>  }
>  
> +static inline unsigned int nf_conntrack_max(const struct net *net)
> +{
> +#if IS_ENABLED(CONFIG_NF_CONNTRACK)
> +	return likely(init_net.ct.sysctl_max && net->ct.sysctl_max) ?
> +	    min(init_net.ct.sysctl_max, net->ct.sysctl_max) :
> +	    max(init_net.ct.sysctl_max, net->ct.sysctl_max);
> +#else
> +	return 0;
> +#endif
> +}
> +
>  int nf_ct_skb_network_trim(struct sk_buff *skb, int family);
>  int nf_ct_handle_fragments(struct net *net, struct sk_buff *skb,
>  			   u16 zone, u8 family, u8 *proto, u16 *mru);
> diff --git a/include/net/netns/conntrack.h b/include/net/netns/conntrack.h
> index bae914815aa3..d3fcd0b92b2d 100644
> --- a/include/net/netns/conntrack.h
> +++ b/include/net/netns/conntrack.h
> @@ -102,6 +102,7 @@ struct netns_ct {
>  	u8			sysctl_acct;
>  	u8			sysctl_tstamp;
>  	u8			sysctl_checksum;
> +	unsigned int		sysctl_max;
>  
>  	struct ip_conntrack_stat __percpu *stat;
>  	struct nf_ct_event_notifier __rcu *nf_conntrack_event_cb;
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index 7f8b245e287a..a738564923ec 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -202,8 +202,6 @@ static void nf_conntrack_all_unlock(void)
>  unsigned int nf_conntrack_htable_size __read_mostly;
>  EXPORT_SYMBOL_GPL(nf_conntrack_htable_size);
>  
> -unsigned int nf_conntrack_max __read_mostly;
> -EXPORT_SYMBOL_GPL(nf_conntrack_max);
>  seqcount_spinlock_t nf_conntrack_generation __read_mostly;
>  static siphash_aligned_key_t nf_conntrack_hash_rnd;
>  
> @@ -1498,7 +1496,7 @@ static bool gc_worker_can_early_drop(const struct nf_conn *ct)
>  
>  static void gc_worker(struct work_struct *work)
>  {
> -	unsigned int i, hashsz, nf_conntrack_max95 = 0;
> +	unsigned int i, hashsz;
>  	u32 end_time, start_time = nfct_time_stamp;
>  	struct conntrack_gc_work *gc_work;
>  	unsigned int expired_count = 0;
> @@ -1509,8 +1507,6 @@ static void gc_worker(struct work_struct *work)
>  	gc_work = container_of(work, struct conntrack_gc_work, dwork.work);
>  
>  	i = gc_work->next_bucket;
> -	if (gc_work->early_drop)
> -		nf_conntrack_max95 = nf_conntrack_max / 100u * 95u;
>  
>  	if (i == 0) {
>  		gc_work->avg_timeout = GC_SCAN_INTERVAL_INIT;
> @@ -1538,6 +1534,7 @@ static void gc_worker(struct work_struct *work)
>  		}
>  
>  		hlist_nulls_for_each_entry_rcu(h, n, &ct_hash[i], hnnode) {
> +			unsigned int nf_conntrack_max95 = 0;
>  			struct nf_conntrack_net *cnet;
>  			struct net *net;
>  			long expires;
> @@ -1567,11 +1564,14 @@ static void gc_worker(struct work_struct *work)
>  			expires = clamp(nf_ct_expires(tmp), GC_SCAN_INTERVAL_MIN, GC_SCAN_INTERVAL_CLAMP);
>  			expires = (expires - (long)next_run) / ++count;
>  			next_run += expires;
> +			net = nf_ct_net(tmp);
> +
> +			if (gc_work->early_drop)
> +				nf_conntrack_max95 = nf_conntrack_max(net) / 100u * 95u;
>  
>  			if (nf_conntrack_max95 == 0 || gc_worker_skip_ct(tmp))
>  				continue;
>  
> -			net = nf_ct_net(tmp);
>  			cnet = nf_ct_pernet(net);
>  			if (atomic_read(&cnet->count) < nf_conntrack_max95)
>  				continue;
> @@ -1648,13 +1648,14 @@ __nf_conntrack_alloc(struct net *net,
>  		     gfp_t gfp, u32 hash)
>  {
>  	struct nf_conntrack_net *cnet = nf_ct_pernet(net);
> -	unsigned int ct_count;
> +	unsigned int ct_max, ct_count;
>  	struct nf_conn *ct;
>  
>  	/* We don't want any race condition at early drop stage */
>  	ct_count = atomic_inc_return(&cnet->count);
> +	ct_max = nf_conntrack_max(net);
>  
> -	if (nf_conntrack_max && unlikely(ct_count > nf_conntrack_max)) {
> +	if (ct_max && unlikely(ct_count > ct_max)) {
>  		if (!early_drop(net, hash)) {
>  			if (!conntrack_gc_work.early_drop)
>  				conntrack_gc_work.early_drop = true;
> @@ -2650,7 +2651,7 @@ int nf_conntrack_init_start(void)
>  	if (!nf_conntrack_hash)
>  		return -ENOMEM;
>  
> -	nf_conntrack_max = max_factor * nf_conntrack_htable_size;
> +	init_net.ct.sysctl_max = max_factor * nf_conntrack_htable_size;
>  
>  	nf_conntrack_cachep = kmem_cache_create("nf_conntrack",
>  						sizeof(struct nf_conn),
> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> index 2cc0fde23344..73e6bb1e939b 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -2608,7 +2608,7 @@ ctnetlink_stat_ct_fill_info(struct sk_buff *skb, u32 portid, u32 seq, u32 type,
>  	if (nla_put_be32(skb, CTA_STATS_GLOBAL_ENTRIES, htonl(nr_conntracks)))
>  		goto nla_put_failure;
>  
> -	if (nla_put_be32(skb, CTA_STATS_GLOBAL_MAX_ENTRIES, htonl(nf_conntrack_max)))
> +	if (nla_put_be32(skb, CTA_STATS_GLOBAL_MAX_ENTRIES, htonl(nf_conntrack_max(net))))
>  		goto nla_put_failure;
>  
>  	nlmsg_end(skb, nlh);
> diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
> index 2f666751c7e7..5db6df0e4eb3 100644
> --- a/net/netfilter/nf_conntrack_standalone.c
> +++ b/net/netfilter/nf_conntrack_standalone.c
> @@ -615,7 +615,7 @@ enum nf_ct_sysctl_index {
>  static struct ctl_table nf_ct_sysctl_table[] = {
>  	[NF_SYSCTL_CT_MAX] = {
>  		.procname	= "nf_conntrack_max",
> -		.data		= &nf_conntrack_max,
> +		.data		= &init_net.ct.sysctl_max,
>  		.maxlen		= sizeof(int),
>  		.mode		= 0644,
>  		.proc_handler	= proc_dointvec_minmax,
> @@ -948,7 +948,7 @@ static struct ctl_table nf_ct_sysctl_table[] = {
>  static struct ctl_table nf_ct_netfilter_table[] = {
>  	{
>  		.procname	= "nf_conntrack_max",
> -		.data		= &nf_conntrack_max,
> +		.data		= &init_net.ct.sysctl_max,
>  		.maxlen		= sizeof(int),
>  		.mode		= 0644,
>  		.proc_handler	= proc_dointvec_minmax,
> @@ -1063,6 +1063,7 @@ static int nf_conntrack_standalone_init_sysctl(struct net *net)
>  
>  	table[NF_SYSCTL_CT_COUNT].data = &cnet->count;
>  	table[NF_SYSCTL_CT_CHECKSUM].data = &net->ct.sysctl_checksum;
> +	table[NF_SYSCTL_CT_MAX].data = &net->ct.sysctl_max;
>  	table[NF_SYSCTL_CT_LOG_INVALID].data = &net->ct.sysctl_log_invalid;
>  	table[NF_SYSCTL_CT_ACCT].data = &net->ct.sysctl_acct;
>  #ifdef CONFIG_NF_CONNTRACK_EVENTS
> @@ -1087,7 +1088,6 @@ static int nf_conntrack_standalone_init_sysctl(struct net *net)
>  
>  	/* Don't allow non-init_net ns to alter global sysctls */
>  	if (!net_eq(&init_net, net)) {
> -		table[NF_SYSCTL_CT_MAX].mode = 0444;
>  		table[NF_SYSCTL_CT_EXPECT_MAX].mode = 0444;
>  		table[NF_SYSCTL_CT_BUCKETS].mode = 0444;
>  	}
> @@ -1139,6 +1139,7 @@ static int nf_conntrack_pernet_init(struct net *net)
>  	int ret;
>  
>  	net->ct.sysctl_checksum = 1;
> +	net->ct.sysctl_max = init_net.ct.sysctl_max;
>  
>  	ret = nf_conntrack_standalone_init_sysctl(net);
>  	if (ret < 0)
> -- 
> 2.40.1
>

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > -        to nf_conntrack_buckets by default.
> > -        Note that connection tracking entries are added to the table twice -- once
> > -        for the original direction and once for the reply direction (i.e., with
> > -        the reversed address). This means that with default settings a maxed-out
> > -        table will have a average hash chain length of 2, not 1.
> > +    - 0 - disabled (unlimited)
> 
> unlimited is too much, and the number of buckets is also global, how
> does this work?

Its an historic wart going back to ip_conntrack - it was never the
default but you could disable any and all limits even in the original
version.

Wether its time to disallow 0 is a different topic and not related to this patch.

I would argue: "yes", disallow 0 -- users can still set INT_MAX if they
 want and that should provide enough rope to strangle yourself.

> > +    The limit of other netns cannot be greater than init_net netns.
> > +    +----------------+-------------+----------------+
> > +    | init_net netns | other netns | limit behavior |
> > +    +----------------+-------------+----------------+
> > +    | 0              | 0           | unlimited      |
> > +    +----------------+-------------+----------------+
> > +    | 0              | not 0       | other          |
> > +    +----------------+-------------+----------------+
> > +    | not 0          | 0           | init_net       |
> > +    +----------------+-------------+----------------+
> > +    | not 0          | not 0       | min            |
> > +    +----------------+-------------+----------------+

I think this is fine, it doesn't really change things from init_net
point of view.

On Thu, May 22, 2025 at 09:32:23PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > -        to nf_conntrack_buckets by default.
> > > -        Note that connection tracking entries are added to the table twice -- once
> > > -        for the original direction and once for the reply direction (i.e., with
> > > -        the reversed address). This means that with default settings a maxed-out
> > > -        table will have a average hash chain length of 2, not 1.
> > > +    - 0 - disabled (unlimited)
> > 
> > unlimited is too much, and the number of buckets is also global, how
> > does this work?
> 
> Its an historic wart going back to ip_conntrack - it was never the
> default but you could disable any and all limits even in the original
> version.

Thanks, I was just sitting here clueless.

> Wether its time to disallow 0 is a different topic and not related to this patch.
>
> I would argue: "yes", disallow 0 -- users can still set INT_MAX if they
>  want and that should provide enough rope to strangle yourself.

The question is how to make it without breaking crazy people.

> > > +    The limit of other netns cannot be greater than init_net netns.
> > > +    +----------------+-------------+----------------+
> > > +    | init_net netns | other netns | limit behavior |
> > > +    +----------------+-------------+----------------+
> > > +    | 0              | 0           | unlimited      |
> > > +    +----------------+-------------+----------------+
> > > +    | 0              | not 0       | other          |
> > > +    +----------------+-------------+----------------+
> > > +    | not 0          | 0           | init_net       |

in this case above...

> > > +    +----------------+-------------+----------------+
> > > +    | not 0          | not 0       | min            |

... and this case, init_net value is used as a cap for other netns.
Then, this is basically allowing to specify a maximum that is smaller
than init_netns.

IIUC, that sounds reasonable.

As for how to discontinue the unlimited in other netns, let me know if
you have any suggestions.

> > > +    +----------------+-------------+----------------+
> 
> I think this is fine, it doesn't really change things from init_net
> point of view.

Thanks for explaning.

On Thu, 22 May 2025 21:58:13 +0200, Pablo Neira Ayuso wrote:

> > Wether its time to disallow 0 is a different topic and not related to this patch.
> >
> > I would argue: "yes", disallow 0 -- users can still set INT_MAX if they
> >  want and that should provide enough rope to strangle yourself.

> The question is how to make it without breaking crazy people.

It seems that we need a new topic to discuss the maximum value that the system can
tolerate to ensure safety:

1. This value is a system limitation, not a user setting

2. This value should be calculated based on system resources

3. This value takes precedence over 0 and other larger values that the user sets

4. This value does not affect the value of the user setting, and 0 in the user
setting can still indicate that the user setting is unlimited, maintaining
compatibility with historical usage.

lvxiafei <xiafei_xupt@163.com> wrote:
> > > Wether its time to disallow 0 is a different topic and not related to this patch.
> > >
> > > I would argue: "yes", disallow 0 -- users can still set INT_MAX if they
> > >  want and that should provide enough rope to strangle yourself.
> 
> > The question is how to make it without breaking crazy people.
> 
> It seems that we need a new topic to discuss the maximum value that the system can
> tolerate to ensure safety:
> 
> 1. This value is a system limitation, not a user setting
> 2. This value should be calculated based on system resources
> 3. This value takes precedence over 0 and other larger values that the user sets
> 4. This value does not affect the value of the user setting, and 0 in the user
> setting can still indicate that the user setting is unlimited, maintaining
> compatibility with historical usage.

I've applied a variant of this patch to nf-next:testing.

Could you please check that I adapted it correctly?
https://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next.git/commit/?h=testing&id=b7bfa7d96fa5a7f3c2a69ad406ede520e658cb07

(I added a patch right before that rejects conntrack_max=0).

I wonder if we should update the sysctl path to reflect the
effective value, i.e., so that when netns sets

nf_conntrack_max=1000000

... but init_net is capped at 65536, then a listing
shows the sysctl at 65536.

It would be similar to what we do for max_buckets.

I also considered to make such a request fail at set time, but it
would make the sysctl fail/not fail 'randomly' and it also would
not do the right thing when init_net setting is reduced later.

>  I've applied a variant of this patch to nf-next:testing.
>  
>  Could you please check that I adapted it correctly?
>  https://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next.git/commit/?h=testing&id=b7bfa7d96fa5a7f3c2a69ad406ede520e658cb07
>  
>  (I added a patch right before that rejects conntrack_max=0).

Historically, some systems or scripts have used 0 to mean “unlimited”.
In this way, some scripts that are set to 0 need to be adjusted.
Rejecting this value may break compatibility, so it would be good to document 
this behavior change clearly in the commit message and/or changelog.

>  
>  I wonder if we should update the sysctl path to reflect the
>  effective value, i.e., so that when netns sets
>  
>  nf_conntrack_max=1000000
>  
>  ... but init_net is capped at 65536, then a listing
>  shows the sysctl at 65536.
>  
>  It would be similar to what we do for max_buckets.

I would argue against updating the sysctl path to reflect the 
effective value. Doing so could be misleading, as it would no 
longer show the value actually configured in the namespace, 
but rather the clamped or capped value. Users might feel that 
their explicit configuration has been silently altered, which 
can be frustrating for professional users who rely on precise 
control. If there is a genuine need to see the effective value, 
it can be computed on demand or exposed via a separate parameter 
specifically indicating the effective value. Keeping the sysctl 
path as-is preserves transparency, predictability, and user trust.

>  
>  I also considered to make such a request fail at set time, but it
>  would make the sysctl fail/not fail 'randomly' and it also would
>  not do the right thing when init_net setting is reduced later.

I would be cautious about making the sysctl fail at set time. Doing 
so could lead to seemingly “random” failures depending on the current 
state of init_net, which would be confusing for users. Moreover, if 
init_net’s setting is reduced later, the sysctl behavior would not be 
consistent, and users might end up with invalid or unexpected values 
anyway. It seems safer to allow the set operation to succeed but let 
the effective value be determined by the existing limits, maintaining
predictable behavior.