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

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

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