2024-04-10 13:19:07

by Breno Leitao

[permalink] [raw]
Subject: [PATCH net-next v5 00/10] allocate dummy device dynamically

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.
v5:
* Added a new patch to fix some typos in the previous code,
suggested by Ido.
* Rebased to net-net/main


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 | 57 ++++++++++++-------
19 files changed, 127 insertions(+), 53 deletions(-)

--
2.43.0



2024-04-10 13:19:58

by Breno Leitao

[permalink] [raw]
Subject: [PATCH net-next v5 02/10] net: free_netdev: exit earlier if dummy

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, 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 987039ffa63c..c74b42bc6888 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -11060,7 +11060,8 @@ void free_netdev(struct net_device *dev)
phy_link_topo_destroy(dev->link_topo);

/* 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


2024-04-10 13:21:10

by Breno Leitao

[permalink] [raw]
Subject: [PATCH net-next v5 06/10] net: ipa: allocate dummy net_device dynamically

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/ipa/gsi.c | 12 ++++++++----
drivers/net/ipa/gsi.h | 2 +-
2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 9a0b1fe4a93a..d70be15e95a6 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -1730,10 +1730,10 @@ static int gsi_channel_setup_one(struct gsi *gsi, u32 channel_id)
gsi_channel_program(channel, true);

if (channel->toward_ipa)
- netif_napi_add_tx(&gsi->dummy_dev, &channel->napi,
+ netif_napi_add_tx(gsi->dummy_dev, &channel->napi,
gsi_channel_poll);
else
- netif_napi_add(&gsi->dummy_dev, &channel->napi,
+ netif_napi_add(gsi->dummy_dev, &channel->napi,
gsi_channel_poll);

return 0;
@@ -2369,12 +2369,14 @@ int gsi_init(struct gsi *gsi, struct platform_device *pdev,
/* GSI uses NAPI on all channels. Create a dummy network device
* for the channel NAPI contexts to be associated with.
*/
- init_dummy_netdev(&gsi->dummy_dev);
+ gsi->dummy_dev = alloc_netdev_dummy(0);
+ if (!gsi->dummy_dev)
+ return -ENOMEM;
init_completion(&gsi->completion);

ret = gsi_reg_init(gsi, pdev);
if (ret)
- return ret;
+ goto err_reg_exit;

