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:
v1:
* https://lore.kernel.org/all/[email protected]/
v2:
* Patch 1: Use a pre-defined name ("dummy#") for the dummy
net_devices.
* Patch 2-5: Added users for the new helper.
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.
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.
Breno Leitao (9):
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 | 57 ++++++++++++-------
19 files changed, 127 insertions(+), 53 deletions(-)
--
2.43.0
For dummy devices, exit earlier at free_netdev() instead of executing
the whole function. This is necessary, because dummy devices are
special, and shouldn't have the second part of the function executed.
Otherwise reg_state, which is NETREG_DUMMY for dummy devices, will be
overwritten and there will be no way to identify that this is a dummy
device. Also, this device do not need the final put_device(), since
dummy devices are not registered (through register_netdevice()), where
the device reference is increased (at netdev_register_kobject() ->
device_add()).
Suggested-by: Jakub Kicinski <[email protected]>
Signed-off-by: Breno Leitao <[email protected]>
---
net/core/dev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 92f5bddbc2de..bf0a335781aa 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -11051,7 +11051,8 @@ void free_netdev(struct net_device *dev)
dev->xdp_bulkq = NULL;
/* Compatibility with error handling in drivers */
- if (dev->reg_state == NETREG_UNINITIALIZED) {
+ if (dev->reg_state == NETREG_UNINITIALIZED ||
+ dev->reg_state == NETREG_DUMMY) {
netdev_freemem(dev);
return;
}
--
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]>
---
include/linux/netdevice.h | 3 +++
net/core/dev.c | 54 ++++++++++++++++++++++++++-------------
2 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0c198620ac93..544767d218c0 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4517,6 +4517,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 bf0a335781aa..5d2cb97d0ae6 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10413,25 +10413,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 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.
+/* 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
- * 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
*/
@@ -10452,8 +10439,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 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
@@ -11065,6 +11072,17 @@ 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
+ */
+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
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
On Tue, 9 Apr 2024 05:57:15 -0700 Breno Leitao wrote:
> For dummy devices, exit earlier at free_netdev() instead of executing
> the whole function. This is necessary, because dummy devices are
> special, and shouldn't have the second part of the function executed.
>
> Otherwise reg_state, which is NETREG_DUMMY for dummy devices, will be
> overwritten and there will be no way to identify that this is a dummy
> device. Also, this device do not need the final put_device(), since
> dummy devices are not registered (through register_netdevice()), where
> the device reference is increased (at netdev_register_kobject() ->
> device_add()).
There's a small fuzz when applying due to the phy topo changes
landing, please rebase, the CI didn't ingest it right.
--
pw-bot: cr
On Tue, Apr 09, 2024 at 05:57:16AM -0700, Breno Leitao wrote:
> 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]>
We were about to submit another user of init_dummy_netdev() when I
noticed this patch. Converted the code to use alloc_netdev_dummy() [1]
and it seems to be working fine. Will submit after your patch is
accepted.
See a few minor comments below.
[...]
> +/**
> + * init_dummy_netdev - init a dummy network device for NAPI
> + * @dev: device to init
> + *
> + * This takes a network device structure and initialize the minimum
s/initialize/initializes/
> + * 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
I assume you meant s/are/as/ ?
> + * 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);
[1]
diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index db2950baf6b4..bf66d996e32e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -132,20 +132,40 @@ struct mlxsw_pci {
u8 num_cqs; /* Number of CQs */
u8 num_sdqs; /* Number of SDQs */
bool skip_reset;
- struct net_device napi_dev_tx;
- struct net_device napi_dev_rx;
+ struct net_device *napi_dev_tx;
+ struct net_device *napi_dev_rx;
};
-static void mlxsw_pci_napi_devs_init(struct mlxsw_pci *mlxsw_pci)
+static int mlxsw_pci_napi_devs_init(struct mlxsw_pci *mlxsw_pci)
{
- init_dummy_netdev(&mlxsw_pci->napi_dev_tx);
- strscpy(mlxsw_pci->napi_dev_tx.name, "mlxsw_tx",
- sizeof(mlxsw_pci->napi_dev_tx.name));
+ int err;
+
+ mlxsw_pci->napi_dev_tx = alloc_netdev_dummy(0);
+ if (!mlxsw_pci->napi_dev_tx)
+ return -ENOMEM;
+ strscpy(mlxsw_pci->napi_dev_tx->name, "mlxsw_tx",
+ sizeof(mlxsw_pci->napi_dev_tx->name));
+
+ mlxsw_pci->napi_dev_rx = alloc_netdev_dummy(0);
+ if (!mlxsw_pci->napi_dev_rx) {
+ err = -ENOMEM;
+ goto err_alloc_rx;
+ }
+ strscpy(mlxsw_pci->napi_dev_rx->name, "mlxsw_rx",
+ sizeof(mlxsw_pci->napi_dev_rx->name));
+ dev_set_threaded(mlxsw_pci->napi_dev_rx, true);
+
+ return 0;
- init_dummy_netdev(&mlxsw_pci->napi_dev_rx);
- strscpy(mlxsw_pci->napi_dev_rx.name, "mlxsw_rx",
- sizeof(mlxsw_pci->napi_dev_rx.name));
- dev_set_threaded(&mlxsw_pci->napi_dev_rx, true);
+err_alloc_rx:
+ free_netdev(mlxsw_pci->napi_dev_tx);
+ return err;
+}
+
+static void mlxsw_pci_napi_devs_fini(struct mlxsw_pci *mlxsw_pci)
+{
+ free_netdev(mlxsw_pci->napi_dev_rx);
+ free_netdev(mlxsw_pci->napi_dev_tx);
}
static char *__mlxsw_pci_queue_elem_get(struct mlxsw_pci_queue *q,
@@ -804,11 +824,11 @@ static void mlxsw_pci_cq_napi_setup(struct mlxsw_pci_queue *q,
switch (cq_type) {
case MLXSW_PCI_CQ_SDQ:
- netif_napi_add(&mlxsw_pci->napi_dev_tx, &q->u.cq.napi,
+ netif_napi_add(mlxsw_pci->napi_dev_tx, &q->u.cq.napi,
mlxsw_pci_napi_poll_cq_tx);
break;
case MLXSW_PCI_CQ_RDQ:
- netif_napi_add(&mlxsw_pci->napi_dev_rx, &q->u.cq.napi,
+ netif_napi_add(mlxsw_pci->napi_dev_rx, &q->u.cq.napi,
mlxsw_pci_napi_poll_cq_rx);
break;
}
@@ -1793,7 +1813,10 @@ static int mlxsw_pci_init(void *bus_priv, struct mlxsw_core *mlxsw_core,
if (err)
goto err_requery_resources;
- mlxsw_pci_napi_devs_init(mlxsw_pci);
+ err = mlxsw_pci_napi_devs_init(mlxsw_pci);
+ if (err)
+ goto err_napi_devs_init;
+
err = mlxsw_pci_aqs_init(mlxsw_pci, mbox);
if (err)
goto err_aqs_init;
@@ -1811,6 +1834,8 @@ static int mlxsw_pci_init(void *bus_priv, struct mlxsw_core *mlxsw_core,
err_request_eq_irq:
mlxsw_pci_aqs_fini(mlxsw_pci);
err_aqs_init:
+ mlxsw_pci_napi_devs_fini(mlxsw_pci);
+err_napi_devs_init:
err_requery_resources:
err_config_profile:
err_cqe_v_check:
@@ -1838,6 +1863,7 @@ static void mlxsw_pci_fini(void *bus_priv)
free_irq(pci_irq_vector(mlxsw_pci->pdev, 0), mlxsw_pci);
mlxsw_pci_aqs_fini(mlxsw_pci);
+ mlxsw_pci_napi_devs_fini(mlxsw_pci);
mlxsw_pci_fw_area_fini(mlxsw_pci);
mlxsw_pci_free_irq_vectors(mlxsw_pci);
}
On Wed, Apr 10, 2024 at 02:10:04PM +0300, Ido Schimmel wrote:
> On Tue, Apr 09, 2024 at 05:57:16AM -0700, Breno Leitao wrote:
> > 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]>
>
> We were about to submit another user of init_dummy_netdev() when I
> noticed this patch. Converted the code to use alloc_netdev_dummy() [1]
> and it seems to be working fine. Will submit after your patch is
> accepted.
Thanks. It seems that this patch is close to get accepted. Let's see...
> See a few minor comments below.
>
> [...]
>
> > +/**
> > + * init_dummy_netdev - init a dummy network device for NAPI
> > + * @dev: device to init
> > + *
> > + * This takes a network device structure and initialize the minimum
>
> s/initialize/initializes/
>
> > + * 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
>
> I assume you meant s/are/as/ ?
Thanks for the feedback, I agree with all of them.
Since these lines were not introduced by this patch, and this patch is
just moving code (and comments) around, I would add a new patch to the
patch series fixing the grammar errors.