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

Jeff Layton
2025-04-10T13:08:05Z

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>