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

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.

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?

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.

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.

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.