Re: [RFC PATCH net-next v2 1/6] ethtool: Add ability to control transceiver modules' power mode

On Thu, 19 Aug 2021 10:47:03 +0300 Ido Schimmel wrote:

quoted

quoted

+ * enum ethtool_module_power_mode_policy - plug-in module power mode policy
+ * @ETHTOOL_MODULE_POWER_MODE_POLICY_LOW: Module is always in low power mode.  

Did you have a use case for this one or is it for completeness? Seems
like user can just bring the port down if they want no carrier? My
understanding was you primarily wanted the latter two, and those can
be freely changed when netdev is running, right?  

In all the implementations I could find (3), the user interface is
high/low (on/off). The proposed interface is more flexible as it
contains both the 'high' / 'low' policies in addition to the more user
friendly 'high-on-up' ('auto') policy.

on/off is probably due to blindly setting the SFP bit. Our intention
here is to set a policy with respect to the netdev state (or integrate
the setting with the rest of the stack, if you will) not just control 
the bit.

IMO we should leave the value out of the enum until the use case for 
it becomes clear. Adding it later is simple enough.

Given that keeping the 'low' policy does not complicate the
implementation / maintenance

I'd argue it does. The netif_running() check is exactly to prevent
carrier from going away, so it only makes sense if the low setting 
exists. We can switch between 'auto' and 'high' any time.

and that it provides users with a similar interface to what they are
used to from other implementations, I would like to keep it in
addition to the other two policies.

IIUC the existing interfaces are build around the architecture where
driver/fw do not control the SFP automatically IOW 'auto' does not
exist. 'low' is there for disabling the SFP when interface is down.

The interface where 'low' can't be set while the netdev is up is
*already* not 1:1 with the out of tree APIs, right?

I'd bet that if you convert users of existing APIs to map 'on' to
(always-)high and 'off' to auto everyone will be happy.