2024-03-18 10:12:18

by Breno Leitao

[permalink] [raw]
Subject: [PATCH v3] IB/hfi1: 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 hfi1_netdev_rx by converting it
into a pointer. Then use the leverage alloc_netdev() to allocate the
net_device object at hfi1_alloc_rx().

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

Signed-off-by: Breno Leitao <[email protected]>
Acked-by: Dennis Dalessandro <[email protected]>

v2:
* Free struct hfi1_netdev_rx allocation if alloc_netdev() fails
* Pass zero as the private size for alloc_netdev().
* Remove wrong reference for iwl in the comments

v3:
* Re-worded the comment, by removing the first paragraph.
---
drivers/infiniband/hw/hfi1/netdev.h | 2 +-
drivers/infiniband/hw/hfi1/netdev_rx.c | 10 ++++++++--
2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/netdev.h b/drivers/infiniband/hw/hfi1/netdev.h
index 8aa074670a9c..07c8f77c9181 100644
--- a/drivers/infiniband/hw/hfi1/netdev.h
+++ b/drivers/infiniband/hw/hfi1/netdev.h
@@ -49,7 +49,7 @@ struct hfi1_netdev_rxq {
* When 0 receive queues will be freed.
*/
struct hfi1_netdev_rx {
- struct net_device rx_napi;
+ struct net_device *rx_napi;
struct hfi1_devdata *dd;
struct hfi1_netdev_rxq *rxq;
int num_rx_q;
diff --git a/drivers/infiniband/hw/hfi1/netdev_rx.c b/drivers/infiniband/hw/hfi1/netdev_rx.c
index 720d4c85c9c9..cd6e78e257ef 100644
--- a/drivers/infiniband/hw/hfi1/netdev_rx.c
+++ b/drivers/infiniband/hw/hfi1/netdev_rx.c
@@ -188,7 +188,7 @@ static int hfi1_netdev_rxq_init(struct hfi1_netdev_rx *rx)
int i;
int rc;
struct hfi1_devdata *dd = rx->dd;
- struct net_device *dev = &rx->rx_napi;
+ struct net_device *dev = rx->rx_napi;

rx->num_rx_q = dd->num_netdev_contexts;
rx->rxq = kcalloc_node(rx->num_rx_q, sizeof(*rx->rxq),
@@ -360,7 +360,12 @@ int hfi1_alloc_rx(struct hfi1_devdata *dd)
if (!rx)
return -ENOMEM;
rx->dd = dd;
- init_dummy_netdev(&rx->rx_napi);
+ rx->rx_napi = alloc_netdev(0, "dummy", NET_NAME_UNKNOWN,
+ init_dummy_netdev);
+ if (!rx->rx_napi) {
+ kfree(rx);
+ return -ENOMEM;
+ }

xa_init(&rx->dev_tbl);
atomic_set(&rx->enabled, 0);
@@ -374,6 +379,7 @@ void hfi1_free_rx(struct hfi1_devdata *dd)
{
if (dd->netdev_rx) {
dd_dev_info(dd, "hfi1 rx freed\n");
+ free_netdev(dd->netdev_rx->rx_napi);
kfree(dd->netdev_rx);
dd->netdev_rx = NULL;
}
--
2.43.0



2024-03-18 12:23:10

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [PATCH v3] IB/hfi1: allocate dummy net_device dynamically

On Mon, Mar 18, 2024 at 03:08:57AM -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 struct hfi1_netdev_rx by converting it
> into a pointer. Then use the leverage alloc_netdev() to allocate the
> net_device object at hfi1_alloc_rx().
>
> [1] https://lore.kernel.org/all/[email protected]/
>
> Signed-off-by: Breno Leitao <[email protected]>
> Acked-by: Dennis Dalessandro <[email protected]>
>
> v2:
> * Free struct hfi1_netdev_rx allocation if alloc_netdev() fails
> * Pass zero as the private size for alloc_netdev().
> * Remove wrong reference for iwl in the comments
>
> v3:
> * Re-worded the comment, by removing the first paragraph.

Please put changelog below "---" marker.
It doesn't belong to commit message.

Thanks

> ---
> drivers/infiniband/hw/hfi1/netdev.h | 2 +-
> drivers/infiniband/hw/hfi1/netdev_rx.c | 10 ++++++++--
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/infiniband/hw/hfi1/netdev.h b/drivers/infiniband/hw/hfi1/netdev.h
> index 8aa074670a9c..07c8f77c9181 100644
> --- a/drivers/infiniband/hw/hfi1/netdev.h
> +++ b/drivers/infiniband/hw/hfi1/netdev.h
> @@ -49,7 +49,7 @@ struct hfi1_netdev_rxq {
> * When 0 receive queues will be freed.
> */
> struct hfi1_netdev_rx {
> - struct net_device rx_napi;
> + struct net_device *rx_napi;
> struct hfi1_devdata *dd;
> struct hfi1_netdev_rxq *rxq;
> int num_rx_q;
> diff --git a/drivers/infiniband/hw/hfi1/netdev_rx.c b/drivers/infiniband/hw/hfi1/netdev_rx.c
> index 720d4c85c9c9..cd6e78e257ef 100644
> --- a/drivers/infiniband/hw/hfi1/netdev_rx.c
> +++ b/drivers/infiniband/hw/hfi1/netdev_rx.c
> @@ -188,7 +188,7 @@ static int hfi1_netdev_rxq_init(struct hfi1_netdev_rx *rx)
> int i;
> int rc;
> struct hfi1_devdata *dd = rx->dd;
> - struct net_device *dev = &rx->rx_napi;
> + struct net_device *dev = rx->rx_napi;
>
> rx->num_rx_q = dd->num_netdev_contexts;
> rx->rxq = kcalloc_node(rx->num_rx_q, sizeof(*rx->rxq),
> @@ -360,7 +360,12 @@ int hfi1_alloc_rx(struct hfi1_devdata *dd)
> if (!rx)
> return -ENOMEM;
> rx->dd = dd;
> - init_dummy_netdev(&rx->rx_napi);
> + rx->rx_napi = alloc_netdev(0, "dummy", NET_NAME_UNKNOWN,
> + init_dummy_netdev);
> + if (!rx->rx_napi) {
> + kfree(rx);
> + return -ENOMEM;
> + }
>
> xa_init(&rx->dev_tbl);
> atomic_set(&rx->enabled, 0);
> @@ -374,6 +379,7 @@ void hfi1_free_rx(struct hfi1_devdata *dd)
> {
> if (dd->netdev_rx) {
> dd_dev_info(dd, "hfi1 rx freed\n");
> + free_netdev(dd->netdev_rx->rx_napi);
> kfree(dd->netdev_rx);
> dd->netdev_rx = NULL;
> }
> --
> 2.43.0
>