Received-SPF: pass (google.com: domain of linux-kernel+bounces-99923-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 12 Mar 2024 09:52:36 +0200
From: Leon Romanovsky <leon@kernel.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Breno Leitao <leitao@debian.org>,
	Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>,
	Jason Gunthorpe <jgg@ziepe.ca>, keescook@chromium.org,
	"open list:HFI1 DRIVER" <linux-rdma@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next] IB/hfi1: allocate dummy net_device dynamically
Message-ID: <20240312075236.GL12921@unreal>
References: <20240308182951.2137779-1-leitao@debian.org>
 <20240310101451.GD12921@unreal>
 <Ze7YNu5TrzClQcxy@gmail.com>
 <20240311102251.GJ12921@unreal>
 <20240311112532.71f1cb35@kernel.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20240311112532.71f1cb35@kernel.org>

On Mon, Mar 11, 2024 at 11:25:32AM -0700, Jakub Kicinski wrote:
> On Mon, 11 Mar 2024 12:22:51 +0200 Leon Romanovsky wrote:
> > > From my experience, you can leverage all the helpers to deal with the
> > > relationship between struct net_device and you private structure. Here
> > > are some examples that comes to my mind:
> > > 
> > > * alloc_netdev() allocates the private structure for you
> > > * netdev_priv() gets the private structure for you
> > > * dev->priv_destructor sets the destructure to be called when the
> > >   interface goes away or failures.  
> > 
> > Everything above is true, but it doesn't relevant to HFI1 devices which
> > are not netdev devices.
> 
> Why are they abusing struct net_device then?

Care to explain what is the abuse here?
HFI1 uses init_dummy_netdev() exactly as it should be used according to
the commit message.

This netdevice is controlled by HFI1 lifetime model and used for NAPI.
https://lore.kernel.org/netdev/1231906495.22571.79.camel@pasglop/

> If you're willing to take care of removing the use of NAPI from this
> driver completely, that'd be great.

I have no plans to remove NAPI from HFI1 driver. We are not against to
accept Bruno's removal patch, but we are asking for justification
in commit message.

> 
> > > > Will it create multiple "dummy" netdev in the system? Will all devices
> > > > have the same "dummy" name?  
> > > 
> > > Are these devices visible to userspace?  
> > 
> > HFI devices yes, dummy device no.
> > 
> > > 
> > > This allocation are using NET_NAME_UNKNOWN, which implies that the
> > > device is not expose to userspace.  
> > 
> > Great
> > 
> > > 
> > > Would you prefer a different name?  
> > 
> > I prefer to see some new wrapper over plain alloc_netdev, which will
> > create this dummy netdevice. For example, alloc_dummy_netdev(...).
> 
> Nope, no bona fide APIs for hacky uses.

Interesting position, instead of making simple and self descriptive API
which is impossible to misuse, you prefer complexity.

HFI1 is not alone in using init_dummy_netdev():
➜  kernel git:(rdma-next) ✗ git grep "init_dummy_netdev(&"
rivers/infiniband/hw/hfi1/netdev_rx.c: init_dummy_netdev(&rx->rx_napi);
drivers/net/ethernet/ibm/emac/mal.c:    init_dummy_netdev(&mal->dummy_dev);
drivers/net/ethernet/marvell/prestera/prestera_rxtx.c:  init_dummy_netdev(&sdma->napi_dev);
drivers/net/ethernet/mediatek/mtk_eth_soc.c:    init_dummy_netdev(&eth->dummy_dev);
drivers/net/ipa/gsi.c:  init_dummy_netdev(&gsi->dummy_dev);
drivers/net/wireless/ath/ath10k/core.c: init_dummy_netdev(&ar->napi_dev);
drivers/net/wireless/ath/ath11k/ahb.c:          init_dummy_netdev(&irq_grp->napi_ndev);
drivers/net/wireless/ath/ath11k/pcic.c:         init_dummy_netdev(&irq_grp->napi_ndev);
drivers/net/wireless/ath/ath12k/pci.c:          init_dummy_netdev(&irq_grp->napi_ndev);
drivers/net/wireless/ath/wil6210/netdev.c:      init_dummy_netdev(&wil->napi_ndev);
drivers/net/wireless/intel/iwlwifi/pcie/trans.c:                init_dummy_netdev(&trans_pcie->napi_dev);
drivers/net/wireless/mediatek/mt76/dma.c:       init_dummy_netdev(&dev->napi_dev);
drivers/net/wireless/mediatek/mt76/dma.c:       init_dummy_netdev(&dev->tx_napi_dev);
drivers/net/wireless/quantenna/qtnfmac/pcie/pcie.c:     init_dummy_netdev(&bus->mux_dev);
drivers/net/wireless/realtek/rtw88/pci.c:       init_dummy_netdev(&rtwpci->netdev);
drivers/net/wireless/realtek/rtw89/core.c:      init_dummy_netdev(&rtwdev->netdev);
drivers/net/wwan/t7xx/t7xx_netdev.c:    init_dummy_netdev(&ctlb->dummy_dev);
net/mptcp/protocol.c:   init_dummy_netdev(&mptcp_napi_dev);
net/xfrm/xfrm_input.c:  init_dummy_netdev(&xfrm_napi_dev);

Thanks