Received-SPF: pass (google.com: domain of linux-kernel+bounces-111665-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Fri, 22 Mar 2024 07:58:02 -0700
From: Breno Leitao <leitao@debian.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Jeff Johnson <quic_jjohnson@quicinc.com>, Kalle Valo <kvalo@kernel.org>,
	Jeff Johnson <jjohnson@kernel.org>, keescook@chromium.org,
	"open list:NETWORKING DRIVERS (WIRELESS)" <linux-wireless@vger.kernel.org>,
	"open list:QUALCOMM ATHEROS ATH10K WIRELESS DRIVER" <ath10k@lists.infradead.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ath10k: allocate dummy net_device dynamically
Message-ID: <Zf2ceu2O47lLbKU3@gmail.com>
References: <20240319104754.2535294-1-leitao@debian.org>
 <9fcdb857-da62-4832-ae11-043fe993e4ad@quicinc.com>
 <20240321072821.59f56757@kernel.org>
 <5039256c-03eb-4cda-8d11-49e4561cf1ef@quicinc.com>
 <20240321151744.246ce2d0@kernel.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240321151744.246ce2d0@kernel.org>

Hello Jakub,

On Thu, Mar 21, 2024 at 03:17:44PM -0700, Jakub Kicinski wrote:
> On Thu, 21 Mar 2024 15:02:39 -0700 Jeff Johnson wrote:
> > >> As suggested there we should just use kmalloc/kfree to match the existing logic.  
> > > 
> > > Please no. There is no magic here. alloc + free must match whether
> > > you're using magic object alloc wrapper (alloc_netdev()) or straight
> > > up kzalloc().  
> > 
> > Based upon the ath11k patch there must be something going on with
> > alloc_netdev()/free_netdev() that doesn't occur when these aren't used.
> 
> Looks like init_dummy_netdev wipes the netdev structure clean, so I
> don't think we can use it directly as the setup function, Breno :(

Before my patch,  init_dummy_netdev was being also used. The patch was
basically replacing the init_dummy_netdev by alloc_netdev() with will
call "setup(dev);" later. 

-               init_dummy_netdev(&irq_grp->napi_ndev);
+               irq_grp->napi_ndev = alloc_netdev(0, "dummy", NET_NAME_UNKNOWN,
+                                                 init_dummy_netdev);

I am wondering if alloc_netdev() is messing with something instead of
init_dummy_netdev().

Also, Kalle's crash is during rmmod, and not during initialization.
getting NULL after free_netdev() is called.

> Maybe we should add a new helper to "alloc dummy netdev" which can
> call alloc_netdev() with right arguments and do necessary init?

What are the right arguments in this case?

Thanks!