netdev - Re: [PATCH 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 for Android: free password hash cracker in your pocket

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

Message-ID: <YjGf3OqijAiqSNE/@unreal>
Date:   Wed, 16 Mar 2022 10:29:16 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, jiri@...dia.com,
        idosch@...dia.com, petrm@...dia.com, simon.horman@...igine.com,
        louis.peens@...igine.com
Subject: Re: [PATCH net-next 1/6] devlink: expose instance locking and add
 locked port registering

On Mon, Mar 14, 2022 at 11:00:04PM -0700, 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.
> 
> devl_lock_is_held() is not defined without lockdep, which is
> the same behavior as lockdep_is_held() itself.
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
> v1: - add a small section to the docs
>     - redo the lockdep ifdef
> ---
>  Documentation/networking/devlink/index.rst | 16 ++++
>  include/net/devlink.h                      | 11 +++
>  net/core/devlink.c                         | 95 ++++++++++++++++------
>  3 files changed, 98 insertions(+), 24 deletions(-)

<...>

> +void devl_assert_locked(struct devlink *devlink)
> +{
> +	lockdep_assert_held(&devlink->lock);
> +}
> +EXPORT_SYMBOL_GPL(devl_assert_locked);
> +
> +#ifdef CONFIG_LOCKDEP
> +/* For use in conjunction with LOCKDEP only e.g. rcu_dereference_protected() */
> +bool devl_lock_is_held(struct devlink *devlink)
> +{
> +	return lockdep_is_held(&devlink->lock);
> +}
> +EXPORT_SYMBOL_GPL(devl_lock_is_held);
> +#endif

Sorry that I'm asking you again same question.
How will this devl_lock_is_held() be used in drivers?

Right now, if I decide to use this function in mlx5 (or in any other driver),
the code will be something like this:

void func(...)
{
   ....
   if (IS_ENABLED(CONFIG_LOCKDEP))
   	if (rcu_dereference_protected(a, devl_lock_is_held(devlink) == b) {
		....
}

The line "if (IS_ENABLED(CONFIG_LOCKDEP))" needs to be in every driver
or it won't compile in release mode.

Thanks