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

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]

Date:   Tue, 24 Aug 2021 23:18:56 +0000
From:   "Keller, Jacob E" <jacob.e.keller@...el.com>
To:     Jakub Kicinski <kuba@...nel.org>, Ido Schimmel <idosch@...sch.org>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "mkubecek@...e.cz" <mkubecek@...e.cz>,
        "pali@...nel.org" <pali@...nel.org>,
        "jiri@...dia.com" <jiri@...dia.com>,
        "vadimp@...dia.com" <vadimp@...dia.com>,
        "mlxsw@...dia.com" <mlxsw@...dia.com>,
        Ido Schimmel <idosch@...dia.com>
Subject: RE: [RFC PATCH net-next v3 1/6] ethtool: Add ability to control
 transceiver modules' power mode



> -----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Tuesday, August 24, 2021 4:13 PM
> To: Ido Schimmel <idosch@...sch.org>
> Cc: netdev@...r.kernel.org; davem@...emloft.net; andrew@...n.ch;
> mkubecek@...e.cz; pali@...nel.org; Keller, Jacob E <jacob.e.keller@...el.com>;
> jiri@...dia.com; vadimp@...dia.com; mlxsw@...dia.com; Ido Schimmel
> <idosch@...dia.com>
> Subject: Re: [RFC PATCH net-next v3 1/6] ethtool: Add ability to control
> transceiver modules' power mode
> 
> On Tue, 24 Aug 2021 16:03:39 +0300 Ido Schimmel wrote:
> > From: Ido Schimmel <idosch@...dia.com>
> >
> > Add a pair of new ethtool messages, 'ETHTOOL_MSG_MODULE_SET' and
> > 'ETHTOOL_MSG_MODULE_GET', that can be used to control transceiver
> > modules parameters and retrieve their status.
> 
> Lgtm! A few "take it or leave it" nit picks below.
> 
> > Signed-off-by: Ido Schimmel <idosch@...dia.com>
> 
> > +The optional ``ETHTOOL_A_MODULE_POWER_MODE_POLICY`` attribute
> encodes the
> > +transceiver module power mode policy enforced by the host. The default
> policy
> > +is driver-dependent and can be queried using this attribute.
> 
> Should we make a recommendation for those who don't have to worry about
> legacy behavior? Like:
> 
>   The default policy is driver-dependent (but "auto" is the recommended
>   and generally assumed to be used for drivers no implementing this API).
> 
> IMHO the "and can be queried using this attribute" part can be skipped.
> 
> > +/**
> > + * struct ethtool_module_power_mode_params - module power mode
> parameters
> > + * @policy: The power mode policy enforced by the host for the plug-in
> module.
> > + * @mode: The operational power mode of the plug-in module. Should be
> filled by
> > + * device drivers on get operations.
> 
> Indent continuation lines by one tab.
> 
> > + * @mode_valid: Indicates the validity of the @mode field. Should be set by
> > + * device drivers on get operations when a module is plugged-in.
> 
> Should we make a firm decision on whether we want to use these kind of
> valid bits or choose invalid defaults? As you may guess my preference
> is the latter since that's what I usually do, that way drivers don't
> have to write two fields.
> 
> Actually I think this may be the first "valid" in ethtool, I thought we
> already had one but I don't see it now..
> 


coalesce settings have a valid mode don't they? Or at least an "accepted modes"?

Thanks,
Jake

> > +struct ethtool_module_power_mode_params {
> > +	enum ethtool_module_power_mode_policy policy;
> > +	enum ethtool_module_power_mode mode;
> > +	u8 mode_valid:1;
> > +};