On Wed, Apr 10, 2024 at 02:10:04PM +0300, Ido Schimmel wrote: > On Tue, Apr 09, 2024 at 05:57:16AM -0700, Breno Leitao wrote: > > It is impossible to use init_dummy_netdev together with alloc_netdev() > > as the 'setup' argument. > > > > This is because alloc_netdev() initializes some fields in the net_device > > structure, and later init_dummy_netdev() memzero them all. This causes > > some problems as reported here: > > > > https://lore.kernel.org/all/20240322082336.49f110cc@xxxxxxxxxx/ > > > > Split the init_dummy_netdev() function in two. Create a new function called > > init_dummy_netdev_core() that does not memzero the net_device structure. > > Then have init_dummy_netdev() memzero-ing and calling > > init_dummy_netdev_core(), keeping the old behaviour. > > > > init_dummy_netdev_core() is the new function that could be called as an > > argument for alloc_netdev(). > > > > Also, create a helper to allocate and initialize dummy net devices, > > leveraging init_dummy_netdev_core() as the setup argument. This function > > basically simplify the allocation of dummy devices, by allocating and > > initializing it. Freeing the device continue to be done through > > free_netdev() > > > > Suggested-by: Jakub Kicinski <kuba@xxxxxxxxxx> > > Signed-off-by: Breno Leitao <leitao@xxxxxxxxxx> > > Reviewed-by: Ido Schimmel <idosch@xxxxxxxxxx> > > We were about to submit another user of init_dummy_netdev() when I > noticed this patch. Converted the code to use alloc_netdev_dummy() [1] > and it seems to be working fine. Will submit after your patch is > accepted. Thanks. It seems that this patch is close to get accepted. Let's see... > See a few minor comments below. > > [...] > > > +/** > > + * init_dummy_netdev - init a dummy network device for NAPI > > + * @dev: device to init > > + * > > + * This takes a network device structure and initialize the minimum > > s/initialize/initializes/ > > > + * amount of fields so it can be used to schedule NAPI polls without > > + * registering a full blown interface. This is to be used by drivers > > + * that need to tie several hardware interfaces to a single NAPI > > + * poll scheduler due to HW limitations. > > + */ > > +void init_dummy_netdev(struct net_device *dev) > > +{ > > + /* Clear everything. Note we don't initialize spinlocks > > + * are they aren't supposed to be taken by any of the > > I assume you meant s/are/as/ ? Thanks for the feedback, I agree with all of them. Since these lines were not introduced by this patch, and this patch is just moving code (and comments) around, I would add a new patch to the patch series fixing the grammar errors.
