[PATCH net-next 0/2] devlink: shared devlink improvements

[PATCH net-next 0/2] devlink: shared devlink improvements

[Przemek Kitszel]

First patch unifies access to priv data set by driver
(changelog in the patch).

Second patch improves support of devlink resources registered for shared
instance (new in v2 of the series).

Przemek Kitszel (2):
  devlink: unify devlink_shd_get_priv() into devlink_priv()
  devlink: unregister shared devlink resources on destroy

 net/devlink/devl_internal.h |  7 +++++++
 net/devlink/core.c          | 10 +++++++++-
 net/devlink/sh_dev.c        |  9 +++++----
 3 files changed, 21 insertions(+), 5 deletions(-)

-- 
2.51.1

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

[Przemek Kitszel]

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 a/net/devlink/sh_dev.c b/net/devlink/sh_dev.c
index 85acce97e788..b85e5cb1edbe 100644
--- a/net/devlink/sh_dev.c
+++ b/net/devlink/sh_dev.c
@@ -43,13 +43,14 @@ static struct devlink_shd *devlink_shd_create(const char *id,
 				  &init_net, NULL, driver);
 	if (!devlink)
 		return NULL;
-	shd = devlink_priv(devlink);
+	shd = __devlink_priv(devlink);
 
 	shd->id = kstrdup(id, GFP_KERNEL);
 	if (!shd->id)
 		goto err_devlink_free;
 	shd->priv_size = priv_size;
 	refcount_set(&shd->refcount, 1);
+	devlink->is_shd = 1;
 
 	devl_lock(devlink);
 	devl_register(devlink);
@@ -136,7 +137,7 @@ void devlink_shd_put(struct devlink *devlink)
 	struct devlink_shd *shd;
 
 	mutex_lock(&shd_mutex);
-	shd = devlink_priv(devlink);
+	shd = __devlink_priv(devlink);
 	if (refcount_dec_and_test(&shd->refcount))
 		devlink_shd_destroy(shd);
 	mutex_unlock(&shd_mutex);
@@ -154,8 +155,7 @@ EXPORT_SYMBOL_GPL(devlink_shd_put);
  */
 void *devlink_shd_get_priv(struct devlink *devlink)
 {
-	struct devlink_shd *shd = devlink_priv(devlink);
+	struct devlink_shd *shd = __devlink_priv(devlink);
 
 	return shd->priv;
 }
-EXPORT_SYMBOL_GPL(devlink_shd_get_priv);
-- 
2.51.1

[PATCH net-next 2/2] devlink: unregister shared devlink resources on destroy

[Przemek Kitszel]

Since shared devlink acts as a normal devlink instance, capable of all
usual devlink operations, it must unregister its resources.

I plan to make use of devlink resources on a shared instance for ice
driver by separate series, coming soon.

Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
 net/devlink/sh_dev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/devlink/sh_dev.c b/net/devlink/sh_dev.c
index b85e5cb1edbe..5de138bf3630 100644
--- a/net/devlink/sh_dev.c
+++ b/net/devlink/sh_dev.c
@@ -71,6 +71,7 @@ static void devlink_shd_destroy(struct devlink_shd *shd)
 
 	list_del(&shd->list);
 	devl_lock(devlink);
+	devl_resources_unregister(devlink);
 	devl_unregister(devlink);
 	devl_unlock(devlink);
 	kfree(shd->id);
-- 
2.51.1

RE: [PATCH net-next 2/2] devlink: unregister shared devlink resources on destroy

[Loktionov, Aleksandr]

