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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1aa72500-79cd-810e-947c-172bbc4db513@intel.com>
Date:   Mon, 19 Dec 2022 14:14:18 -0800
From:   Jacob Keller <jacob.e.keller@...el.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     <jiri@...nulli.us>, <leon@...nel.org>, <netdev@...r.kernel.org>
Subject: Re: [RFC net-next 05/10] devlink: remove the registration guarantee
 of references



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.