[PATCH] netfilter: nf_conntrack: table full detailed log

[PATCH] netfilter: nf_conntrack: table full detailed log

[lvxiafei]

From: lvxiafei <lvxiafei@sensetime.com>

Add the netns field in the "nf_conntrack: table full,
dropping packet" log to help locate the specific netns
when the table is full.

Signed-off-by: lvxiafei <lvxiafei@sensetime.com>
---
 net/netfilter/nf_conntrack_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 7f8b245e287a..71849960cf0c 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1659,7 +1659,8 @@ __nf_conntrack_alloc(struct net *net,
 			if (!conntrack_gc_work.early_drop)
 				conntrack_gc_work.early_drop = true;
 			atomic_dec(&cnet->count);
-			net_warn_ratelimited("nf_conntrack: table full, dropping packet\n");
+			net_warn_ratelimited("nf_conntrack: table full in netns %u, dropping packet\n",
+					     net->ns.inum);
 			return ERR_PTR(-ENOMEM);
 		}
 	}
-- 
2.40.1

Re: [PATCH] netfilter: nf_conntrack: table full detailed log

[Pablo Neira Ayuso]

Hi,

On Thu, May 08, 2025 at 04:13:12PM +0800, lvxiafei wrote:
> From: lvxiafei <lvxiafei@sensetime.com>
> 
> Add the netns field in the "nf_conntrack: table full,
> dropping packet" log to help locate the specific netns
> when the table is full.
> 
> Signed-off-by: lvxiafei <lvxiafei@sensetime.com>
> ---
>  net/netfilter/nf_conntrack_core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index 7f8b245e287a..71849960cf0c 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -1659,7 +1659,8 @@ __nf_conntrack_alloc(struct net *net,
>  			if (!conntrack_gc_work.early_drop)
>  				conntrack_gc_work.early_drop = true;
>  			atomic_dec(&cnet->count);
> -			net_warn_ratelimited("nf_conntrack: table full, dropping packet\n");
> +			net_warn_ratelimited("nf_conntrack: table full in netns %u, dropping packet\n",
> +					     net->ns.inum);

Maybe print this only if this is not init_netns?

Thanks

>  			return ERR_PTR(-ENOMEM);
>  		}
>  	}
> -- 
> 2.40.1
> 

Re: [PATCH] netfilter: nf_conntrack: table full detailed log

[lvxiafei]

Hi,

On Wed, 21 May 2025 12:28:40 +0200, Pablo Neira Ayuso wrote:
> Maybe print this only if this is not init_netns?
>
> Thanks

Thanks for the feedback.

You're right — for init_net, the netns information may not be helpful and could be redundant.
I'll update the patch to print the netns information only when net != &init_net.

Will send v2 shortly.

Thanks

[PATCH V2] netfilter: nf_conntrack: table full detailed log

[lvxiafei]

From: lvxiafei <lvxiafei@sensetime.com>

Add the netns field in the "nf_conntrack: table full,
dropping packet" log to help locate the specific netns
when the table is full.

Signed-off-by: lvxiafei <lvxiafei@sensetime.com>
---
 net/netfilter/nf_conntrack_core.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 7f8b245e287a..47036a9d4acc 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1659,7 +1659,11 @@ __nf_conntrack_alloc(struct net *net,
 			if (!conntrack_gc_work.early_drop)
 				conntrack_gc_work.early_drop = true;
 			atomic_dec(&cnet->count);
-			net_warn_ratelimited("nf_conntrack: table full, dropping packet\n");
+			if (net == &init_net)
+				net_warn_ratelimited("nf_conntrack: table full, dropping packet\n");
+			else
+				net_warn_ratelimited("nf_conntrack: table full in netns %u, dropping packet\n",
+						     net->ns.inum);
 			return ERR_PTR(-ENOMEM);
 		}
 	}
-- 
2.40.1

Re: [PATCH V2] netfilter: nf_conntrack: table full detailed log

[Pablo Neira Ayuso]

