Re: [Intel-wired-lan] [PATCH net-next 1/2] devlink: unify devlink_shd_get_priv() into devlink_priv()

[Jacob Keller <jacob.e.keller@intel.com>]

On 3/25/2026 12:46 AM, Loktionov, Aleksandr wrote:
> 
> 
>> -----Original Message-----
>> From: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>
>> Sent: Wednesday, March 25, 2026 7:27 AM
>> To: Jiri Pirko <jiri@resnulli.us>; netdev@vger.kernel.org; Jakub
>> Kicinski <kuba@kernel.org>
>> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; intel-wired-
>> lan@lists.osuosl.org; Loktionov, Aleksandr
>> <aleksandr.loktionov@intel.com>; edumazet@google.com;
>> horms@kernel.org; pabeni@redhat.com; davem@davemloft.net; Schmidt,
>> Michal <mschmidt@redhat.com>; Kitszel, Przemyslaw
>> <przemyslaw.kitszel@intel.com>
>> Subject: [PATCH net-next 1/2] devlink: unify devlink_shd_get_priv()
>> into devlink_priv()
>>
>> Unify access API to shared devlink priv data with normal devlink.
>>
>> Thanks to Jiri Pirko, we now have ability to create shared devlink
>> instances [1]. Introduction series have added usage of those for mlx,
>> but without priv data attached to the shared devlink.
>>
>> Current API makes it possible to access shared devlink instance's priv
>> data:
>>
>> 	void *devlink_shd_get_priv(struct devlink *devlink);
>>
>> but it is easy to forget (especially during rebase from "before shared
>> devlinks" era) and call:
>>
>> 	void *devlink_priv(struct devlink *devlink);
>>
>> which even has the same signature, so it's hard to catch the error.
>>
>> New proposed API unifies both calls into one, without any increase in
>> the observed struct size. (Alternative could be to store additional
>> pointer, set during devlink_alloc).
>>
>> Unexport the less convenient API call.
>>
>> [1] commit 411ad0605875 ("Merge branch 'devlink-introduce-shared-
>> devlink-instance-for-pfs-on-same-chip'")
>> [1] https://lore.kernel.org/all/20260312100407.551173-1-
>> jiri@resnulli.us
>>
>> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>> ---
>> v1:
>> https://lore.kernel.org/netdev/20260323132136.13191-1-
>> przemyslaw.kitszel@intel.com
>>
>> v2:
>> - fix typos (Alex, Jiri)
>> - fix infinite recurrence (Alex)
>> - add __devlink_priv(), which is more general than v1's
>> devlink_to_shd()
>>   (Jiri)
>> ---
>>  net/devlink/devl_internal.h |  7 +++++++
>>  net/devlink/core.c          | 10 +++++++++-
>>  net/devlink/sh_dev.c        |  8 ++++----
>>  3 files changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
>> index 7dfb7cdd2d23..0a57318d92f8 100644
>> --- a/net/devlink/devl_internal.h
>> +++ b/net/devlink/devl_internal.h
>> @@ -58,6 +58,7 @@ struct devlink {
>>  	struct mutex lock;
>>  	struct lock_class_key lock_key;
>>  	u8 reload_failed:1;
>> +	u8 is_shd:1;
>>  	refcount_t refcount;
>>  	struct rcu_work rwork;
>>  	struct devlink_rel *rel;
>> @@ -72,6 +73,12 @@ struct devlink *__devlink_alloc(const struct
>> devlink_ops *ops, size_t priv_size,
>>  				struct net *net, struct device *dev,
>>  				const struct device_driver *dev_driver);
>>
>> +/* Get priv allocated for struct devlink */ void
>> *__devlink_priv(struct
>> +devlink *devlink);
>> +
>> +/* Get private data from shared devlink instance */ void
>> +*devlink_shd_get_priv(struct devlink *devlink);
>> +
>>  #define devl_warn(devlink, format, args...)				\
>>  	do {								\
>>  		if ((devlink)->dev)					\
>> diff --git a/net/devlink/core.c b/net/devlink/core.c index
>> eeb6a71f5f56..a242be203fe8 100644
>> --- a/net/devlink/core.c
>> +++ b/net/devlink/core.c
>> @@ -230,10 +230,18 @@ int devlink_rel_devlink_handle_put(struct
>> sk_buff *msg, struct devlink *devlink,
>>  	return err;
>>  }
>>
>> -void *devlink_priv(struct devlink *devlink)
>> +void *__devlink_priv(struct devlink *devlink)
>>  {
>>  	return &devlink->priv;
>>  }
>> +
>> +void *devlink_priv(struct devlink *devlink) {
>> +	if (devlink->is_shd)
>> +		return devlink_shd_get_priv(devlink);
>> +
>> +	return __devlink_priv(devlink);
>> +}
>>  EXPORT_SYMBOL_GPL(devlink_priv);
>>
>>  struct devlink *priv_to_devlink(void *priv) diff --git
> I'm worried about priv_to_devlink(), if someone passes the result of devlink_priv(shared_dl) as priv,
> container_of computes garbage - because the pointer came from shd->priv, NOT from &devlink->priv.
> 

There's no good way to detect that inside the priv_to_devlink either,
since it can't know which private pointer it is looking at. Hmm.