Subject: Re: [PATCH net-next v2 2/2] pcnet32: switch to napi_alloc_skb() and napi_gro_receive()

From: Oscar Maes <[email-masked]>
Date: Tue, 2 Jun 2026 18:37:56 +0200

On Sun, May 31, 2026 at 10:09:44PM +0200, Andrew Lunn wrote:
> On Sun, May 31, 2026 at 06:51:40PM +0200, Oscar Maes wrote:
> > On Thu, May 28, 2026 at 12:12:10PM -0700, Jakub Kicinski wrote:
> > > On Thu, 28 May 2026 19:05:10 +0200 Oscar Maes wrote:
> > > > On Thu, May 28, 2026 at 08:07:48AM -0700, Jakub Kicinski wrote:
> > > > > On Thu, 28 May 2026 16:28:51 +0200 Oscar Maes wrote:  
> > > > > > On Mon, May 25, 2026 at 08:45:28AM -0700, Jakub Kicinski wrote:  
> > > > > > Intel x86  
> > > > > 
> > > > > LOL. Please. Which _hardware_ platform.
> > > > > Or to ask more directly - is it QEMU.  
> > > > 
> > > > It was indeed tested on QEMU.
> > > > Either way, the performance measurements were just a sanity check.
> > > > Changing the code to use the modern API is just better.
> > > 
> > > Please don't waste our time with optimizations to unused code.
> > 
> > It's not unused code. VMware emulates this device by default in some cases.
> > I've encountered it when installing QubesOS in vmware which is based on
> > relatively modern Linux.
> 
> There is a saying: If it is not broken, don't fix it.
> 
> "Changing the code to use the modern API is just better" if not a good
> enough reason on its own. Using older APIs does not mean it is
> broken. It does not need fixing. Some percentage of changes like this
> break stuff.  We Reviewers/Maintainers need to look closely at such
> changes and decide if it does actually break stuff. And we want to
> know what the return on investment is? I would much rather spend my
> time reviewing new drivers for hardware which currently does not have
> a driver.
> 
> So, if you say something like: I'm using this driver daily for X. I've
> spent some time optimising it, and with this change, my use case gets
> a 10% increase in performance. Here are my numbers....
> 
> That gives us a better feeling that you have extensively tested this,
> so it is less likely you have broken something. You can also show
> there is a real improvement. So the change is actually worth while.
> 
> 	Andrew
> 
> 
> 
> 
> 

Fair enough, I appreciate the input.
This change isn't a big deal to me, so I guess we can drop it.
I'll make sure to keep your advice in mind for future patches.