Re: [PATCH net-next 1/2] devlink, mlx5: add init/fini ops for shared devlink

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

On 4/28/26 13:10, Jiri Pirko wrote:
> Tue, Apr 28, 2026 at 11:09:11AM +0200, przemyslaw.kitszel@intel.com wrote:
>> Add .shd_init() and .shd_fini() ops, that will be called for the first
>> devlink_shd_get() (to initialize driver' priv data) and on the last
>> devlink_shd_put() (to allow for the cleanup). Both ops are optional.
>>
>> .shd_init() could return an error, which will stop creation of shd
>> instance. The initializer also gets an additional, optional param,
>> that driver could use for any needs.
>>
>> If any of the callbacks will need to get devlink instance, it could
>> be accessed by shd_priv_to_devlink().
>>
>> Both callbacks are called with devl_lock held and devlink registered.
>>
>> Next commit will make use of the callbacks, another one will make use also
>> of the non-null additional param (outside of this series).
>>
>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>> ---
>> first discussed at:
>> https://lore.kernel.org/netdev/20260325063143.261806-3-przemyslaw.kitszel@intel.com
>>
>> Sashiko suggested to convert devlink_shd_create() to return ERR_PTR(),
>> and propagate that up to the driver. It think it will just make code more
>> verbose for not much benefit. And drivers could just store err if they
>> want in the passed @init_param.
>>
>> ---
>> include/net/devlink.h                         | 26 +++++++++++++
>> .../ethernet/mellanox/mlx5/core/sh_devlink.c  |  2 +-
>> net/devlink/sh_dev.c                          | 39 ++++++++++++++++++-
>> 3 files changed, 64 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>> index bcd31de1f890..5d3a1337bfa1 100644
>> --- a/include/net/devlink.h
>> +++ b/include/net/devlink.h
>> @@ -1586,6 +1586,30 @@ struct devlink_ops {
>> 				    struct devlink_rate *parent,
>> 				    void *priv_child, void *priv_parent,
>> 				    struct netlink_ext_ack *extack);
>> +
>> +	/**
>> +	 * shd_init: Shared devlink instance initializer
>> +	 * @priv: shd_devlink' priv
>> +	 * @init_param: additional param to pass to driver callback
>> +	 *
>> +	 * Called once when the shared instance is first created (by the first
>> +	 * devlink_shd_get() call).
>> +	 * Should initialize the driver's private data embedded in the shared
>> +	 * devlink. May be NULL.
>> +	 *
>> +	 * Return: 0 on success, negative to prevent shared instance usage.
>> +	 */
>> +	int (*shd_init)(void *priv, void *init_param);
> 
> 1. "param" has specific meaning in devlink context
> 2. You don't use the arg in driver
> 
> Care to drop it?

I have a user for it, but it will be a separate series
(I have already 15 patches there), will post RFC to link here
to the user, will that work?

my intention was to not tie touching mlx code with big series for intel

> 
> Otherwise, this looks fine to me. Thanks! (small nitpick below)

ack for the nit