Re: [PATCH] ath10k: allocate dummy net_device dynamically

From: Breno Leitao
Date: Wed Mar 27 2024 - 11:11:48 EST


Hello Jakub,

On Fri, Mar 22, 2024 at 08:23:36AM -0700, Jakub Kicinski wrote:
> > > Maybe we should add a new helper to "alloc dummy netdev" which can
> > > call alloc_netdev() with right arguments and do necessary init?
> >
> > What are the right arguments in this case?
>
> I'm not sure we have a noop setup() callback today. If you define a
> wrapper to allocate a dummy netdev you can define a new empty function
> next to it and pass that as init? Hope I got the question right.

Thanks for the explanation, it is clear now. I've been working on it,
and this is what I came up with. This is compile-tested by now, and, if
this is what you had in mind, I will do more extensive testing.

commit db794d99950f68731884a67d911094d94179c522
Author: Breno Leitao <leitao@xxxxxxxxxx>
Date: Wed Mar 27 07:20:03 2024 -0700

net: Create net_device allocator for dummy

Create a helper to allocate and initialize dummy netdevices. This
function basically simplify the allocation of dummy devices, by
allocating and initializing it.

Suggested-by: Jakub Kicinski <kuba@xxxxxxxxxx>
Signed-off-by: Breno Leitao <leitao@xxxxxxxxxx>

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 643d2b469c49..9d1a5383c23f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4546,6 +4546,9 @@ static inline void netif_addr_unlock_bh(struct net_device *dev)

void ether_setup(struct net_device *dev);

+/* Allocate dummy net_device */
+struct net_device *alloc_netdev_dummy(int sizeof_priv, const char *name);
+
/* Support for loadable net-drivers */
struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
unsigned char name_assign_type,
diff --git a/net/core/dev.c b/net/core/dev.c
index a08d698fe45c..628f35c3cfa2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10998,6 +10998,13 @@ void free_netdev(struct net_device *dev)
}
EXPORT_SYMBOL(free_netdev);

+struct net_device *alloc_netdev_dummy(int sizeof_priv, const char *name)
+{
+ return alloc_netdev(sizeof_priv, name, NET_NAME_UNKNOWN,
+ init_dummy_netdev_core);
+}
+EXPORT_SYMBOL_GPL(alloc_netdev_dummy);
+
/**
* synchronize_net - Synchronize with packet receive processing
*

commit 35500cd6a5db0bbdedbc1067758948769c7ce57e
Author: Breno Leitao <leitao@xxxxxxxxxx>
Date: Wed Mar 27 07:07:40 2024 -0700

net: Split init_dummy_netdev

It is impossible to use init_dummy_netdev together with alloc_netdev()
as the 'setup' argument.

This is because alloc_netdev() initializes some fields in the net_device
structure, and later init_dummy_netdev() memzero them all. This casues
some problems as reported here:

https://lore.kernel.org/all/20240322082336.49f110cc@xxxxxxxxxx/

Split the function in two. Create a new function called
init_dummy_netdev_core() that does not memset the net_device structure.
Then have init_dummy_netdev() memseting and calling
init_dummy_netdev_core().

init_dummy_netdev_core() will be the function that could be called as
an argument for alloc_netdev().

Signed-off-by: Breno Leitao <leitao@xxxxxxxxxx>

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c6f6ac779b34..643d2b469c49 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3119,6 +3119,7 @@ int netdev_refcnt_read(const struct net_device *dev);
void free_netdev(struct net_device *dev);
void netdev_freemem(struct net_device *dev);
void init_dummy_netdev(struct net_device *dev);
+void init_dummy_netdev_core(struct net_device *dev);

struct net_device *netdev_get_xmit_slave(struct net_device *dev,
struct sk_buff *skb,
diff --git a/net/core/dev.c b/net/core/dev.c
index 0766a245816b..a08d698fe45c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10340,25 +10340,11 @@ int register_netdevice(struct net_device *dev)
}
EXPORT_SYMBOL(register_netdevice);

-/**
- * init_dummy_netdev - init a dummy network device for NAPI
- * @dev: device to init
- *
- * This takes a network device structure and initialize the minimum
- * amount of fields so it can be used to schedule NAPI polls without
- * registering a full blown interface. This is to be used by drivers
- * that need to tie several hardware interfaces to a single NAPI
- * poll scheduler due to HW limitations.
+/* Same as init_dummy_netdev, but, basically do not call memset.
+ * This is useful if you are calling this function after alloc_netdev()
*/
-void init_dummy_netdev(struct net_device *dev)
+void init_dummy_netdev_core(struct net_device *dev)
{
- /* Clear everything. Note we don't initialize spinlocks
- * are they aren't supposed to be taken by any of the
- * NAPI code and this dummy netdev is supposed to be
- * only ever used for NAPI polls
- */
- memset(dev, 0, sizeof(struct net_device));
-
/* make sure we BUG if trying to hit standard
* register/unregister code path
*/
@@ -10379,8 +10365,29 @@ void init_dummy_netdev(struct net_device *dev)
* its refcount.
*/
}
-EXPORT_SYMBOL_GPL(init_dummy_netdev);
+EXPORT_SYMBOL_GPL(init_dummy_netdev_core);

+/**
+ * init_dummy_netdev - init a dummy network device for NAPI
+ * @dev: device to init
+ *
+ * This takes a network device structure and initialize the minimum
+ * amount of fields so it can be used to schedule NAPI polls without
+ * registering a full blown interface. This is to be used by drivers
+ * that need to tie several hardware interfaces to a single NAPI
+ * poll scheduler due to HW limitations.
+ */
+void init_dummy_netdev(struct net_device *dev)
+{
+ /* Clear everything. Note we don't initialize spinlocks
+ * are they aren't supposed to be taken by any of the
+ * NAPI code and this dummy netdev is supposed to be
+ * only ever used for NAPI polls
+ */
+ memset(dev, 0, sizeof(struct net_device));
+ init_dummy_netdev_core(dev);
+}
+EXPORT_SYMBOL_GPL(init_dummy_netdev);

/**
* register_netdev - register a network device