Received-SPF: pass (google.com: domain of linux-kernel+bounces-123372-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Thu, 28 Mar 2024 10:40:02 -0700
From: Breno Leitao <leitao@debian.org>
To: Alexander Lobakin <aleksander.lobakin@intel.com>
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	quic_jjohnson@quicinc.com, kvalo@kernel.org, leon@kernel.org,
	dennis.dalessandro@cornelisnetworks.com,
	Jiri Pirko <jiri@resnulli.us>, Simon Horman <horms@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Lorenzo Bianconi <lorenzo@kernel.org>,
	Coco Li <lixiaoyan@google.com>,
	"open list:NETWORKING DRIVERS" <netdev@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next] net: create a dummy net_device allocator
Message-ID: <ZgWrcvKyAzDvl/jn@gmail.com>
References: <20240327200809.512867-1-leitao@debian.org>
 <10d89693-21af-4560-a088-d58d16cbb9dd@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <10d89693-21af-4560-a088-d58d16cbb9dd@intel.com>

On Thu, Mar 28, 2024 at 04:02:12PM +0100, Alexander Lobakin wrote:
> From: Breno Leitao <leitao@debian.org>
> Date: Wed, 27 Mar 2024 13:08:04 -0700

> > @@ -10991,6 +10998,18 @@ void free_netdev(struct net_device *dev)
> >  }
> >  EXPORT_SYMBOL(free_netdev);
> >  
> > +/**
> > + * 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?

We need to have this private size for cases where we need to get the
pointer to their private structure given the dummy device.  in the
embedded case you can use container_of(), but, with a pointer to
net_device, that is not the case anymore, and we should use the dummy private
data pointer back to the private data.

For instance, see the example of iwlwifi: 
https://lore.kernel.org/all/20240307174843.1719130-1-leitao@debian.org/

> > +{
> > +	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
> 
> (and then a followup which converts &net_device to proper flex arrays)

That is the goal in fact, but I personally think this will take a while,
given that we need to rely on maintainers to test the changes to be able
to move forward.