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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <07f0e1d1-d526-4b35-8958-0abaf7ef4829@intel.com>
Date: Tue, 11 Feb 2025 15:38:25 +0100
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Jiri Pirko <jiri@...nulli.us>, "Jagielski, Jedrzej"
	<jedrzej.jagielski@...el.com>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"Nguyen, Anthony L" <anthony.l.nguyen@...el.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "horms@...nel.org" <horms@...nel.org>, "Polchlopek,
 Mateusz" <mateusz.polchlopek@...el.com>
Subject: Re: [PATCH iwl-next v2 02/13] ixgbe: add handler for devlink
 .info_get()

On 2/11/25 13:52, Jiri Pirko wrote:
> Tue, Feb 11, 2025 at 01:12:12PM +0100, jedrzej.jagielski@...el.com wrote:
>> From: Jiri Pirko <jiri@...nulli.us>
>> Sent: Monday, February 10, 2025 5:26 PM
>>> Mon, Feb 10, 2025 at 02:56:28PM +0100, jedrzej.jagielski@...el.com wrote:
>>>
>>> [...]
>>>
>>>> +enum ixgbe_devlink_version_type {
>>>> +	IXGBE_DL_VERSION_FIXED,
>>>> +	IXGBE_DL_VERSION_RUNNING,
>>>> +};
>>>> +
>>>> +static int ixgbe_devlink_info_put(struct devlink_info_req *req,
>>>> +				  enum ixgbe_devlink_version_type type,
>>>> +				  const char *key, const char *value)
>>>
>>> I may be missing something, but what's the benefit of having this helper
>>> instead of calling directly devlink_info_version_*_put()?
>>
>> ixgbe devlink .info_get() supports various adapters across ixgbe portfolio which
>> have various sets of version types - some version types are not applicable
>> for some of the adapters - so we want just to check if it's *not empty.*
>>
>> If so then we don't want to create such entry at all so avoid calling
>> devlink_info_version_*_put() in this case.
>> Putting value check prior each calling of devlink_info_version_*_put()
>> would provide quite a code redundancy and would look not so good imho.
>>
>> Me and Przemek are not fully convinced by adding such additional
>> layer of abstraction but we defineltly need this value check to not
>> print empty type or get error and return from the function.
>>
>> Another solution would be to add such check to devlink function.
> 
> That sounds fine to me. Someone else may find this handy as well.
Cool!

perhaps we could also EXPORT devlink_info_version_put(), that would also
help us reduce number of wrappers (also in other intel drivers)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