netdev - Re: [RFC/RFT PATCH net-next v3 1/2] net: Add ndo_write_rx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251030192018.28dcd830@kernel.org>
Date: Thu, 30 Oct 2025 19:20:18 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: I Viswanath <viswanathiyyappan@...il.com>
Cc: davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com,
 horms@...nel.org, sdf@...ichev.me, kuniyu@...gle.com, ahmed.zaki@...el.com,
 aleksander.lobakin@...el.com, jacob.e.keller@...el.com,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 skhan@...uxfoundation.org, linux-kernel-mentees@...ts.linux.dev,
 david.hunter.linux@...il.com, khalid@...nel.org
Subject: Re: [RFC/RFT PATCH net-next v3 1/2] net: Add ndo_write_rx_config
 and helper structs and functions:

On Tue, 28 Oct 2025 23:12:21 +0530 I Viswanath wrote:
> @@ -1421,6 +1426,7 @@ struct net_device_ops {
>  	void			(*ndo_change_rx_flags)(struct net_device *dev,
>  						       int flags);
>  	void			(*ndo_set_rx_mode)(struct net_device *dev);
> +	void			(*ndo_write_rx_config)(struct net_device *dev);
>  	int			(*ndo_set_mac_address)(struct net_device *dev,
>  						       void *addr);
>  	int			(*ndo_validate_addr)(struct net_device *dev);
> @@ -1767,6 +1773,12 @@ enum netdev_reg_state {
>  	NETREG_DUMMY,		/* dummy device for NAPI poll */
>  };
>  
> +struct rx_config_work {

pls make sure to prefix names of types and functions with netif,
netdev or net

> +	struct work_struct config_write;
> +	struct net_device *dev;
> +	spinlock_t config_lock;
> +};
> +

> +#define update_snapshot(config_ptr, type)						\
> +	do {										\
> +		typeof((config_ptr)) rx_config = ((type *)(dev->priv))->rx_config;	\
> +		unsigned long flags;							\
> +		spin_lock_irqsave(&((dev)->rx_work->config_lock), flags);		\
> +		*rx_config = *(config_ptr);						\
> +		spin_unlock_irqrestore(&((dev)->rx_work->config_lock), flags);		\
> +	} while (0)

The driver you picked is relatively trivial, advanced drivers need
to sync longer lists of mcast / ucast addresses. Bulk of the complexity
is in keeping those lists. Simple 

	*rx_config = *(config_ptr);

assignment is not enough. The driver needs to know old and new entries
and send ADD/DEL commands to FW. Converting virtio_net would be better,
but it does one huge dump which is also not representative of most
advanced NICs.

> @@ -11961,9 +11989,17 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
>  	refcount_set(&dev->dev_refcnt, 1);
>  #endif
>  
> -	if (dev_addr_init(dev))
> +	dev->rx_work = kmalloc(sizeof(*dev->rx_work), GFP_KERNEL);

Let's only allocate any extra state if driver has the NDO

> +	if (!dev->rx_work)
>  		goto free_pcpu;
>  
> +	dev->rx_work->dev = dev;
> +	spin_lock_init(&dev->rx_work->config_lock);
> +	INIT_WORK(&dev->rx_work->config_write, execute_write_rx_config);
> +
> +	if (dev_addr_init(dev))
> +		goto free_rx_work;
> +
>  	dev_mc_init(dev);
>  	dev_uc_init(dev);
>  
> @@ -12045,6 +12081,10 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
>  	free_netdev(dev);
>  	return NULL;
>  
> +free_rx_work:
> +	cancel_work_sync(&dev->rx_work->config_write);
> +	kfree(dev->rx_work);
> +
>  free_pcpu:
>  #ifdef CONFIG_PCPU_DEV_REFCNT
>  	free_percpu(dev->pcpu_refcnt);
> @@ -12130,6 +12170,9 @@ void free_netdev(struct net_device *dev)
>  		return;
>  	}
>  
> +	cancel_work_sync(&dev->rx_work->config_write);
> +	kfree(dev->rx_work);

We need to shut down sooner, some time between ndo_stop and ndo_uninit