netdev - Re: [RFC PATCH v3 net-next 6/7] net: ethtool: add a mutex protecting RSS contexts

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: <ZQCUeTrMpmxhlW9C@shell.armlinux.org.uk>
Date: Tue, 12 Sep 2023 17:40:25 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: edward.cree@....com
Cc: linux-net-drivers@....com, davem@...emloft.net, kuba@...nel.org,
	edumazet@...gle.com, pabeni@...hat.com,
	Edward Cree <ecree.xilinx@...il.com>, netdev@...r.kernel.org,
	habetsm.xilinx@...il.com, sudheer.mogilappagari@...el.com,
	jdamato@...tly.com, andrew@...n.ch, mw@...ihalf.com,
	sgoutham@...vell.com, gakula@...vell.com, sbhatta@...vell.com,
	hkelam@...vell.com, saeedm@...dia.com, leon@...nel.org
Subject: Re: [RFC PATCH v3 net-next 6/7] net: ethtool: add a mutex protecting
 RSS contexts

On Tue, Sep 12, 2023 at 03:21:41PM +0100, edward.cree@....com wrote:
> From: Edward Cree <ecree.xilinx@...il.com>
> 
> While this is not needed to serialise the ethtool entry points (which
>  are all under RTNL), drivers may have cause to asynchronously access
>  dev->ethtool->rss_ctx; taking dev->ethtool->rss_lock allows them to
>  do this safely without needing to take the RTNL.
> 
> Signed-off-by: Edward Cree <ecree.xilinx@...il.com>
> ---
>  include/linux/ethtool.h | 3 +++
>  net/core/dev.c          | 5 +++++
>  net/ethtool/ioctl.c     | 7 +++++++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 8977aa8523e3..1f8293deebd5 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -1026,11 +1026,14 @@ int ethtool_virtdev_set_link_ksettings(struct net_device *dev,
>  /**
>   * struct ethtool_netdev_state - per-netdevice state for ethtool features
>   * @rss_ctx:		IDR storing custom RSS context state
> + * @rss_lock:		Protects entries in @rss_ctx.  May be taken from
> + *			within RTNL.
>   * @rss_ctx_max_id:	maximum (exclusive) supported RSS context ID
>   * @wol_enabled:	Wake-on-LAN is enabled
>   */
>  struct ethtool_netdev_state {
>  	struct idr		rss_ctx;
> +	struct mutex		rss_lock;
>  	u32			rss_ctx_max_id;
>  	u32			wol_enabled:1;
>  };
> diff --git a/net/core/dev.c b/net/core/dev.c
> index f12767466427..2acb4d8cd4c7 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -10054,6 +10054,7 @@ int register_netdevice(struct net_device *dev)
>  	idr_init_base(&dev->ethtool->rss_ctx, 1);
>  
>  	spin_lock_init(&dev->addr_list_lock);
> +	mutex_init(&dev->ethtool->rss_lock);

Is there a reason to split this from the idr (eventually xarray)
initialisation above? Surely initialisations for a feature (rss)
should all be grouped together?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!