Received-SPF: pass (google.com: domain of linux-kernel+bounces-106105-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Mon, 18 Mar 2024 14:20:16 +0200
From: Leon Romanovsky <leon@kernel.org>
To: Breno Leitao <leitao@debian.org>
Cc: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>,
	Jason Gunthorpe <jgg@ziepe.ca>, kuba@kernel.org,
	keescook@chromium.org,
	"open list:HFI1 DRIVER" <linux-rdma@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3] IB/hfi1: allocate dummy net_device dynamically
Message-ID: <20240318122016.GA4341@unreal>
References: <20240318100858.3696961-1-leitao@debian.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240318100858.3696961-1-leitao@debian.org>

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/20240229225910.79e224cf@kernel.org/
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> Acked-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
> 
> 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
>