Message-ID: <f44bfb41-d3bd-471c-916e-53fd66b04f27@intel.com>
Date: Wed, 30 Jul 2025 17:11:20 +0300
From: "Lifshits, Vitaly" <vitaly.lifshits@...el.com>
To: "Ruinskiy, Dima" <dima.ruinskiy@...el.com>, "Keller, Jacob E"
	<jacob.e.keller@...el.com>, Simon Horman <horms@...nel.org>
CC: "andrew+netdev@...n.ch" <andrew+netdev@...n.ch>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>
Subject: Re: [RFC net-next v1 1/1] e1000e: Introduce private flag and module
 param to disable K1



On 7/16/2025 1:25 PM, Ruinskiy, Dima wrote:
> On 15/07/2025 0:30, Keller, Jacob E wrote:
>>
>>
>>> -----Original Message-----
>>> From: Simon Horman <horms@...nel.org>
>>> Sent: Monday, July 14, 2025 9:55 AM
>>> To: Lifshits, Vitaly <vitaly.lifshits@...el.com>
>>> Cc: andrew+netdev@...n.ch; davem@...emloft.net; edumazet@...gle.com;
>>> kuba@...nel.org; pabeni@...hat.com; netdev@...r.kernel.org; Ruinskiy, 
>>> Dima
>>> <dima.ruinskiy@...el.com>; Nguyen, Anthony L 
>>> <anthony.l.nguyen@...el.com>;
>>> Keller, Jacob E <jacob.e.keller@...el.com>
>>> Subject: Re: [RFC net-next v1 1/1] e1000e: Introduce private flag and 
>>> module
>>> param to disable K1
>>>
>>> On Thu, Jul 10, 2025 at 12:24:55PM +0300, Vitaly Lifshits wrote:
>>>> The K1 state reduces power consumption on ICH family network 
>>>> controllers
>>>> during idle periods, similarly to L1 state on PCI Express NICs. 
>>>> Therefore,
>>>> it is recommended and enabled by default.
>>>> However, on some systems it has been observed to have adverse side
>>>> effects, such as packet loss. It has been established through debug 
>>>> that
>>>> the problem may be due to firmware misconfiguration of specific 
>>>> systems,
>>>> interoperability with certain link partners, or marginal electrical
>>>> conditions of specific units.
>>>>
>>>> These problems typically cannot be fixed in the field, and generic
>>>> workarounds to resolve the side effects on all systems, while 
>>>> keeping K1
>>>> enabled, were found infeasible.
>>>> Therefore, add the option for system administrators to globally disable
>>>> K1 idle state on the adapter.
>>>>
>>>> Link: https://lore.kernel.org/intel-wired-
>>> lan/CAMqyJG3LVqfgqMcTxeaPur_Jq0oQH7GgdxRuVtRX_6TTH2mX5Q@...l.gmail.
>>> com/
>>>> Link: https://lore.kernel.org/intel-wired-
>>> lan/20250626153544.1853d106@...x.my.domain/
>>>> Link: https://lore.kernel.org/intel-wired-lan/Z_z9EjcKtwHCQcZR@mail- 
>>>> itl/
>>>>
>>>> Signed-off-by: Vitaly Lifshits <vitaly.lifshits@...el.com>
>>>
>>> Hi Vitaly,
>>>
>>> If I understand things correctly, this patch adds a new module parameter
>>> to the e1000 driver. As adding new module parameters to networking 
>>> driver
>>> is discouraged I'd like to ask if another mechanism can be found.
>>> E.g. devlink.
>>
>> One motivation for the module parameter is that it is simple to set it 
>> "permanently" by setting the module parameter to be loaded by default. 
>> I don't think any distro has something equivalent for devlink or 
>> ethtool flags. Of course that’s not really the kernel's fault.
>>
>> I agree that new module parameters are generally discouraged from 
>> being added. A devlink parameter could work, but it does require 
>> administrator to script setting the parameter at boot on affected 
>> systems. This also will require a bit more work to implement because 
>> the e1000e driver does not expose devlink.
>>
>> Would an ethtool private flag on its own be sufficient/accepted..? I 
>> know those are also generally discouraged because of past attempts to 
>> avoid implementing generic interfaces.. However I don't think there is 
>> a "generic" interface for this, at least based on my understanding. It 
>> appears to be a low power state for the embedded device on a platform, 
>> which is quite specific to this device and hardware design ☹
> 
> Basically what we are looking for here is, as Jake mentioned, a way for 
> a system administrator / "power-user" to "permanently" set the driver 
> option in order to mask the issue on specific systems suffering from it.
> 
> As it can sometimes manifest during early hardware initialization 
> stages, I'm concerned that just an ethtool private flag is insufficient, 
> as it may be 'too late' to set it after 'probe'.
> 
> Not being familiar enough with devlink, I do not understand if it can be 
> active already as early as 'probe', but given the fact that e1000e 
> currently does not implement any devlink stuff, this would require a 
> bigger (and riskier?) change to the code. The module parameter is fairly 
> trivial, since e1000e already supports a number of these.
> 
> I do not know the history and why module parameters are discouraged, but 
> it seems that there has to be some standardized way to pass user 
> configuration to kernel modules, which takes effect as soon as the 
> module is loaded. I always thought module parameters were that 
> interface; if things have evolved, I would be happy to learn. :)
> 
> --Dima

While I understand that module params are generally discouraged—as
Jacob Keller pointed out—implementing the same functionality via devlink
presents some challenges. Although it may be technically feasible, it
would likely complicate configuration for sysadmins who need to disable
K1 on affected systems.

In my view, extending an existing interface in an older driver is a 
safer and more pragmatic approach than introducing a new one, especially 
given the legacy nature of the devices involved. These systems are often 
beyond the scope of our current test coverage, and minimizing the risk 
of regressions is critical.

Regarding the ethtool private flag: while it may not address all 
potential link issues, it does help mitigate certain packet loss 
scenarios. My motivation for proposing it was to offer end-users a 
straightforward workaround—by setting the flag and retriggering 
auto-negotiation, they may resolve issues without needing to unload and 
reload the e1000e module.

Powered by blists - more mailing lists