lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cdfc5c6f260ee1f81b8bb0402488bb97dd4351bb.camel@kernel.org>
Date: Thu, 24 Apr 2025 06:56:06 -0400
From: Jeff Layton <jlayton@...nel.org>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>, "David S. Miller"	
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni	
 <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, Kuniyuki Iwashima	
 <kuniyu@...zon.com>, Qasim Ijaz <qasdev00@...il.com>, Nathan Chancellor	
 <nathan@...nel.org>, Andrew Lunn <andrew@...n.ch>,
 linux-kernel@...r.kernel.org, 	netdev@...r.kernel.org
Subject: Re: [PATCH v4 7/7] net: register debugfs file for net_device refcnt
 tracker

On Wed, 2025-04-23 at 17:32 -0700, Jakub Kicinski wrote:
> On Wed, 23 Apr 2025 20:04:58 -0400 Jeff Layton wrote:
> > On Wed, 2025-04-23 at 16:53 -0700, Jakub Kicinski wrote:
> > > Names are not unique and IIUC debugfs is not namespaced.

Correct, debugfs is not namespaced.

I meant to say earlier that I'm open to suggestions on how to make the
netdev names unique. Low-level netdev stuff is not my area of
expertise. We can drop this patch if doing so is problematic.

> > > How much naming the objects in a "user readable" fashion actually
> > > matter? It'd be less churn to create some kind of "object class"
> > > with a directory level named after what's already passed to
> > > ref_tracker_dir_init() and then id the objects by the pointer value 
> > > as sub-dirs of that?  
> > 
> > That sounds closer to what I had done originally. Andrew L. suggested
> > the flat directory that this version represents. I'm fine with whatever
> > hierarchy, but let's decide that before I respin again.
> 
> Sorry about that :(
> 

No worries...but we do need to decide what this directory hierarchy
should look like.

Andrew's point earlier was that this is just debugfs, so a flat
"ref_tracker" directory full of files is fine. I tend to agree with
him; NAME_MAX is 255, so we have plenty of room to make uniquely-named
files.

We could build a dir hierarchy though. Something like:

- ref_tracker
    + netdev
    + netns
    
...and then each object class subdir would hold uniquely-named files.
Each subsys would need to create the directory and then the files
within it, but we could add helpers for that.

Note that this is debugfs, so we can always change that later as it's
not counted as part of ABI.

Which way should we do it?
 
> > When I was tracking down net namespace leaks recently, it was very nice
> > to have the inode number of the leaked netns's in the filenames. I
> > would have probably had to grovel around with drgn to figure out the
> > address if that weren't embedded in the name. I think we probably ought
> > to leave it up to each subsystem how it names its files. The
> > discriminators between different types of objects can vary wildly.
> > 
> > One thing that might be simpler is to make ref_tracker_dir_debugfs() a
> > varargs function and allow passing in a format string and set of
> > arguments for it. That might make things simpler for the callers.
> 
> Yes, cutting out the formatting in the callers would definitely 
> be a win. Maybe that'd make the whole thing sufficiently palatable :)

Ok, I'll plan to change that. That does make the callers a lot cleaner.
-- 
Jeff Layton <jlayton@...nel.org>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