Re: [PATCH net-next] net: create a dummy net_device allocator
From: Breno Leitao
Date: Thu Mar 28 2024 - 13:46:54 EST
On Thu, Mar 28, 2024 at 10:10:53AM -0700, Jakub Kicinski wrote:
> On Thu, 28 Mar 2024 16:02:12 +0100 Alexander Lobakin wrote:
> > > +/**
> > > + * alloc_netdev_dummy - Allocate and initialize a dummy net device.
> > > + * @sizeof_priv: size of private data to allocate space for
> > > + * @name: device name format string
> > > + */
> > > +struct net_device *alloc_netdev_dummy(int sizeof_priv, const char *name)
> >
> > Since the users of init_dummy_netdev embed &net_device into their
> > private structures, do we need sizeof_priv here at all? Or maybe we
> > could unconditionally pass 0?
>
> FWIW similar thing could be said about @name, if we never intend to
> register the device - it will never have a legitimate (user visible)
> name. So we may be better off naming them "dummy#" or some such.
> No strong preference, tho. Adding params back later may be a bit
> of a pain.
Removing the @name seems to be safer than @sizeof_priv. I can remove it
in v2 if any one has any strong preference.
Unfortunately removing @sizeof_priv might not be possible given cases as
iwlwifi.
> > > +{
> > > + return alloc_netdev(sizeof_priv, name, NET_NAME_UNKNOWN,
> > > + init_dummy_netdev_core);
> > > +}
> > > +EXPORT_SYMBOL_GPL(alloc_netdev_dummy);
> > > +
> > > /**
> > > * synchronize_net - Synchronize with packet receive processing
> > > *
> >
> > As Jakub mentioned, you need to introduce consumers of the functionality
> > you add within the same series. Personally, I'd like to see a series
> > with agressive conversion of all the affected drivers from
> > init_dummy_netdev() to alloc_dummy_netdev() and final removal of
> > init_dummy_netdev() :D
>
> We can, and put it on a shared branch so other trees can also pull in
> the conversions. No preference on my side, tho. I think that Breno
> doesn't have any of the HW in question, so starting with one and making
> sure it works could be more "work conserving", than redoing all
> patches..
I would prefer to do the more conservative approach first and make sure
there is no major regression, and then complete the work once the risk
is low.