[PATCH] net: set struct net_device::name earlier

[PATCH] net: set struct net_device::name earlier

[Alexey Dobriyan]

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(-)

--- 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)

Re: [PATCH] net: set struct net_device::name earlier

[Andrew Lunn]

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

Re: [PATCH] net: set struct net_device::name earlier

[Alexey Dobriyan]

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.

Re: [PATCH] net: set struct net_device::name earlier

[Andrew Lunn]

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