ret = gsi_irq_init(gsi, pdev); /* No matching exit required */
if (ret)
@@ -2389,6 +2391,7 @@ int gsi_init(struct gsi *gsi, struct platform_device *pdev,
return 0;

err_reg_exit:
+ free_netdev(gsi->dummy_dev);
gsi_reg_exit(gsi);

return ret;
@@ -2399,6 +2402,7 @@ void gsi_exit(struct gsi *gsi)
{
mutex_destroy(&gsi->mutex);
gsi_channel_exit(gsi);
+ free_netdev(gsi->dummy_dev);
gsi_reg_exit(gsi);
}

diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h
index 42063b227c18..6b7ec2a39676 100644
--- a/drivers/net/ipa/gsi.h
+++ b/drivers/net/ipa/gsi.h
@@ -155,7 +155,7 @@ struct gsi {
struct mutex mutex; /* protects commands, programming */
struct gsi_channel channel[GSI_CHANNEL_COUNT_MAX];
struct gsi_evt_ring evt_ring[GSI_EVT_RING_COUNT_MAX];
- struct net_device dummy_dev; /* needed for NAPI */
+ struct net_device *dummy_dev; /* needed for NAPI */
};

/**
--
2.43.0


2024-04-10 13:24:16

by Breno Leitao

[permalink] [raw]
Subject: [PATCH net-next v5 05/10] net: mediatek: mtk_eth_sock: allocate dummy net_device dynamically

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, &eth->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(&eth->dummy_dev);
- netif_napi_add(&eth->dummy_dev, &eth->tx_napi, mtk_napi_tx);
- netif_napi_add(&eth->dummy_dev, &eth->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, &eth->tx_napi, mtk_napi_tx);
+ netif_napi_add(eth->dummy_dev, &eth->rx_napi, mtk_napi_rx);

platform_set_drvdata(pdev, eth);
schedule_delayed_work(&eth->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


2024-04-10 13:25:15

by Breno Leitao

[permalink] [raw]
Subject: [PATCH net-next v5 09/10] wifi: ath10k: allocate dummy net_device dynamically

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 struct ath10k by converting it
into a pointer. Then use the leverage alloc_netdev() to allocate the
net_device object at ath10k_core_create(). The free of the device occurs
at ath10k_core_destroy().

[1] https://lore.kernel.org/all/[email protected]/

Signed-off-by: Breno Leitao <[email protected]>
---
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 +-
6 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 9ce6f49ab261..8663822e0b8d 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -3673,11 +3673,13 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
INIT_WORK(&ar->set_coverage_class_work,
ath10k_core_set_coverage_class_work);

- init_dummy_netdev(&ar->napi_dev);
+ ar->napi_dev = alloc_netdev_dummy(0);
+ if (!ar->napi_dev)
+ goto err_free_tx_complete;

ret = ath10k_coredump_create(ar);
if (ret)
- goto err_free_tx_complete;
+ goto err_free_netdev;

ret = ath10k_debug_create(ar);
if (ret)
@@ -3687,6 +3689,8 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,

err_free_coredump:
ath10k_coredump_destroy(ar);
+err_free_netdev:
+ free_netdev(ar->napi_dev);
err_free_tx_complete:
destroy_workqueue(ar->workqueue_tx_complete);
err_free_aux_wq:
@@ -3708,6 +3712,7 @@ void ath10k_core_destroy(struct ath10k *ar)

destroy_workqueue(ar->workqueue_tx_complete);

+ free_netdev(ar->napi_dev);
ath10k_debug_destroy(ar);
ath10k_coredump_destroy(ar);
ath10k_htt_tx_destroy(&ar->htt);
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index c110d15528bd..26003b519574 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -1269,7 +1269,7 @@ struct ath10k {
struct ath10k_per_peer_tx_stats peer_tx_stats;

/* NAPI */
- struct net_device napi_dev;
+ struct net_device *napi_dev;
struct napi_struct napi;

struct work_struct set_coverage_class_work;
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 5c34b156b4ff..558bec96ae40 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -3217,7 +3217,7 @@ static void ath10k_pci_free_irq(struct ath10k *ar)

void ath10k_pci_init_napi(struct ath10k *ar)
{
- netif_napi_add(&ar->napi_dev, &ar->napi, ath10k_pci_napi_poll);
+ netif_napi_add(ar->napi_dev, &ar->napi, ath10k_pci_napi_poll);
}

static int ath10k_pci_init_irq(struct ath10k *ar)
diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index 0ab5433f6cf6..e28f2fe1101b 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -2532,7 +2532,7 @@ static int ath10k_sdio_probe(struct sdio_func *func,
return -ENOMEM;
}

- netif_napi_add(&ar->napi_dev, &ar->napi, ath10k_sdio_napi_poll);
+ netif_napi_add(ar->napi_dev, &ar->napi, ath10k_sdio_napi_poll);

ath10k_dbg(ar, ATH10K_DBG_BOOT,
"sdio new func %d vendor 0x%x device 0x%x block 0x%x/0x%x\n",
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index 2c39bad7ebfb..0449b9ffc32d 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -935,7 +935,7 @@ static int ath10k_snoc_hif_start(struct ath10k *ar)

bitmap_clear(ar_snoc->pending_ce_irqs, 0, CE_COUNT_MAX);

- dev_set_threaded(&ar->napi_dev, true);
+ dev_set_threaded(ar->napi_dev, true);
ath10k_core_napi_enable(ar);
ath10k_snoc_irq_enable(ar);
ath10k_snoc_rx_post(ar);
@@ -1253,7 +1253,7 @@ static int ath10k_snoc_napi_poll(struct napi_struct *ctx, int budget)

static void ath10k_snoc_init_napi(struct ath10k *ar)
{
- netif_napi_add(&ar->napi_dev, &ar->napi, ath10k_snoc_napi_poll);
+ netif_napi_add(ar->napi_dev, &ar->napi, ath10k_snoc_napi_poll);
}

static int ath10k_snoc_request_irq(struct ath10k *ar)
diff --git a/drivers/net/wireless/ath/ath10k/usb.c b/drivers/net/wireless/ath/ath10k/usb.c
index 3c482baacec1..3b51b7f52130 100644
--- a/drivers/net/wireless/ath/ath10k/usb.c
+++ b/drivers/net/wireless/ath/ath10k/usb.c
@@ -1014,7 +1014,7 @@ static int ath10k_usb_probe(struct usb_interface *interface,
return -ENOMEM;
}

- netif_napi_add(&ar->napi_dev, &ar->napi, ath10k_usb_napi_poll);
+ netif_napi_add(ar->napi_dev, &ar->napi, ath10k_usb_napi_poll);

usb_get_dev(dev);
vendor_id = le16_to_cpu(dev->descriptor.idVendor);
--
2.43.0


2024-04-10 13:39:39

by Breno Leitao

[permalink] [raw]
Subject: [PATCH net-next v5 07/10] net: ibm/emac: allocate dummy net_device dynamically

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


2024-04-10 16:22:57

by Ido Schimmel

[permalink] [raw]
Subject: Re: [PATCH net-next v5 02/10] net: free_netdev: exit earlier if dummy

On Wed, Apr 10, 2024 at 06:13:43AM -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, 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]>

Reviewed-by: Ido Schimmel <[email protected]>