Message-ID: <b7265975-d28c-4081-811c-bf7316954192@intel.com>
Date: Wed, 16 Jul 2025 13:25:45 +0300
From: "Ruinskiy, Dima" <dima.ruinskiy@...el.com>
To: "Keller, Jacob E" <jacob.e.keller@...el.com>, Simon Horman
	<horms@...nel.org>, "Lifshits, Vitaly" <vitaly.lifshits@...el.com>
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 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

Powered by blists - more mailing lists