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/20240229225910.79e224cf@kernel.org/
Signed-off-by: Breno Leitao <leitao@debian.org>
---
.../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..ed33a201a0f5 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");
+ 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);
@@ -682,6 +690,7 @@ static void prestera_sdma_switch_fini(struct prestera_switch *sw)
prestera_sdma_tx_fini(sdma);
prestera_sdma_rx_fini(sdma);
dma_pool_destroy(sdma->desc_pool);
+ kfree(sdma->napi_dev);
}
static bool prestera_sdma_is_ready(struct prestera_sdma *sdma)
--
2.43.0
On Thu, Mar 28, 2024 at 04:52:02PM -0700, Breno Leitao wrote: > 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/20240229225910.79e224cf@kernel.org/ > > Signed-off-by: Breno Leitao <leitao@debian.org> > --- > .../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 ... > @@ -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"); > + goto err_alloc_dummy; Hi Breno, This goto will result in the function returning err. But err is 0 here. Perhaps it should be set to a negative error value instead? Flagged by Smatch. > + } > + > > - 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); ...
On Sun, Mar 31, 2024 at 09:54:38AM +0100, Simon Horman wrote: > On Thu, Mar 28, 2024 at 04:52:02PM -0700, Breno Leitao wrote: > > 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/20240229225910.79e224cf@kernel.org/ > > > > Signed-off-by: Breno Leitao <leitao@debian.org> > > --- > > .../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 > > ... > > > @@ -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"); > > + goto err_alloc_dummy; > > Hi Breno, > > This goto will result in the function returning err. > But err is 0 here. Perhaps it should be set to a negative error value > instead? Definitely, that was a good catch. > Flagged by Smatch. I am curious how you are running Smatch. I tried to run it here according to[1] , and I found different and valid errors also, that I will fix soon. For instance: drivers/net/ethernet/marvell/prestera/prestera_main.c:433 prestera_port_sfp_bind() error: uninitialized symbol 'err'. drivers/net/ethernet/marvell/prestera/prestera_main.c:861 prestera_switch_set_base_mac_addr() error: uninitialized symbol 'ret'. [1] https://rajanvaja.wordpress.com/2021/02/06/how-to-run-smatch-on-linux-kernel/ Thanks
On Thu, 28 Mar 2024 16:52:02 -0700 Breno Leitao wrote: > - 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"); > + goto err_alloc_dummy; > + } > + > duplicated empty line here > - 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); > @@ -682,6 +690,7 @@ static void prestera_sdma_switch_fini(struct prestera_switch *sw) > prestera_sdma_tx_fini(sdma); > prestera_sdma_rx_fini(sdma); > dma_pool_destroy(sdma->desc_pool); > + kfree(sdma->napi_dev); Why kfree()? Let's use free_netdev() consistently, in case one day we have to undo something alloc_netdev_dummy() has done.
On Fri, Mar 29, 2024 at 08:56:33AM -0700, Jakub Kicinski wrote: > > @@ -682,6 +690,7 @@ static void prestera_sdma_switch_fini(struct prestera_switch *sw) > > prestera_sdma_tx_fini(sdma); > > prestera_sdma_rx_fini(sdma); > > dma_pool_destroy(sdma->desc_pool); > > + kfree(sdma->napi_dev); > > Why kfree()? Let's use free_netdev() consistently, in case one day > we have to undo something alloc_netdev_dummy() has done. I should have used free_netdev() in fact. I will update.
© 2016 - 2024 Red Hat, Inc.