On Thu, May 22, 2025 at 05:19:54PM +0800, lvxiafei wrote:
> From: lvxiafei <lvxiafei@sensetime.com>
> 
> Add the netns field in the "nf_conntrack: table full,
> dropping packet" log to help locate the specific netns
> when the table is full.
> 
> Signed-off-by: lvxiafei <lvxiafei@sensetime.com>
> ---
>  net/netfilter/nf_conntrack_core.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index 7f8b245e287a..47036a9d4acc 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -1659,7 +1659,11 @@ __nf_conntrack_alloc(struct net *net,
>  			if (!conntrack_gc_work.early_drop)
>  				conntrack_gc_work.early_drop = true;
>  			atomic_dec(&cnet->count);
> -			net_warn_ratelimited("nf_conntrack: table full, dropping packet\n");
> +			if (net == &init_net)
> +				net_warn_ratelimited("nf_conntrack: table full, dropping packet\n");
> +			else
> +				net_warn_ratelimited("nf_conntrack: table full in netns %u, dropping packet\n",
> +						     net->ns.inum);

This is slightly better, but it still does not say what packet has
been dropped, right?

Probably a similar approach to nf_tcp_log_invalid() would better here.

Thus, nf_log infrastructure could be used as logging hub.

Logging the packet probably provides more context information than
simply logging the netns inode number.

Re: [PATCH V2] netfilter: nf_conntrack: table full detailed log

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > +				net_warn_ratelimited("nf_conntrack: table full in netns %u, dropping packet\n",
> > +						     net->ns.inum);
> 
> This is slightly better, but it still does not say what packet has
> been dropped, right?
> 
> Probably a similar approach to nf_tcp_log_invalid() would better here.
>
> Thus, nf_log infrastructure could be used as logging hub.
> 
> Logging the packet probably provides more context information than
> simply logging the netns inode number.

Hmm, the conntrack table is full, and packet creates a new flow.
What would logging the packet tell us what the printk message doesn't?

Re: [PATCH V2] netfilter: nf_conntrack: table full detailed log

[Pablo Neira Ayuso]

On Thu, Jul 24, 2025 at 05:26:33PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > +				net_warn_ratelimited("nf_conntrack: table full in netns %u, dropping packet\n",
> > > +						     net->ns.inum);
> > 
> > This is slightly better, but it still does not say what packet has
> > been dropped, right?
> > 
> > Probably a similar approach to nf_tcp_log_invalid() would better here.
> >
> > Thus, nf_log infrastructure could be used as logging hub.
> > 
> > Logging the packet probably provides more context information than
> > simply logging the netns inode number.
> 
> Hmm, the conntrack table is full, and packet creates a new flow.
> What would logging the packet tell us what the printk message doesn't?

I was thinking, does the packet logging exposes already the
net->ns.inum? IIUC the goal is to find what netns is dropping what
packet and the reason for the packet drop, not only in this case but
in every case, to ease finding the needle in the stack. If so, then it
probably makes sense to consolidate this around nf_log()
infrastructure.

Anyway, maybe I'm overdoing, I'll be fine with this approach if you
consider it good enough to improve the situation.

Thanks.

Re: [PATCH V2] netfilter: nf_conntrack: table full detailed log

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> I was thinking, does the packet logging exposes already the
> net->ns.inum? IIUC the goal is to find what netns is dropping what
> packet and the reason for the packet drop, not only in this case but
> in every case, to ease finding the needle in the stack. If so, then it
> probably makes sense to consolidate this around nf_log()
> infrastructure.

No, it doesn't.  It also depends on the backend:
for syslog, nothing will be logged unless nf_log_all_netns sysctl is
enabled.

For nflog, it is logged, to the relevant namespaces ulogd, or not in
case that netns doesn't have ulogd running.

For syslog one could extend nf_log_dump_packet_common() but I'm not sure
how forgiving existing log parsers are when this gets additional
field.

Also, would (in case we use this for the "table full" condition), should
this log unconditionally or does it need a new sysctl?

Does it need auto-ratelimit (probably yes, its called during packet
flood so we dont want to flood syslog/ulog)?

> Anyway, maybe I'm overdoing, I'll be fine with this approach if you
> consider it good enough to improve the situation.

I think its better than current state of affairs since it at least
allows to figure out which netns is experiencing this.

Re: [PATCH V2] netfilter: nf_conntrack: table full detailed log

[Pablo Neira Ayuso]

On Fri, Jul 25, 2025 at 01:55:33AM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > I was thinking, does the packet logging exposes already the
> > net->ns.inum? IIUC the goal is to find what netns is dropping what
> > packet and the reason for the packet drop, not only in this case but
> > in every case, to ease finding the needle in the stack. If so, then it
> > probably makes sense to consolidate this around nf_log()
> > infrastructure.
> 
> No, it doesn't.  It also depends on the backend:
> for syslog, nothing will be logged unless nf_log_all_netns sysctl is
> enabled.
> 
> For nflog, it is logged, to the relevant namespaces ulogd, or not in
> case that netns doesn't have ulogd running.
> 
> For syslog one could extend nf_log_dump_packet_common() but I'm not sure
> how forgiving existing log parsers are when this gets additional
> field.
> 
> Also, would (in case we use this for the "table full" condition), should
> this log unconditionally or does it need a new sysctl?
> 
> Does it need auto-ratelimit (probably yes, its called during packet
> flood so we dont want to flood syslog/ulog)?

Yes, such extension would need to answer these questions.

> > Anyway, maybe I'm overdoing, I'll be fine with this approach if you
> > consider it good enough to improve the situation.
> 
> I think its better than current state of affairs since it at least
> allows to figure out which netns is experiencing this.

Thanks for explaining, let's take this patch as is then.