Re: [PATCH for-next 10/10] IB/hfi1: Rework AIP and VNIC dummy netdev usage

From: Jason Gunthorpe <jgg@nvidia.com>
To: dennis.dalessandro@cornelisnetworks.com
Cc: dledford@redhat.com, linux-rdma@vger.kernel.org,
	Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>
Subject: Re: [PATCH for-next 10/10] IB/hfi1: Rework AIP and VNIC dummy netdev usage
Date: Tue, 13 Apr 2021 13:59:04 -0300	[thread overview]
Message-ID: <20210413165904.GA1320285@nvidia.com> (raw)
In-Reply-To: <1617026056-50483-11-git-send-email-dennis.dalessandro@cornelisnetworks.com>

On Mon, Mar 29, 2021 at 09:54:16AM -0400, dennis.dalessandro@cornelisnetworks.com wrote:
> From: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>
> 
> All other users of the dummy netdevice embed the netdev in other
> structures:
> 
> init_dummy_netdev(&mal->dummy_dev);
> init_dummy_netdev(&eth->dummy_dev);
> init_dummy_netdev(&ar->napi_dev);
> init_dummy_netdev(&irq_grp->napi_ndev);
> init_dummy_netdev(&wil->napi_ndev);
> init_dummy_netdev(&trans_pcie->napi_dev);
> init_dummy_netdev(&dev->napi_dev);
> init_dummy_netdev(&bus->mux_dev);
> 
> The AIP and VNIC implementation turns that model inside out and used a
> kfree() to free what appears to be a netdev struct when in reality, it is
> a struct that enbodies the rx state as well as the dummy netdev used to
> support napi_poll across disparate receive contexts.  The relationship is
> infered by the odd allocation:
> 
> 	const int netdev_size = sizeof(*dd->dummy_netdev) +
> 		sizeof(struct hfi1_netdev_priv);
> 	<snip>
> 	dd->dummy_netdev = kcalloc_node(1, netdev_size, GFP_KERNEL, dd->node);
> 
> 
> Correct the issue by:
> - Correctly naming the alloc and free functions
> - Renaming hfi1_netdev_priv to hfi1_netdev_rx
> - Replacing dd dummy_netdev with a netdev_rx pointer
> - Embedding the net_device in hfi1_netdev_rx
> - Moving the init_dummy_netdev to the alloc routine
> - Adjusting wrappers to fit the new model
> 
> Fixes: 6991abcb993c ("IB/hfi1: Add functions to receive accelerated ipoib packets")
> Reviewed-by: Kaike Wan <kaike.wan@intel.com>
> Signed-off-by: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>
> Signed-off-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
> ---
>  drivers/infiniband/hw/hfi1/chip.c      |   6 +-
>  drivers/infiniband/hw/hfi1/hfi.h       |   4 +-
>  drivers/infiniband/hw/hfi1/init.c      |   2 +-
>  drivers/infiniband/hw/hfi1/netdev.h    |  39 +++-----
>  drivers/infiniband/hw/hfi1/netdev_rx.c | 177 +++++++++++++++++----------------
>  5 files changed, 108 insertions(+), 120 deletions(-)

Applied to for-next

Thanks,
Jason