netdev - Re: [RFT net-next 1/6] devlink: expose instance locking and add locked port registering

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]

Date:   Fri, 11 Mar 2022 18:09:36 +0200
From:   Leon Romanovsky <leonro@...dia.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     <idosch@...dia.com>, <petrm@...dia.com>,
        <simon.horman@...igine.com>, <netdev@...r.kernel.org>,
        <jiri@...nulli.us>
Subject: Re: [RFT net-next 1/6] devlink: expose instance locking and add
 locked port registering

On Wed, Mar 09, 2022 at 04:16:27PM -0800, Jakub Kicinski wrote:
> It should be familiar and beneficial to expose devlink instance
> lock to the drivers. This way drivers can block devlink from
> calling them during critical sections without breakneck locking.
> 
> Add port helpers, port splitting callbacks will be the first
> target.
> 
> Use 'devl_' prefix for "explicitly locked" API. Initial RFC used
> '__devlink' but that's too much typing.
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
>  include/net/devlink.h | 11 +++++
>  net/core/devlink.c    | 99 ++++++++++++++++++++++++++++++++-----------
>  2 files changed, 86 insertions(+), 24 deletions(-)
> 
> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index 8d5349d2fb68..9de0d091aee9 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
> @@ -1479,6 +1479,17 @@ void *devlink_priv(struct devlink *devlink);
>  struct devlink *priv_to_devlink(void *priv);
>  struct device *devlink_to_dev(const struct devlink *devlink);
>  
> +/* Devlink instance explicit locking */
> +void devl_lock(struct devlink *devlink);
> +void devl_unlock(struct devlink *devlink);
> +void devl_assert_locked(struct devlink *devlink);
> +bool devl_lock_is_held(struct devlink *devlink);
> +
> +int devl_port_register(struct devlink *devlink,
> +		       struct devlink_port *devlink_port,
> +		       unsigned int port_index);
> +void devl_port_unregister(struct devlink_port *devlink_port);
> +
>  struct ib_device;
>  
>  struct net *devlink_net(const struct devlink *devlink);
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index fcd9f6d85cf1..c30da1fc023d 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -225,6 +225,37 @@ struct devlink *__must_check devlink_try_get(struct devlink *devlink)
>  	return NULL;
>  }
>  
> +void devl_assert_locked(struct devlink *devlink)
> +{
> +	lockdep_assert_held(&devlink->lock);
> +}
> +EXPORT_SYMBOL_GPL(devl_assert_locked);
> +
> +bool devl_lock_is_held(struct devlink *devlink)
> +{
> +	/* We have to check this at runtime because struct devlink
> +	 * is now private. Normally lock_is_held() should be eliminated
> +	 * as dead code in the caller and we can depend on the linker error.
> +	 */
> +	if (!IS_ENABLED(CONFIG_LOCKDEP))
> +		return WARN_ON_ONCE(true);
> +
> +	return lockdep_is_held(&devlink->lock);
> +}
> +EXPORT_SYMBOL_GPL(devl_lock_is_held);

What about this?

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 8d5349d2fb68..33b47d1a6800 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1762,5 +1762,12 @@ devlink_compat_switch_id_get(struct net_device *dev,
 }
 
 #endif
-
+#if IS_ENABLED(CONFIG_LOCKDEP)
+bool devl_lock_is_held(struct devlink *devlink);
+#else
+static inline bool devl_lock_is_held(struct devlink *devlink)
+{
+       return true;
+}
+#endif
 #endif /* _NET_DEVLINK_H_ */