netdev - Re: [PATCH iwl-next] ice: use DSN instead of PCI BDF for ice

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: <4809c248-fde6-4c3c-93ca-743238ed6706@intel.com>
Date: Fri, 7 Mar 2025 15:21:55 -0800
From: Jacob Keller <jacob.e.keller@...el.com>
To: Jiri Pirko <jiri@...nulli.us>
CC: Przemek Kitszel <przemyslaw.kitszel@...el.com>,
	<intel-wired-lan@...ts.osuosl.org>, Tony Nguyen <anthony.l.nguyen@...el.com>,
	<netdev@...r.kernel.org>, Jakub Kicinski <kuba@...nel.org>, "Aleksandr
 Loktionov" <aleksandr.loktionov@...el.com>, Karol Kolacinski
	<karol.kolacinski@...el.com>, Grzegorz Nitka <grzegorz.nitka@...el.com>,
	Michal Schmidt <mschmidt@...hat.com>, Sergey Temerkhanov
	<sergey.temerkhanov@...el.com>
Subject: Re: [PATCH iwl-next] ice: use DSN instead of PCI BDF for ice_adapter
 index

On 3/7/2025 4:40 AM, Jiri Pirko wrote:
> Fri, Mar 07, 2025 at 12:53:05AM +0100, jacob.e.keller@...el.com wrote:
>>
>>
>> On 3/6/2025 1:11 PM, Przemek Kitszel wrote:
>>> Use Device Serial Number instead of PCI bus/device/function for
>>> index of struct ice_adapter.
>>> Functions on the same physical device should point to the very same
>>> ice_adapter instance.
>>>
>>> This is not only simplification, but also fixes things up when PF
>>> is passed to VM (and thus has a random BDF).
>>>
>>> Suggested-by: Jacob Keller <jacob.e.keller@...el.com>
>>> Suggested-by: Jakub Kicinski <kuba@...nel.org>
>>> Suggested-by: Jiri Pirko <jiri@...nulli.us>
>>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
>>> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
>>> ---
>>
>> The only caution I have here is that we might run into issues with
>> pre-production or poorly flashed boards which don't have DSN properly
>> flashed. This shouldn't be an impact outside of early testing or
>> mistakes by devs. I think there is a default ID which is almost all 0s
>> we could check and log a warning to help prevent confusion in such a case?
>>
>> A couple systems I've seen have serial numbers like:
>>
>>  serial_number 00-00-00-00-00-00-00-00
>>  serial_number 00-00-00-00-00-00-00-00
>>
>> or
>>
>>  serial_number 00-01-00-ff-ff-00-00-00
>>  serial_number 00-01-00-ff-ff-00-00-00
>>
>>
>> In practice I'm not sure how big a deal breaker this is. Properly
>> initialized boards should have unique IDs, and if you update via
>> devlink, or any of our standard update tools, it will maintain the ID
>> across flash. However, during early development, boards were often
>> flashed manually which could lead to such non-unique IDs.
> 
> Do we need a workaround for pre-production buggy hw now? Sounds a bit
> weird tbh.
> 

I agree that use of the serial number is preferred over BDF for the
reasons described in this thread.

But I also know that sometimes the DSN is not available, or is not set
properly during pre-production and early testing. This could cause
issues for early development. These issues can likely be worked around
and should not impact what we do for properly functioning boards.

I just want to make it clear on the record, since it is likely that we
would see this if using an old or badly flashed board, or if we use this
same scheme on a future hardware (or even just a spin of the ice
hardware). In those cases, developers might have breaking functionality
like multiple adapters being tied to the same adapter structure.

I *don't* want those to be reported as issues with this code, as they
are really issues with the flash data. Perhaps we could have some sort
of warning message to go "this doesn't look right" when the DSN
capability doesn't exist or when the DSN is 0.

In the end, the places where this is likely to fail are also the places
where hopefully the developers are smart enough to know what is going on.