Re: [RFC net-next 05/10] devlink: remove the registration guarantee of references

Jacob Keller
2022-12-19T22:14:39Z

On 12/19/2022 2:02 PM, Jakub Kicinski wrote:
> On Mon, 19 Dec 2022 09:56:26 -0800 Jacob Keller wrote:
>>> -void devlink_register(struct devlink *devlink)
>>> +int devl_register(struct devlink *devlink)
>>> {
>>> ASSERT_DEVLINK_NOT_REGISTERED(devlink);
>>> - /* Make sure that we are in .probe() routine */
>>> + devl_assert_locked(devlink);
>>>
>>> xa_set_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
>>> devlink_notify_register(devlink);
>>> +
>>> + return 0;
>>
>> Any particular reason to change this to int when it doesn't have a
>> failure case yet? Future patches I assume? You don't check the
>> devl_register return value.
>
> I was wondering if anyone would notice :)
>

I'm fine with it, but I would expect that devlink_register would want to
report it at least?

> Returning errors from the registration helper seems natural,
> and if we don't have this ability it may impact our ability
> to extend the core in the long run.
> I was against making core functions void in the first place.
> It's a good opportunity to change back.

Sure. I think its better to be able to report an error but wanted to
make sure its actually caught or at least logged if it occurs.

We can ofcourse change the function templates again since we don't
really guarantee API stability across versions, but it is more work for
backporting in the future.