Hello Jakub,

On Fri, Mar 22, 2024 at 08:23:36AM -0700, Jakub Kicinski wrote:
> > > 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?
> 
> I'm not sure we have a noop setup() callback today. If you define a
> wrapper to allocate a dummy netdev you can define a new empty function
> next to it and pass that as init? Hope I got the question right.

Thanks for the explanation, it is clear now. I've been working on it,
and this is what I came up with. This is compile-tested by now, and, if
this is what you had in mind, I will do more extensive testing.

  commit db794d99950f68731884a67d911094d94179c522
  Author: Breno Leitao <lei...@debian.org>
  Date:   Wed Mar 27 07:20:03 2024 -0700
  
      net: Create net_device allocator for dummy
      
      Create a helper to allocate and initialize dummy netdevices. This
      function basically simplify the allocation of dummy devices, by
      allocating and initializing it.
      
      Suggested-by: Jakub Kicinski <k...@kernel.org>
      Signed-off-by: Breno Leitao <lei...@debian.org>
  
  diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
  index 643d2b469c49..9d1a5383c23f 100644
  --- a/include/linux/netdevice.h
  +++ b/include/linux/netdevice.h
  @@ -4546,6 +4546,9 @@ static inline void netif_addr_unlock_bh(struct 
net_device *dev)
   
   void ether_setup(struct net_device *dev);
   
  +/* Allocate dummy net_device */
  +struct net_device *alloc_netdev_dummy(int sizeof_priv, const char *name);
  +
   /* Support for loadable net-drivers */
   struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
                                    unsigned char name_assign_type,
  diff --git a/net/core/dev.c b/net/core/dev.c
  index a08d698fe45c..628f35c3cfa2 100644
  --- a/net/core/dev.c
  +++ b/net/core/dev.c
  @@ -10998,6 +10998,13 @@ void free_netdev(struct net_device *dev)
   }
   EXPORT_SYMBOL(free_netdev);
   
  +struct net_device *alloc_netdev_dummy(int sizeof_priv, const char *name)
  +{
  +     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
    *
  
  commit 35500cd6a5db0bbdedbc1067758948769c7ce57e
  Author: Breno Leitao <lei...@debian.org>
  Date:   Wed Mar 27 07:07:40 2024 -0700
  
      net: Split init_dummy_netdev
      
      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 casues
      some problems as reported here:
      
              https://lore.kernel.org/all/20240322082336.49f11...@kernel.org/
      
      Split the function in two. Create a new function called
      init_dummy_netdev_core() that does not memset the net_device structure.
      Then have init_dummy_netdev() memseting and calling
      init_dummy_netdev_core().
      
      init_dummy_netdev_core() will be the function that could be called as
      an argument for alloc_netdev().
      
      Signed-off-by: Breno Leitao <lei...@debian.org>
  
  diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
  index c6f6ac779b34..643d2b469c49 100644
  --- a/include/linux/netdevice.h
  +++ b/include/linux/netdevice.h
  @@ -3119,6 +3119,7 @@ int netdev_refcnt_read(const struct net_device *dev);
   void free_netdev(struct net_device *dev);
   void netdev_freemem(struct net_device *dev);
   void init_dummy_netdev(struct net_device *dev);
  +void init_dummy_netdev_core(struct net_device *dev);
   
   struct net_device *netdev_get_xmit_slave(struct net_device *dev,
                                         struct sk_buff *skb,
  diff --git a/net/core/dev.c b/net/core/dev.c
  index 0766a245816b..a08d698fe45c 100644
  --- a/net/core/dev.c
  +++ b/net/core/dev.c
  @@ -10340,25 +10340,11 @@ int register_netdevice(struct net_device *dev)
   }
   EXPORT_SYMBOL(register_netdevice);
   
  -/**
  - *   init_dummy_netdev       - init a dummy network device for NAPI
  - *   @dev: device to init
  - *
  - *   This takes a network device structure and initialize the minimum
  - *   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.
  +/* Same as init_dummy_netdev, but, basically do not call memset.
  + * This is useful if you are calling this function after alloc_netdev()
    */
  -void init_dummy_netdev(struct net_device *dev)
  +void init_dummy_netdev_core(struct net_device *dev)
   {
  -     /* Clear everything. Note we don't initialize spinlocks
  -      * are they aren't supposed to be taken by any of the
  -      * NAPI code and this dummy netdev is supposed to be
  -      * only ever used for NAPI polls
  -      */
  -     memset(dev, 0, sizeof(struct net_device));
  -
        /* make sure we BUG if trying to hit standard
         * register/unregister code path
         */
  @@ -10379,8 +10365,29 @@ void init_dummy_netdev(struct net_device *dev)
         * its refcount.
         */
   }
  -EXPORT_SYMBOL_GPL(init_dummy_netdev);
  +EXPORT_SYMBOL_GPL(init_dummy_netdev_core);
   
  +/**
  + *   init_dummy_netdev       - init a dummy network device for NAPI
  + *   @dev: device to init
  + *
  + *   This takes a network device structure and initialize the minimum
  + *   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
  +      * NAPI code and this dummy netdev is supposed to be
  +      * only ever used for NAPI polls
  +      */
  +     memset(dev, 0, sizeof(struct net_device));
  +     init_dummy_netdev_core(dev);
  +}
  +EXPORT_SYMBOL_GPL(init_dummy_netdev);
   
   /**
    *   register_netdev - register a network device
  

Reply via email to