netdev - Re: [Intel-wired-lan] [PATCH iwl-next v1] idpf: refactor some missing field get/prep conversions

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: <1bed60db-3f6b-4b99-84b7-8021787c9508@intel.com>
Date: Wed, 6 Dec 2023 13:33:13 +0100
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Jesse Brandeburg <jesse.brandeburg@...el.com>
CC: <intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>,
	<anthony.l.nguyen@...el.com>, Julia Lawall <julia.lawall@...ia.fr>
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v1] idpf: refactor some missing
 field get/prep conversions

From: Jesse Brandeburg <jesse.brandeburg@...el.com>
Date: Tue, 5 Dec 2023 17:10:58 -0800

> On 12/4/2023 2:26 AM, Alexander Lobakin wrote:
> yping.
>>
>> What is "not a constant"?
>>
>> 	ring[nta] = FIELD_PREP(IDPF_RX_BI_GEN_M,
>> 			       test_bit(__IDPF_Q_GEN_CHK, flags));
>>
>> Is there a problem with this ^ code?
>>
>> "The scripts ignored that" is not a good argument I'd say :>
> 
> Fixed in v2
> 
> 
>>> Generally I'd prefer that the kind of check above returned a bool with a
>>> constant conversion of the mask (compile time) to an LE16 mask, and then
>>> use that, which is why all of our other drivers do that instead.
>>
>> Ah, good point. Smth like
>>
>> 		gen = !!(tx_desc->qid_comptype_gen &
>> 			 IDPF_TXQ_COMPLQ_GEN_M_LE);
> 
> Yeah, it would be nice but I didn't add that to this series. But you
> have the idea.
> 
>>
>> OTOH x86 is always LE and BE is seen more and more rarely nowadays. It
>> might just not worth having a LE-version of each such mask for the sake
>> of a bit more optimized code on architectures where our drivers are
>> barely used.
> 
> Our drivers should work on BE, IMO. sparse annotations takes care of
> making sure we have the conversions right, they cost nothing on x86.

They do work, what I meant is that adding _LE constant masks would make
the code a bit more optimized on BE only, then would it make sense to
add +1 line per each such mask to get some almost invisible
optimizations on non-common architectures?

Thanks,
Olek