netdev - Re: [PATCH iwl-next v3] e1000e: Fix real-time violations on link up

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

Message-ID: <af154371-8513-4ff2-a288-c8301cc8c65c@engleder-embedded.com>
Date: Wed, 18 Dec 2024 20:21:17 +0100
From: Gerhard Engleder <gerhard@...leder-embedded.com>
To: Przemek Kitszel <przemyslaw.kitszel@...el.com>,
 "Lifshits, Vitaly" <vitaly.lifshits@...el.com>
Cc: anthony.l.nguyen@...el.com, andrew+netdev@...n.ch, davem@...emloft.net,
 kuba@...nel.org, linux-pci@...r.kernel.org, edumazet@...gle.com,
 pabeni@...hat.com, bhelgaas@...gle.com, pmenzel@...gen.mpg.de,
 Gerhard Engleder <eg@...a.com>, netdev@...r.kernel.org,
 intel-wired-lan@...ts.osuosl.org
Subject: Re: [PATCH iwl-next v3] e1000e: Fix real-time violations on link up

On 18.12.24 09:36, Przemek Kitszel wrote:
> On 12/16/24 20:23, Gerhard Engleder wrote:
>>>> @@ -331,8 +331,15 @@ void e1000e_update_mc_addr_list_generic(struct 
>>>> e1000_hw *hw,
>>>>       }
>>>>       /* replace the entire MTA table */
>>>> -    for (i = hw->mac.mta_reg_count - 1; i >= 0; i--)
>>>> +    for (i = hw->mac.mta_reg_count - 1; i >= 0; i--) {
>>>>           E1000_WRITE_REG_ARRAY(hw, E1000_MTA, i, hw- 
>>>> >mac.mta_shadow[i]);
>>>> +
>>>> +        /* do not queue up too many posted writes to prevent increased
>>>> +         * latency for other devices on the interconnect
>>>> +         */
>>>> +        if ((i % 8) == 0 && i != 0)
>>>> +            e1e_flush();
>>>
>>>
>>> I would prefer to avoid adding this code to all devices, particularly 
>>> those that don't operate on real-time systems. Implementing this code 
>>> will introduce three additional MMIO transactions which will increase 
>>> the driver start time in various flows (up, probe, etc.).
>>>
>>> Is there a specific reason not to use if 
>>> (IS_ENABLED(CONFIG_PREEMPT_RT)) as Andrew initially suggested?
>>
>> Andrew made two suggestions: IS_ENABLED(CONFIG_PREEMPT_RT) which I used
>> in the first version after the RFC. And he suggested to check for a
>> compromise between RT and none RT performance, as some distros might
>> enable PREEMPT_RT in the future.
>> Przemek suggested to remove the PREEMPT_RT check as "this change sounds
>> reasonable also for the standard kernel" after the first version with
>> IS_ENABLED(CONFIG_PREEMPT_RT).
>>
>> I used the PREEMPT_RT dependency to limit effects to real-time systems,
>> to not make none real-time systems slower. But I could also follow the
>> reasoning of Andrew and Przemek. With that said, I have no problem to
>> add IS_ENABLED(CONFIG_PREEMPT_RT) again.
>>
>> Gerhard
> 
> I'm also fine with limiting the change to RT kernels.

I will add IS_ENABLED(CONFIG_PREEMPT_RT).

Thanks!

Gerhard