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 except for wireless drivers.
PS: Due to lack of hardware, unfortunately all these patches are
compiled tested only.
---
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.
Breno Leitao (5):
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
drivers/net/ethernet/ibm/emac/mal.c | 13 +++--
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 +-
include/linux/netdevice.h | 3 ++
net/core/dev.c | 54 ++++++++++++-------
9 files changed, 85 insertions(+), 35 deletions(-)
--
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]>
---
.../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/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
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
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 818699dea9d7..4d0109f2fe80 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10412,25 +10412,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
*/
@@ -10451,8 +10438,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
@@ -11063,6 +11070,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
> -----Original Message-----
> From: Breno Leitao <[email protected]>
> Sent: Thursday, April 4, 2024 2:49 PM
> To: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; linux-arm-
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; Taras Chornyi <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: [EXTERNAL] [PATCH net-next v3 2/5] net: marvell: prestera: allocate
> dummy net_device dynamically
>
> Prioritize security for external emails: Confirm sender and content safety
> before clicking links or opening attachments
>
> ----------------------------------------------------------------------
> 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://urldefense.proofpoint.com/v2/url?u=https-
> 3A__lore.kernel.org_all_20240229225910.79e224cf-
> 40kernel.org_&d=DwIDAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=eTeNTLEK5-
> TxXczjOcKPhANIFtlB9pP4lq9qhdlFrwQ&m=8Fqw_UUg1WoXjQcCjAtMa9J6QAd
> lmUzCuk3nWriMXtYMbgBqNpdDSO4rBUanJDxw&s=lXBodHe9jPOjWAOTFCjnZ
> 2ZDZYoF79OGsWN38gbxhTE&e=
>
> Signed-off-by: Breno Leitao <[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
>
Acked-by: Elad Nachman <[email protected]>
Breno Leitao <[email protected]> writes:
> 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 except for wireless drivers.
>
> PS: Due to lack of hardware, unfortunately all these patches are
> compiled tested only.
BTW if it helps, and if you have an ath10k or ath11k patch already, I
can run a quick test on real hardware.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Hello Kalle,
On Thu, Apr 04, 2024 at 02:59:59PM +0300, Kalle Valo wrote:
> Breno Leitao <[email protected]> writes:
>
> > 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 except for wireless drivers.
> >
> > PS: Due to lack of hardware, unfortunately all these patches are
> > compiled tested only.
>
> BTW if it helps, and if you have an ath10k or ath11k patch already, I
> can run a quick test on real hardware.
That would be very much appreciated! Thanks!
I don't have them ready yet, but, I will work on them soon and I will
send it to you probably tomorrow.
Should I send them as RFC, or as a regular patch, and we iterate over?
What would you prefer?
Thanks!
Breno Leitao <[email protected]> writes:
> Hello Kalle,
>
> On Thu, Apr 04, 2024 at 02:59:59PM +0300, Kalle Valo wrote:
>> Breno Leitao <[email protected]> writes:
>>
>> > 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 except for wireless drivers.
>> >
>> > PS: Due to lack of hardware, unfortunately all these patches are
>> > compiled tested only.
>>
>> BTW if it helps, and if you have an ath10k or ath11k patch already, I
>> can run a quick test on real hardware.
>
> That would be very much appreciated! Thanks!
>
> I don't have them ready yet, but, I will work on them soon and I will
> send it to you probably tomorrow.
>
> Should I send them as RFC, or as a regular patch, and we iterate over?
> What would you prefer?
A regular patch, like you did last time with ath11k, is fine for me. But
please do add a lore or patchwork link to the depency patchset so that
I'm testing with correct patches.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
From: Breno Leitao <[email protected]>
Date: Thu, 4 Apr 2024 04:48:41 -0700
> 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()
[...]
> @@ -11063,6 +11070,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)
Repeating my question from the previous thread: I see that in your
series you always pass 0 as @sizeof_priv, does it make sense to have
this argument or we can just pass 0 here to alloc_netdev() unconditionally?
Drivers that have &net_device embedded can't have any private data there
anyway.
> +{
> + 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
> *
Thanks,
Olek
Hello Alexander,
On Thu, Apr 04, 2024 at 06:40:33PM +0200, Alexander Lobakin wrote:
> > @@ -11063,6 +11070,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)
>
> Repeating my question from the previous thread: I see that in your
> series you always pass 0 as @sizeof_priv, does it make sense to have
> this argument or we can just pass 0 here to alloc_netdev() unconditionally?
> Drivers that have &net_device embedded can't have any private data there
> anyway.
Sorry, I answered you this question in the previous thread, and gave you an
example why we need the @sizeof_priv.
https://lore.kernel.org/all/ZgWrcvKyAzDvl%[email protected]/
I didn't see any reply from you, so, I suppose you were OK with it, but
maybe you missed it?!
Anyway, let me paste what I've sent there here, so, we can continue the
discussion in this thread, which might make the reviewing process here.
This is what I wrote in the thread/message above:
We need to have this private size for cases where we need to get the
pointer to their private structure given the dummy device. In the
embedded case you can use container_of(), but, with a pointer to
net_device, that is not the case anymore, and we should use the dummy private
data pointer back to the private data.
For instance, see the example of iwlwifi:
https://lore.kernel.org/all/[email protected]/