> -----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 2/2] devlink: unregister shared devlink
> resources on destroy
> 
> Since shared devlink acts as a normal devlink instance, capable of all
> usual devlink operations, it must unregister its resources.
> 
> I plan to make use of devlink resources on a shared instance for ice
> driver by separate series, coming soon.
> 
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
>  net/devlink/sh_dev.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/devlink/sh_dev.c b/net/devlink/sh_dev.c index
> b85e5cb1edbe..5de138bf3630 100644
> --- a/net/devlink/sh_dev.c
> +++ b/net/devlink/sh_dev.c
> @@ -71,6 +71,7 @@ static void devlink_shd_destroy(struct devlink_shd
> *shd)
> 
>  	list_del(&shd->list);
>  	devl_lock(devlink);
> +	devl_resources_unregister(devlink);
>  	devl_unregister(devlink);
>  	devl_unlock(devlink);
>  	kfree(shd->id);
> --
> 2.51.1

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

RE: [PATCH net-next 1/2] devlink: unify devlink_shd_get_priv() into devlink_priv()

[Loktionov, Aleksandr]

> -----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.


...


> 
>  	return shd->priv;
>  }
> -EXPORT_SYMBOL_GPL(devlink_shd_get_priv);
> --
> 2.51.1

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

[Jacob Keller]

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.

Re: [PATCH net-next 2/2] devlink: unregister shared devlink resources on destroy

[Jiri Pirko]

Wed, Mar 25, 2026 at 07:26:53AM +0100, przemyslaw.kitszel@intel.com wrote:
>Since shared devlink acts as a normal devlink instance, capable of all
>usual devlink operations, it must unregister its resources.
>
>I plan to make use of devlink resources on a shared instance for ice
>driver by separate series, coming soon.
>
>Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>---
> net/devlink/sh_dev.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/net/devlink/sh_dev.c b/net/devlink/sh_dev.c
>index b85e5cb1edbe..5de138bf3630 100644
>--- a/net/devlink/sh_dev.c
>+++ b/net/devlink/sh_dev.c
>@@ -71,6 +71,7 @@ static void devlink_shd_destroy(struct devlink_shd *shd)
> 
> 	list_del(&shd->list);
> 	devl_lock(devlink);
>+	devl_resources_unregister(devlink);

Hmm. It is driver's responsibility to call this on appropriate place,
symmetric to resource register. Why to have this in code for sh?

The idea I had was to have callbacks to driver to do things like this
and more eventually. Would it work for you?


> 	devl_unregister(devlink);
> 	devl_unlock(devlink);
> 	kfree(shd->id);
>-- 
>2.51.1
>

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

[Jiri Pirko]

Wed, Mar 25, 2026 at 07:26:52AM +0100, przemyslaw.kitszel@intel.com wrote:
>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>

Reviewed-by: Jiri Pirko <jiri@nvidia.com>

Re: [PATCH net-next 2/2] devlink: unregister shared devlink resources on destroy

[Przemek Kitszel]

On 3/26/26 06:20, Jiri Pirko wrote:
> Wed, Mar 25, 2026 at 07:26:53AM +0100, przemyslaw.kitszel@intel.com wrote:
>> Since shared devlink acts as a normal devlink instance, capable of all
>> usual devlink operations, it must unregister its resources.
>>
>> I plan to make use of devlink resources on a shared instance for ice
>> driver by separate series, coming soon.
>>
>> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>> ---
>> net/devlink/sh_dev.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/net/devlink/sh_dev.c b/net/devlink/sh_dev.c
>> index b85e5cb1edbe..5de138bf3630 100644
>> --- a/net/devlink/sh_dev.c
>> +++ b/net/devlink/sh_dev.c
>> @@ -71,6 +71,7 @@ static void devlink_shd_destroy(struct devlink_shd *shd)
>>
>> 	list_del(&shd->list);
>> 	devl_lock(devlink);
>> +	devl_resources_unregister(devlink);
> 
> Hmm. It is driver's responsibility to call this on appropriate place,
> symmetric to resource register. Why to have this in code for sh?
> 
> The idea I had was to have callbacks to driver to do things like this
> and more eventually. Would it work for you?

for driver stuff that could be useful (say, someone wants to have
an additional data structure to iterate and free), OTOH, each
entity that is sharing should "unplug" from shared devlink, so
those structures should be emptied anyway
for my stuff in ice I managed to do without destructor

