Re: [PATCH v2 2/2] net: add debugfs files for showing netns refcount tracking info

[Jeff Layton <jlayton@kernel.org>]

On Thu, 2025-04-10 at 14:36 +0200, Andrew Lunn wrote:
> On Tue, Apr 08, 2025 at 09:36:38AM -0400, Jeff Layton wrote:
> > CONFIG_NET_NS_REFCNT_TRACKER currently has no convenient way to display
> > its tracking info. Add a new net_ns directory under the debugfs
> > ref_tracker directory. Create a directory in there for every netns, with
> > refcnt and notrefcnt files that show the currently tracked active and
> > passive references.
> 
> I think most if not all of this should be moved into the tracker
> sources, there is very little which is netdev specific. 
> 

Fair enough. I can move most of this into helpers in ref_tracker.c.

> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  net/core/net_namespace.c | 151 +++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 151 insertions(+)
> > 
> > diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> > index 4303f2a4926243e2c0ff0c0387383cd8e0658019..7e9dc487f46d656ee4ae3d6d18d35bb2aba2b176 100644
> > --- a/net/core/net_namespace.c
> > +++ b/net/core/net_namespace.c
> > @@ -1512,3 +1512,154 @@ const struct proc_ns_operations netns_operations = {
> >  	.owner		= netns_owner,
> >  };
> >  #endif
> > +
> > +#ifdef CONFIG_DEBUG_FS
> > +#ifdef CONFIG_NET_NS_REFCNT_TRACKER
> > +
> > +#include <linux/debugfs.h>
> > +
> > +static struct dentry *ns_ref_tracker_dir;
> > +static unsigned int ns_debug_net_id;
> > +
> > +struct ns_debug_net {
> > +	struct dentry *netdir;
> > +	struct dentry *refcnt;
> > +	struct dentry *notrefcnt;
> > +};
> > +
> > +#define MAX_NS_DEBUG_BUFSIZE	(32 * PAGE_SIZE)
> > +
> > +static int
> > +ns_debug_tracker_show(struct seq_file *f, void *v)
> > +{
> > +	struct ref_tracker_dir *tracker = f->private;
> > +	int len, bufsize = PAGE_SIZE;
> > +	char *buf;
> > +
> > +	for (;;) {
> > +		buf = kvmalloc(bufsize, GFP_KERNEL);
> > +		if (!buf)
> > +			return -ENOMEM;
> > +
> > +		len = ref_tracker_dir_snprint(tracker, buf, bufsize);
> > +		if (len < bufsize)
> > +			break;
> > +
> > +		kvfree(buf);
> > +		bufsize *= 2;
> > +		if (bufsize > MAX_NS_DEBUG_BUFSIZE)
> > +			return -ENOBUFS;
> 
> Maybe consider storing bufsize between calls to dump the tracker? I
> guess you then have about the correct size for most calls, and from
> looking at len, you can decide to downsize it if needed.
> 

Eric had a proposed change to make ref_tracker_dir_snprint() not sit
with hard IRQs disabled for so long. That involved passing back a
needed size, so I might rather integrate this into that change.

> > +static int
> > +ns_debug_init_net(struct net *net)
> > +{
> > +	struct ns_debug_net *dnet = net_generic(net, ns_debug_net_id);
> > +	char name[11]; /* 10 decimal digits + NULL term */
> > +	int len;
> > +
> > +	len = snprintf(name, sizeof(name), "%u", net->ns.inum);
> > +	if (len >= sizeof(name))
> > +		return -EOVERFLOW;
> > +
> > +	dnet->netdir = debugfs_create_dir(name, ns_ref_tracker_dir);
> > +	if (IS_ERR(dnet->netdir))
> > +		return PTR_ERR(dnet->netdir);
> 
> As i pointed out before, the tracker already has a name. Is that name
> useless? Not specific enough? Rather than having two names, maybe
> change the name to make it useful. Once it has a usable name, you
> should be able to push more code into the core.
> 

I don't understand which name you mean.

This patch creates a ref_tracker/net_ns dir and then creates
directories under that that have names that match the net namespace
inode numbers as displayed in /proc/<pid>/ns/net symlink. Those
directories hold two files "refcnt" and "notrefcnt" that display the
different trackers.

It's not clear to me what you'd like to see changed in that scheme.
-- 
Jeff Layton <jlayton@kernel.org>