diff mbox series

net: set struct net_device::name earlier

Message ID d116cbdb-4dc5-484a-b53b-fec50f8ef2bf@p183 (mailing list archive)
State Deferred
Delegated to: Netdev Maintainers
Headers show
Series net: set struct net_device::name earlier | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 912 this patch: 912
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 4 of 4 maintainers
netdev/build_clang success Errors and warnings before: 909 this patch: 909
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 916 this patch: 916
netdev/checkpatch warning WARNING: Prefer strscpy over strcpy - see: https://github.com/KSPP/linux/issues/88
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 79 this patch: 79
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-05-19--03-00 (tests: 1039)

Commit Message

Alexey Dobriyan May 18, 2024, 8:24 p.m. UTC
I've tried debugging networking allocations with bpftrace and doing

	$dev = (struct net_device*)arg0;
	printf("dev %s\n", $dev->name);

doesn't print anything useful in functions called right after netdevice
allocation. The reason is very simple: dev->name has not been set yet.

Make name copying much earlier for smoother debugging experience.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 net/core/dev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andrew Lunn May 18, 2024, 10:04 p.m. UTC | #1
On Sat, May 18, 2024 at 11:24:57PM +0300, Alexey Dobriyan wrote:
> I've tried debugging networking allocations with bpftrace and doing
> 
> 	$dev = (struct net_device*)arg0;
> 	printf("dev %s\n", $dev->name);
> 
> doesn't print anything useful in functions called right after netdevice
> allocation. The reason is very simple: dev->name has not been set yet.
> 
> Make name copying much earlier for smoother debugging experience.

Does this really help? Instead of "" don't you get "eth%d"? The
expansion of the %d to eth42 does not happen until you register the
netdev.

	Andrew
Alexey Dobriyan May 19, 2024, 5:41 a.m. UTC | #2
On Sun, May 19, 2024 at 12:04:05AM +0200, Andrew Lunn wrote:
> On Sat, May 18, 2024 at 11:24:57PM +0300, Alexey Dobriyan wrote:
> > I've tried debugging networking allocations with bpftrace and doing
> > 
> > 	$dev = (struct net_device*)arg0;
> > 	printf("dev %s\n", $dev->name);
> > 
> > doesn't print anything useful in functions called right after netdevice
> > allocation. The reason is very simple: dev->name has not been set yet.
> > 
> > Make name copying much earlier for smoother debugging experience.
> 
> Does this really help?

Yes and no. One could infer names from stacktraces and overall ordering
of the allocations but it isn't convenient.

The snippet works everywhere except small number of functions
but it doesn't cost anything to make it work everywhere.

> Instead of "" don't you get "eth%d"? The expansion of the %d to eth42
> does not happen until you register the netdev.

Expansion happens later, yes. %d is fine. It is immediately obvious
which type of device allocates what.
Andrew Lunn May 19, 2024, 1:42 p.m. UTC | #3
On Sun, May 19, 2024 at 08:41:40AM +0300, Alexey Dobriyan wrote:
> On Sun, May 19, 2024 at 12:04:05AM +0200, Andrew Lunn wrote:
> > On Sat, May 18, 2024 at 11:24:57PM +0300, Alexey Dobriyan wrote:
> > > I've tried debugging networking allocations with bpftrace and doing
> > > 
> > > 	$dev = (struct net_device*)arg0;
> > > 	printf("dev %s\n", $dev->name);
> > > 
> > > doesn't print anything useful in functions called right after netdevice
> > > allocation. The reason is very simple: dev->name has not been set yet.
> > > 
> > > Make name copying much earlier for smoother debugging experience.
> > 
> > Does this really help?
> 
> Yes and no. One could infer names from stacktraces and overall ordering
> of the allocations but it isn't convenient.
> 
> The snippet works everywhere except small number of functions
> but it doesn't cost anything to make it work everywhere.
> 
> > Instead of "" don't you get "eth%d"? The expansion of the %d to eth42
> > does not happen until you register the netdev.
> 
> Expansion happens later, yes. %d is fine. It is immediately obvious
> which type of device allocates what.

We are in the merge window at the moment, so patches are not being
accepted at the moment. You will need to repost in a weeks time. When
you do, please expand the commit message. The "which type of device"
is what is important here, not the true device name.

Also, please take a look at:

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

	Andrew
diff mbox series

Patch

--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10952,6 +10952,7 @@  struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 	dev = PTR_ALIGN(p, NETDEV_ALIGN);
 	dev->padded = (char *)dev - (char *)p;
 
+	strcpy(dev->name, name);
 	ref_tracker_dir_init(&dev->refcnt_tracker, 128, name);
 #ifdef CONFIG_PCPU_DEV_REFCNT
 	dev->pcpu_refcnt = alloc_percpu(int);
@@ -11015,7 +11016,6 @@  struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 	if (netif_alloc_rx_queues(dev))
 		goto free_all;
 
-	strcpy(dev->name, name);
 	dev->name_assign_type = name_assign_type;
 	dev->group = INIT_NETDEV_GROUP;
 	if (!dev->ethtool_ops)