struct net_device shouldn't be embedded into any structure, instead,
the owner should use the private space to embed their state into
net_device.
But, in some cases the net_device is embedded inside the private
structure, which blocks the usage of zero-length arrays inside
net_device.
Create a helper to allocate a dummy device at dynamically runtime, and
move the Ethernet devices to use it, instead of embedding the dummy
device inside the private structure.
This fixes all the network cases plus some wireless drivers.
PS: Due to lack of hardware, unfortunately most these patches are
compiled tested only, except ath11k that was kindly tested by Kalle Valo.
---
Changelog:
v7:
* Document the return value of alloc_netdev_dummy()
v6:
* No code change. Just added Reviewed-by: and fix a commit message
v5:
* Added a new patch to fix some typos in the previous code
* Rebased to net-net/main
v4:
* Added a new patch to add dummy device at free_netdev(), as suggested
by Jakub.
* Added support for some wireless driver.
* Added some Acked-by and Reviewed-by.
v3:
* Use free_netdev() instead of kfree() as suggested by Jakub.
* Change the free_netdev() place in ipa driver, as suggested by
Alex Elder.
* Set err in the error path in the Marvell driver, as suggested
by Simon Horman.
v2:
* Patch 1: Use a pre-defined name ("dummy#") for the dummy
net_devices.
* Patch 2-5: Added users for the new helper.
v1:
* https://lore.kernel.org/all/[email protected]/
Breno Leitao (10):
net: core: Fix documentation
net: free_netdev: exit earlier if dummy
net: create a dummy net_device allocator
net: marvell: prestera: allocate dummy net_device dynamically
net: mediatek: mtk_eth_sock: allocate dummy net_device dynamically
net: ipa: allocate dummy net_device dynamically
net: ibm/emac: allocate dummy net_device dynamically
wifi: qtnfmac: Use netdev dummy allocator helper
wifi: ath10k: allocate dummy net_device dynamically
wifi: ath11k: allocate dummy net_device dynamically
drivers/net/ethernet/ibm/emac/mal.c | 14 ++++-
drivers/net/ethernet/ibm/emac/mal.h | 2 +-
.../ethernet/marvell/prestera/prestera_rxtx.c | 15 ++++-
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 17 ++++--
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 2 +-
drivers/net/ipa/gsi.c | 12 ++--
drivers/net/ipa/gsi.h | 2 +-
drivers/net/wireless/ath/ath10k/core.c | 9 ++-
drivers/net/wireless/ath/ath10k/core.h | 2 +-
drivers/net/wireless/ath/ath10k/pci.c | 2 +-
drivers/net/wireless/ath/ath10k/sdio.c | 2 +-
drivers/net/wireless/ath/ath10k/snoc.c | 4 +-
drivers/net/wireless/ath/ath10k/usb.c | 2 +-
drivers/net/wireless/ath/ath11k/ahb.c | 9 ++-
drivers/net/wireless/ath/ath11k/core.h | 2 +-
drivers/net/wireless/ath/ath11k/pcic.c | 21 +++++--
.../wireless/quantenna/qtnfmac/pcie/pcie.c | 3 +-
include/linux/netdevice.h | 3 +
net/core/dev.c | 59 +++++++++++++------
19 files changed, 129 insertions(+), 53 deletions(-)
--
2.43.0
Fix bad grammar in description of init_dummy_netdev() function. This
topic showed up in the review of the "allocate dummy device dynamically"
patch set.
Suggested-by: Ido Schimmel <[email protected]>
Signed-off-by: Breno Leitao <[email protected]>
Reviewed-by: Ido Schimmel <[email protected]>
---
net/core/dev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 8bdc59074b29..44c2d698fd1b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10424,7 +10424,7 @@ 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
+ * This takes a network device structure and initializes 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
@@ -10433,7 +10433,7 @@ EXPORT_SYMBOL(register_netdevice);
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
+ * as 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
*/
--
2.43.0
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 causes
some problems as reported here:
https://lore.kernel.org/all/[email protected]/
Split the init_dummy_netdev() function in two. Create a new function called
init_dummy_netdev_core() that does not memzero the net_device structure.
Then have init_dummy_netdev() memzero-ing and calling
init_dummy_netdev_core(), keeping the old behaviour.
init_dummy_netdev_core() is the new function that could be called as an
argument for alloc_netdev().
Also, create a helper to allocate and initialize dummy net devices,
leveraging init_dummy_netdev_core() as the setup argument. This function
basically simplify the allocation of dummy devices, by allocating and
initializing it. Freeing the device continue to be done through
free_netdev()
Suggested-by: Jakub Kicinski <[email protected]>
Signed-off-by: Breno Leitao <[email protected]>
Reviewed-by: Ido Schimmel <[email protected]>
---
include/linux/netdevice.h | 3 +++
net/core/dev.c | 56 ++++++++++++++++++++++++++-------------
2 files changed, 41 insertions(+), 18 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d45f330d083d..f849e7d110ed 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4519,6 +4519,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);
+
/* 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 62b39d6b1d8f..e09aa3785c15 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10420,25 +10420,12 @@ 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 initializes 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.
+/* Initialize the core of a dummy net device.
+ * This is useful if you are calling this function after alloc_netdev(),
+ * since it does not memset the net_device fields.
*/
-void init_dummy_netdev(struct net_device *dev)
+static void init_dummy_netdev_core(struct net_device *dev)
{
- /* Clear everything. Note we don't initialize spinlocks
- * as 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
*/
@@ -10459,8 +10446,28 @@ void init_dummy_netdev(struct net_device *dev)
* its refcount.
*/
}
-EXPORT_SYMBOL_GPL(init_dummy_netdev);
+/**
+ * init_dummy_netdev - init a dummy network device for NAPI
+ * @dev: device to init
+ *
+ * This takes a network device structure and initializes 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
+ * as 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
@@ -11080,6 +11087,19 @@ void free_netdev(struct net_device *dev)
}
EXPORT_SYMBOL(free_netdev);
+/**
+ * alloc_netdev_dummy - Allocate and initialize a dummy net device.
+ * @sizeof_priv: size of private data to allocate space for
+ *
+ * Return: the allocated net_device on success, NULL otherwise
+ */
+struct net_device *alloc_netdev_dummy(int sizeof_priv)
+{
+ return alloc_netdev(sizeof_priv, "dummy#", NET_NAME_UNKNOWN,
+ init_dummy_netdev_core);
+}
+EXPORT_SYMBOL_GPL(alloc_netdev_dummy);
+
/**
* synchronize_net - Synchronize with packet receive processing
*
--
2.43.0
Embedding net_device into structures prohibits the usage of flexible
arrays in the net_device structure. For more details, see the discussion
at [1].
Un-embed the net_device from the private struct by converting it
into a pointer. Then use the leverage the new alloc_netdev_dummy()
helper to allocate and initialize dummy devices.
[1] https://lore.kernel.org/all/[email protected]/
Signed-off-by: Breno Leitao <[email protected]>
Acked-by: Elad Nachman <[email protected]>
---
.../net/ethernet/marvell/prestera/prestera_rxtx.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_rxtx.c b/drivers/net/ethernet/marvell/prestera/prestera_rxtx.c
index cc2a9ae794be..39d9bf82c115 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_rxtx.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_rxtx.c
@@ -96,7 +96,7 @@ struct prestera_sdma {
struct dma_pool *desc_pool;
struct work_struct tx_work;
struct napi_struct rx_napi;
- struct net_device napi_dev;
+ struct net_device *napi_dev;
u32 map_addr;
u64 dma_mask;
/* protect SDMA with concurrent access from multiple CPUs */
@@ -654,13 +654,21 @@ static int prestera_sdma_switch_init(struct prestera_switch *sw)
if (err)
goto err_evt_register;
- init_dummy_netdev(&sdma->napi_dev);
+ sdma->napi_dev = alloc_netdev_dummy(0);
+ if (!sdma->napi_dev) {
+ dev_err(dev, "not able to initialize dummy device\n");
+ err = -ENOMEM;
+ goto err_alloc_dummy;
+ }
- netif_napi_add(&sdma->napi_dev, &sdma->rx_napi, prestera_sdma_rx_poll);
+ netif_napi_add(sdma->napi_dev, &sdma->rx_napi, prestera_sdma_rx_poll);
napi_enable(&sdma->rx_napi);
return 0;
+err_alloc_dummy:
+ prestera_hw_event_handler_unregister(sw, PRESTERA_EVENT_TYPE_RXTX,
+ prestera_rxtx_handle_event);
err_evt_register:
err_tx_init:
prestera_sdma_tx_fini(sdma);
@@ -677,6 +685,7 @@ static void prestera_sdma_switch_fini(struct prestera_switch *sw)
napi_disable(&sdma->rx_napi);
netif_napi_del(&sdma->rx_napi);
+ free_netdev(sdma->napi_dev);
prestera_hw_event_handler_unregister(sw, PRESTERA_EVENT_TYPE_RXTX,
prestera_rxtx_handle_event);
prestera_sdma_tx_fini(sdma);
--
2.43.0
Embedding net_device into structures prohibits the usage of flexible
arrays in the net_device structure. For more details, see the discussion
at [1].
Un-embed the net_device from the private struct by converting it
into a pointer. Then use the leverage the new alloc_netdev_dummy()
helper to allocate and initialize dummy devices.
[1] https://lore.kernel.org/all/[email protected]/
Signed-off-by: Breno Leitao <[email protected]>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 17 +++++++++++++----
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 2 +-
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index caa13b9cedff..d7a96dc11c07 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1710,7 +1710,7 @@ static struct page_pool *mtk_create_page_pool(struct mtk_eth *eth,
if (IS_ERR(pp))
return pp;
- err = __xdp_rxq_info_reg(xdp_q, ð->dummy_dev, id,
+ err = __xdp_rxq_info_reg(xdp_q, eth->dummy_dev, id,
eth->rx_napi.napi_id, PAGE_SIZE);
if (err < 0)
goto err_free_pp;
@@ -4188,6 +4188,8 @@ static int mtk_free_dev(struct mtk_eth *eth)
metadata_dst_free(eth->dsa_meta[i]);
}
+ free_netdev(eth->dummy_dev);
+
return 0;
}
@@ -4983,9 +4985,14 @@ static int mtk_probe(struct platform_device *pdev)
/* we run 2 devices on the same DMA ring so we need a dummy device
* for NAPI to work
*/
- init_dummy_netdev(ð->dummy_dev);
- netif_napi_add(ð->dummy_dev, ð->tx_napi, mtk_napi_tx);
- netif_napi_add(ð->dummy_dev, ð->rx_napi, mtk_napi_rx);
+ eth->dummy_dev = alloc_netdev_dummy(0);
+ if (!eth->dummy_dev) {
+ err = -ENOMEM;
+ dev_err(eth->dev, "failed to allocated dummy device\n");
+ goto err_unreg_netdev;
+ }
+ netif_napi_add(eth->dummy_dev, ð->tx_napi, mtk_napi_tx);
+ netif_napi_add(eth->dummy_dev, ð->rx_napi, mtk_napi_rx);
platform_set_drvdata(pdev, eth);
schedule_delayed_work(ð->reset.monitor_work,
@@ -4993,6 +5000,8 @@ static int mtk_probe(struct platform_device *pdev)
return 0;
+err_unreg_netdev:
+ mtk_unreg_dev(eth);
err_deinit_ppe:
mtk_ppe_deinit(eth);
mtk_mdio_cleanup(eth);
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 9ae3b8a71d0e..723fc637027c 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -1242,7 +1242,7 @@ struct mtk_eth {
spinlock_t page_lock;
spinlock_t tx_irq_lock;
spinlock_t rx_irq_lock;
- struct net_device dummy_dev;
+ struct net_device *dummy_dev;
struct net_device *netdev[MTK_MAX_DEVS];
struct mtk_mac *mac[MTK_MAX_DEVS];
int irq[3];
--
2.43.0
Embedding net_device into structures prohibits the usage of flexible
arrays in the net_device structure. For more details, see the discussion
at [1].
Un-embed the net_device from the private struct by converting it
into a pointer. Then use the leverage the new alloc_netdev_dummy()
helper to allocate and initialize dummy devices.
[1] https://lore.kernel.org/all/[email protected]/
Signed-off-by: Breno Leitao <[email protected]>
---
drivers/net/ethernet/ibm/emac/mal.c | 14 +++++++++++---
drivers/net/ethernet/ibm/emac/mal.h | 2 +-
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/emac/mal.c b/drivers/net/ethernet/ibm/emac/mal.c
index 2439f7e96e05..d92dd9c83031 100644
--- a/drivers/net/ethernet/ibm/emac/mal.c
+++ b/drivers/net/ethernet/ibm/emac/mal.c
@@ -605,9 +605,13 @@ static int mal_probe(struct platform_device *ofdev)
INIT_LIST_HEAD(&mal->list);
spin_lock_init(&mal->lock);
- init_dummy_netdev(&mal->dummy_dev);
+ mal->dummy_dev = alloc_netdev_dummy(0);
+ if (!mal->dummy_dev) {
+ err = -ENOMEM;
+ goto fail_unmap;
+ }
- netif_napi_add_weight(&mal->dummy_dev, &mal->napi, mal_poll,
+ netif_napi_add_weight(mal->dummy_dev, &mal->napi, mal_poll,
CONFIG_IBM_EMAC_POLL_WEIGHT);
/* Load power-on reset defaults */
@@ -637,7 +641,7 @@ static int mal_probe(struct platform_device *ofdev)
GFP_KERNEL);
if (mal->bd_virt == NULL) {
err = -ENOMEM;
- goto fail_unmap;
+ goto fail_dummy;
}
for (i = 0; i < mal->num_tx_chans; ++i)
@@ -703,6 +707,8 @@ static int mal_probe(struct platform_device *ofdev)
free_irq(mal->serr_irq, mal);
fail2:
dma_free_coherent(&ofdev->dev, bd_size, mal->bd_virt, mal->bd_dma);
+ fail_dummy:
+ free_netdev(mal->dummy_dev);
fail_unmap:
dcr_unmap(mal->dcr_host, 0x100);
fail:
@@ -734,6 +740,8 @@ static void mal_remove(struct platform_device *ofdev)
mal_reset(mal);
+ free_netdev(mal->dummy_dev);
+
dma_free_coherent(&ofdev->dev,
sizeof(struct mal_descriptor) *
(NUM_TX_BUFF * mal->num_tx_chans +
diff --git a/drivers/net/ethernet/ibm/emac/mal.h b/drivers/net/ethernet/ibm/emac/mal.h
index d212373a72e7..e0ddc41186a2 100644
--- a/drivers/net/ethernet/ibm/emac/mal.h
+++ b/drivers/net/ethernet/ibm/emac/mal.h
@@ -205,7 +205,7 @@ struct mal_instance {
int index;
spinlock_t lock;
- struct net_device dummy_dev;
+ struct net_device *dummy_dev;
unsigned int features;
};
--
2.43.0
There is a new dummy netdev allocator, use it instead of
alloc_netdev()/init_dummy_netdev combination.
Using alloc_netdev() with init_dummy_netdev might cause some memory
corruption at the driver removal side.
Fixes: 61cdb09ff760 ("wifi: qtnfmac: allocate dummy net_device dynamically")
Signed-off-by: Breno Leitao <[email protected]>
---
drivers/net/wireless/quantenna/qtnfmac/pcie/pcie.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/quantenna/qtnfmac/pcie/pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pcie/pcie.c
index f8f55db2f454..f66eb43094d4 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pcie/pcie.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/pcie/pcie.c
@@ -372,8 +372,7 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
goto error;
}
- bus->mux_dev = alloc_netdev(0, "dummy", NET_NAME_UNKNOWN,
- init_dummy_netdev);
+ bus->mux_dev = alloc_netdev_dummy(0);
if (!bus->mux_dev) {
ret = -ENOMEM;
goto error;
--
2.43.0
Breno Leitao <[email protected]> writes:
> There is a new dummy netdev allocator, use it instead of
> alloc_netdev()/init_dummy_netdev combination.
>
> Using alloc_netdev() with init_dummy_netdev might cause some memory
> corruption at the driver removal side.
>
> Fixes: 61cdb09ff760 ("wifi: qtnfmac: allocate dummy net_device dynamically")
> Signed-off-by: Breno Leitao <[email protected]>
I assume this goes via net-next:
Acked-by: Kalle Valo <[email protected]>
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Hello:
This series was applied to netdev/net-next.git (main)
by David S. Miller <[email protected]>:
On Mon, 22 Apr 2024 05:38:53 -0700 you wrote:
> struct net_device shouldn't be embedded into any structure, instead,
> the owner should use the private space to embed their state into
> net_device.
>
> But, in some cases the net_device is embedded inside the private
> structure, which blocks the usage of zero-length arrays inside
> net_device.
>
> [...]
Here is the summary with links:
- [net-next,v7,01/10] net: core: Fix documentation
https://git.kernel.org/netdev/net-next/c/c6e7f276841d
- [net-next,v7,02/10] net: free_netdev: exit earlier if dummy
https://git.kernel.org/netdev/net-next/c/f8d05679fb3f
- [net-next,v7,03/10] net: create a dummy net_device allocator
https://git.kernel.org/netdev/net-next/c/c661050f93d3
- [net-next,v7,04/10] net: marvell: prestera: allocate dummy net_device dynamically
https://git.kernel.org/netdev/net-next/c/ec24c06eb312
- [net-next,v7,05/10] net: mediatek: mtk_eth_sock: allocate dummy net_device dynamically
https://git.kernel.org/netdev/net-next/c/b209bd6d0bff
- [net-next,v7,06/10] net: ipa: allocate dummy net_device dynamically
https://git.kernel.org/netdev/net-next/c/1bdab0ee635d
- [net-next,v7,07/10] net: ibm/emac: allocate dummy net_device dynamically
https://git.kernel.org/netdev/net-next/c/2eb5e25d8495
- [net-next,v7,08/10] wifi: qtnfmac: Use netdev dummy allocator helper
https://git.kernel.org/netdev/net-next/c/4a8b77eff7e5
- [net-next,v7,09/10] wifi: ath10k: allocate dummy net_device dynamically
https://git.kernel.org/netdev/net-next/c/57738dab12d9
- [net-next,v7,10/10] wifi: ath11k: allocate dummy net_device dynamically
https://git.kernel.org/netdev/net-next/c/bca592ead825
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html