netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Breno Leitao <leitao@debian.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Alexander Lobakin <aleksander.lobakin@intel.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	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
Date: Thu, 28 Mar 2024 10:46:37 -0700	[thread overview]
Message-ID: <ZgWs/Z7d1hgFyytO@gmail.com> (raw)
In-Reply-To: <20240328101053.69a968ec@kernel.org>

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.

  reply	other threads:[~2024-03-28 17:46 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-27 20:08 [PATCH net-next] net: create a dummy net_device allocator Breno Leitao
2024-03-27 23:17 ` Jakub Kicinski
2024-03-28 14:57   ` Alexander Lobakin
2024-03-28 15:02 ` Alexander Lobakin
2024-03-28 17:10   ` Jakub Kicinski
2024-03-28 17:46     ` Breno Leitao [this message]
2024-03-28 17:40   ` Breno Leitao
2024-04-02 18:01 ` Leon Romanovsky
2024-04-02 18:17   ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZgWs/Z7d1hgFyytO@gmail.com \
    --to=leitao@debian.org \
    --cc=aleksander.lobakin@intel.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=dennis.dalessandro@cornelisnetworks.com \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=kvalo@kernel.org \
    --cc=leon@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lixiaoyan@google.com \
    --cc=lorenzo@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=quic_jjohnson@quicinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).