coming back to devlink stuff - all things (resources, health)
would need same cleaning in all drives, so putting that in
devlink_shd_destroy() will simply take the burden off devs
(at the expense of less symmetric code for normal/shared
devlinks)

in short: I'm ok with any of
a. callback for all cleanup
b. callback for driver-specific cleanup
c. no callback and no driver-specific cleanup (until someone needs
    that very much)

thanks!

> 
> 
>> 	devl_unregister(devlink);
>> 	devl_unlock(devlink);
>> 	kfree(shd->id);
>> -- 
>> 2.51.1
>>

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

[Przemek Kitszel]

>>>
>>>   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.

We could achieve that by adding a marker prior to priv data, in the same
layout for both structs. I have code handy, will post v3 later to don't
spam too much (and hopefully resolve discussion on the other patch)

@Jiri, I will not add your RB, as this would be significant change

thanks a lot for reviewing so far to all of you

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

[Jakub Kicinski]

On Wed, 25 Mar 2026 07:26:52 +0100 Przemek Kitszel wrote:
> 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.

The implicit conversion may make things hard to reason about.
Are you sure you actually mean that it's "easy to forget" or
it's easier for OOT transition?

If we are worried about misuse we should instead add an accessor
for "individual" (better name welcome) instance and WARN_ON()
when devlink_priv() is used in the shared setup.

Or add a third argument to devlink_priv() which will pass the size 
of the LHS ptr, and warn on attempts to access priv of the wrong
size?

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

[Przemek Kitszel]

On 3/26/26 22:38, Jakub Kicinski wrote:
> On Wed, 25 Mar 2026 07:26:52 +0100 Przemek Kitszel wrote:
>> 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.
> 
> The implicit conversion may make things hard to reason about.

hmm...
now we have simple devlink code with a possibility of hard to detect bug
in drivers, plus hypothetical drivers code that we will need to reason
about. I would replace that for:
"complex devlink code", which will be harder to review, but then without
bugs and with hypothetical drivers code that would not require reasoning
at all (one API call == no decision, and no checking for correctness
later, when someone will be hunting an unrelated bug)

Anyway, I have used "hypothetical drivers code" twice, so maybe it will
be best to forgo this patch. I will just call correct functions in ice,
and we will wait for other users with priv data on shd devlink to see
if they struggle.

The priv_to_devlink issue that Alex pointed out will remain unresolved,
although it's easy to just keep the devlink pointer in priv (as I will
do in ice).

Thank you.

> Are you sure you actually mean that it's "easy to forget" or
> it's easier for OOT transition?

I have literally forgot to change (written from scratch for upstream)
code, in the part that was rebased conflict-free. I guess, I was the
only person that were in need of devlink-shared, with code ready to
rebase over Jiri's work. And I'm done, so there is not much relevance
now for this point.

> 
> If we are worried about misuse we should instead add an accessor
> for "individual" (better name welcome) instance and WARN_ON()
> when devlink_priv() is used in the shared setup.

that would require the same amount of code as this patch (curr ver)
has, only with WARN_ON() instead proper value (IOW: we detect what
developer wanted, and give them big warning instead)
I'm not interested :)

> 
> Or add a third argument to devlink_priv() which will pass the size
> of the LHS ptr, and warn on attempts to access priv of the wrong
> size?

this could incidentally match sizeof(devlink_shd) :P

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

[Jakub Kicinski]

On Fri, 27 Mar 2026 08:42:47 +0100 Przemek Kitszel wrote:
> > If we are worried about misuse we should instead add an accessor
> > for "individual" (better name welcome) instance and WARN_ON()
> > when devlink_priv() is used in the shared setup.  
> 
> that would require the same amount of code as this patch (curr ver)
> has, only with WARN_ON() instead proper value (IOW: we detect what
> developer wanted, and give them big warning instead)

I guess I don't understand why you're so certain that you know what
the developer wanted. I'm of the opinion that the "individual" devlink
instances should not exist at all. Y'all want them, and claim to have
use cases for them. And yet, it is somehow not valid to get their priv.