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