netdev - Re: [PATCH iwl-next v2 02/13] ixgbe: add handler for devlink .info

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] [day] [month] [year] [list]

Message-ID: <wftf7fg3gtgmmjufdrtnnlkqzp4x233kmty67hnlchkym4u4ci@ubujjvrt2txa>
Date: Tue, 11 Feb 2025 15:58:24 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: Przemek Kitszel <przemyslaw.kitszel@...el.com>
Cc: "Jagielski, Jedrzej" <jedrzej.jagielski@...el.com>, 
	"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()

Tue, Feb 11, 2025 at 03:38:25PM +0100, przemyslaw.kitszel@...el.com wrote:
>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)

Why not. Make sure you sanitize attr value.