netdev - Re: [PATCH] r8169: skip DASH fw status checks when DASH is disabled

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: <0dee563a-08ea-4e50-b285-5d0527458057@gmail.com>
Date: Fri, 22 Mar 2024 11:16:50 +0100
From: Heiner Kallweit <hkallweit1@...il.com>
To: pseudoc <atlas.yu@...onical.com>, hau@...ltek.com
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 netdev@...r.kernel.org, nic_swsd@...ltek.com, pabeni@...hat.com
Subject: Re: [PATCH] r8169: skip DASH fw status checks when DASH is disabled

On 22.03.2024 09:33, pseudoc wrote:
> On Fri, Mar 22, 2024 at 3:01 PM Heiner Kallweit <hkallweit1@...il.com> wrote:
>> To me this seems to be somewhat in conflict with the commit message of the
>> original change. There's a statement that DASH firmware may influence driver
>> behavior even if DASH is disabled.
>> I think we have to consider three cases in the driver:
>> 1. DASH enabled (implies firmware is present)
>> 2. DASH disabled (firmware present)
>> 3. DASH disabled (no firmware)
> 
>> I assume your change is for case 3.
> I checked the r8168 driver[1], for both DP and EP DASH types,
> "rtl8168_wait_dash_fw_ready" will immediately return if DASH is disabled.
> So I think the firmware presence doesn't really matter.
> 
>> Is there a way to detect firmware presence on driver load?
> By comparing r8168_n.c and r8169_main.c, I think "rtl_ep_ocp_read_cond" and
> "rtl_dp_ocp_read_cond" is checking that, which is redundant when DASH is disabled.
> 
No, this only checks whether DASH is enabled.
I don't think is redundant, because the original change explicitly mentions that
DASH fw may impact behavior even if DASH is disabled.

I understand that on your test system DASH is disabled. But does your system have
a DASH fw or not?

My assumption is that the poll loop is relevant on systems with DASH fw, even if
DASH is disabled. I'd appreciate if somebody from Realtek could comment on this. Hau?
Including the question whether DASH fw presence can be detected, even if DASH is disabled.

> [1] r8168 driver: https://www.realtek.com/en/component/zoo/category/network-interface-controllers-10-100-1000m-gigabit-ethernet-pci-express-software