On Mon, Mar 11, 2024 at 03:08:54AM -0700, Breno Leitao wrote: > Hello Leon, > > On Sun, Mar 10, 2024 at 12:14:51PM +0200, Leon Romanovsky wrote: > > On Fri, Mar 08, 2024 at 10:29:50AM -0800, Breno Leitao wrote: > > > struct net_device shouldn't be embedded into any structure, instead, > > > the owner should use the priv space to embed their state into net_device. > > > > Why? > > 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. > > > > @@ -360,7 +360,11 @@ 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(sizeof(struct iwl_trans_pcie *), > > > + "dummy", NET_NAME_UNKNOWN, > > > > 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(...). > > > > + init_dummy_netdev); + if > > > (!rx->rx_napi) + return -ENOMEM; > > > > You forgot to release previously allocated "rx" here. > > Good catch, I will update. Thanks > > Thanks
