| 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
| ||
|
Date: Fri, 29 Mar 2024 15:29:54 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Wojciech Drewek <wojciech.drewek@...el.com>
Cc: netdev@...r.kernel.org, intel-wired-lan@...ts.osuosl.org,
simon.horman@...igine.com, anthony.l.nguyen@...el.com, edumazet@...gle.com,
pabeni@...hat.com, idosch@...dia.com, przemyslaw.kitszel@...el.com,
marcin.szycik@...ux.intel.com
Subject: Re: [PATCH net-next 2/3] ethtool: Introduce max power support
On Fri, 29 Mar 2024 10:23:20 +0100 Wojciech Drewek wrote:
> Some modules use nonstandard power levels. Adjust ethtool
> module implementation to support new attributes that will allow user
> to change maximum power.
>
> Add three new get attributes:
> ETHTOOL_A_MODULE_MAX_POWER_SET (used for set as well) - currently set
> maximum power in the cage
1) I'd keep the ETHTOOL_A_MODULE_POWER_ prefix, consistently.
2) The _SET makes it sound like an action. Can we go with
ETHTOOL_A_MODULE_POWER_MAX ? Or ETHTOOL_A_MODULE_POWER_LIMIT?
Yes, ETHTOOL_A_MODULE_POWER_LIMIT
ETHTOOL_A_MODULE_POWER_MAX
ETHTOOL_A_MODULE_POWER_MIN
would sound pretty good to me.
> ETHTOOL_A_MODULE_MIN_POWER_ALLOWED - minimum power allowed in the
> cage reported by device
> ETHTOOL_A_MODULE_MAX_POWER_ALLOWED - maximum power allowed in the
> cage reported by device
>
> Add two new set attributes:
> ETHTOOL_A_MODULE_MAX_POWER_SET (used for get as well) - change
> maximum power in the cage to the given value (milliwatts)
> ETHTOOL_A_MODULE_MAX_POWER_RESET - reset maximum power setting to the
> default value
>
> Reviewed-by: Marcin Szycik <marcin.szycik@...ux.intel.com>
> Signed-off-by: Wojciech Drewek <wojciech.drewek@...el.com>
> ---
> include/linux/ethtool.h | 17 +++++--
> include/uapi/linux/ethtool_netlink.h | 4 ++
> net/ethtool/module.c | 74 ++++++++++++++++++++++++++--
> net/ethtool/netlink.h | 2 +-
> 4 files changed, 87 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index f3af6b31c9f1..74ed8997443a 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -510,10 +510,18 @@ struct ethtool_module_eeprom {
> * @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.
> + * @min_pwr_allowed: minimum power allowed in the cage reported by device
> + * @max_pwr_allowed: maximum power allowed in the cage reported by device
> + * @max_pwr_set: maximum power currently set in the cage
> + * @max_pwr_reset: restore default minimum power
> */
> struct ethtool_module_power_params {
> enum ethtool_module_power_mode_policy policy;
> enum ethtool_module_power_mode mode;
> + u32 min_pwr_allowed;
> + u32 max_pwr_allowed;
> + u32 max_pwr_set;
> + u8 max_pwr_reset;
bool ?
> diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
> index 3f89074aa06c..f7cd446b2a83 100644
> --- a/include/uapi/linux/ethtool_netlink.h
> +++ b/include/uapi/linux/ethtool_netlink.h
> @@ -882,6 +882,10 @@ enum {
> ETHTOOL_A_MODULE_HEADER, /* nest - _A_HEADER_* */
> ETHTOOL_A_MODULE_POWER_MODE_POLICY, /* u8 */
> ETHTOOL_A_MODULE_POWER_MODE, /* u8 */
> + ETHTOOL_A_MODULE_MAX_POWER_SET, /* u32 */
> + ETHTOOL_A_MODULE_MIN_POWER_ALLOWED, /* u32 */
> + ETHTOOL_A_MODULE_MAX_POWER_ALLOWED, /* u32 */
> + ETHTOOL_A_MODULE_MAX_POWER_RESET, /* u8 */
flag ?
> @@ -77,6 +86,7 @@ static int module_fill_reply(struct sk_buff *skb,
> const struct ethnl_reply_data *reply_base)
> {
> const struct module_reply_data *data = MODULE_REPDATA(reply_base);
> + u32 temp;
tmp ? temp sounds too much like temperature in context of power
> static int
> ethnl_set_module(struct ethnl_req_info *req_info, struct genl_info *info)
> {
> struct ethtool_module_power_params power = {};
> struct ethtool_module_power_params power_new;
> - const struct ethtool_ops *ops;
> struct net_device *dev = req_info->dev;
> struct nlattr **tb = info->attrs;
> + const struct ethtool_ops *ops;
> int ret;
> + bool mod;
>
> ops = dev->ethtool_ops;
>
> - power_new.policy = nla_get_u8(tb[ETHTOOL_A_MODULE_POWER_MODE_POLICY]);
> ret = ops->get_module_power_cfg(dev, &power, info->extack);
> if (ret < 0)
> return ret;
>
> - if (power_new.policy == power.policy)
> + power_new.max_pwr_set = power.max_pwr_set;
> + power_new.policy = power.policy;
> +
> + ethnl_update_u32(&power_new.max_pwr_set,
> + tb[ETHTOOL_A_MODULE_MAX_POWER_SET], &mod);
> + if (mod) {
I think we can use if (tb[ETHTOOL_A_MODULE_MAX_POWER_SET]) here
Less error prone for future additions.
> + if (power_new.max_pwr_set > power.max_pwr_allowed) {
> + NL_SET_ERR_MSG(info->extack, "Provided value is higher than maximum allowed");
NL_SET_ERR_MSG_ATTR() to point at the bad attribute.
> + return -EINVAL;
ERANGE?
> + } else if (power_new.max_pwr_set < power.min_pwr_allowed) {
> + NL_SET_ERR_MSG(info->extack, "Provided value is lower than minimum allowed");
> + return -EINVAL;
> + }
> + }
> +
> + ethnl_update_policy(&power_new.policy,
> + tb[ETHTOOL_A_MODULE_POWER_MODE_POLICY], &mod);
> + ethnl_update_u8(&power_new.max_pwr_reset,
> + tb[ETHTOOL_A_MODULE_MAX_POWER_RESET], &mod);
I reckon reset should not be allowed if none of the max_pwr values
are set (i.e. most likely driver doesn't support the config)?
> + if (!mod)
> return 0;
>
> + if (power_new.max_pwr_reset && power_new.max_pwr_set) {
Mmm. How is that gonna work? The driver is going to set max_pwr_set
to what's currently configured. So the user is expected to send
ETHTOOL_A_MODULE_MAX_POWER_SET = 0
ETHTOOL_A_MODULE_MAX_POWER_RESET = 1
to reset?
Just:
if (tb[ETHTOOL_A_MODULE_MAX_POWER_RESET] &&
tb[ETHTOOL_A_MODULE_MAX_POWER_SET])
And you can validate this before doing any real work.
> + NL_SET_ERR_MSG(info->extack, "Maximum power set and reset cannot be used at the same time");
> + return 0;
> + }
> +
> ret = ops->set_module_power_cfg(dev, &power_new, info->extack);
> return ret < 0 ? ret : 1;
> }
--
pw-bot: cr
X-sender: <netdev+bounces-83478-steffen.klassert=secunet.com@...r.kernel.org>
X-Receiver: <steffen.klassert@...unet.com> ORCPT=rfc822;steffen.klassert@...unet.com NOTIFY=NEVER; X-ExtendedProps=BQAVABYAAgAAAAUAFAARAPDFCS25BAlDktII2g02frgPADUAAABNaWNyb3NvZnQuRXhjaGFuZ2UuVHJhbnNwb3J0LkRpcmVjdG9yeURhdGEuSXNSZXNvdXJjZQIAAAUAagAJAAEAAAAAAAAABQAWAAIAAAUAQwACAAAFAEYABwADAAAABQBHAAIAAAUAEgAPAGIAAAAvbz1zZWN1bmV0L291PUV4Y2hhbmdlIEFkbWluaXN0cmF0aXZlIEdyb3VwIChGWURJQk9IRjIzU1BETFQpL2NuPVJlY2lwaWVudHMvY249U3RlZmZlbiBLbGFzc2VydDY4YwUACwAXAL4AAACheZxkHSGBRqAcAp3ukbifQ049REI2LENOPURhdGFiYXNlcyxDTj1FeGNoYW5nZSBBZG1pbmlzdHJhdGl2ZSBHcm91cCAoRllESUJPSEYyM1NQRExUKSxDTj1BZG1pbmlzdHJhdGl2ZSBHcm91cHMsQ049c2VjdW5ldCxDTj1NaWNyb3NvZnQgRXhjaGFuZ2UsQ049U2VydmljZXMsQ049Q29uZmlndXJhdGlvbixEQz1zZWN1bmV0LERDPWRlBQAOABEABiAS9uuMOkqzwmEZDvWNNQUAHQAPAAwAAABtYngtZXNzZW4tMDIFADwAAgAADwA2AAAATWljcm9zb2Z0LkV4Y2hhbmdlLlRyYW5zcG9ydC5NYWlsUmVjaXBpZW50LkRpc3BsYXlOYW1lDwARAAAAS2xhc3NlcnQsIFN0ZWZmZW4FAAwAAgAABQBsAAIAAAUAWAAXAEoAAADwxQktuQQJQ5LSCNoNNn64Q049S2xhc3NlcnQgU3RlZmZlbixPVT1Vc2VycyxPVT1NaWdyYXRpb24sREM9c2VjdW5ldCxEQz1kZQUAJgACAAEFACIADwAxAAAAQXV0b1Jlc3BvbnNlU3VwcHJlc3M6IDANClRyYW5zbWl0SGlzdG9yeTogRmFsc2UNCg8ALwAAAE1pY3Jvc29mdC5FeGNoYW5nZS5UcmFuc3BvcnQuRXhwYW5zaW9uR3JvdXBUeXBlDwAVAAAATWVtYmVyc0dyb3VwRXhwYW5zaW9uBQAjAAIAAQ==
X-CreatedBy: MSExchange15
X-HeloDomain: a.mx.secunet.com
X-ExtendedProps: BQBjAAoAkQ1rGbMv3AgFAGEACAABAAAABQA3AAIAAA8APAAAAE1pY3Jvc29mdC5FeGNoYW5nZS5UcmFuc3BvcnQuTWFpbFJlY2lwaWVudC5Pcmdhbml6YXRpb25TY29wZREAAAAAAAAAAAAAAAAAAAAAAAUASQACAAEFAAQAFCABAAAAHAAAAHN0ZWZmZW4ua2xhc3NlcnRAc2VjdW5ldC5jb20FAAYAAgABBQApAAIAAQ8ACQAAAENJQXVkaXRlZAIAAQUAAgAHAAEAAAAFAAMABwAAAAAABQAFAAIAAQUAYgAKAFIAAADMigAABQBkAA8AAwAAAEh1Yg==
X-Source: SMTP:Default MBX-ESSEN-01
X-SourceIPAddress: 62.96.220.36
X-EndOfInjectedXHeaders: 20181
Received: from cas-essen-02.secunet.de (10.53.40.202) by
mbx-essen-01.secunet.de (10.53.40.197) with Microsoft SMTP Server
(version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
15.1.2507.35; Fri, 29 Mar 2024 23:30:12 +0100
Received: from a.mx.secunet.com (62.96.220.36) by cas-essen-02.secunet.de
(10.53.40.202) with Microsoft SMTP Server (version=TLS1_2,
cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35 via Frontend
Transport; Fri, 29 Mar 2024 23:30:12 +0100
Received: from localhost (localhost [127.0.0.1])
by a.mx.secunet.com (Postfix) with ESMTP id 870DE20883
for <steffen.klassert@...unet.com>; Fri, 29 Mar 2024 23:30:12 +0100 (CET)
X-Virus-Scanned: by secunet
X-Spam-Flag: NO
X-Spam-Score: -3.099
X-Spam-Level:
X-Spam-Status: No, score=-3.099 tagged_above=-999 required=2.1
tests=[BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.099, DKIM_SIGNED=0.1,
DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, MAILING_LIST_MULTI=-1,
RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001]
autolearn=unavailable autolearn_force=no
Authentication-Results: a.mx.secunet.com (amavisd-new);
dkim=pass (2048-bit key) header.d=kernel.org
Received: from a.mx.secunet.com ([127.0.0.1])
by localhost (a.mx.secunet.com [127.0.0.1]) (amavisd-new, port 10024)
with ESMTP id 66TQ_oaDF-3X for <steffen.klassert@...unet.com>;
Fri, 29 Mar 2024 23:30:11 +0100 (CET)
Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=147.75.80.249; helo=am.mirrors.kernel.org; envelope-from=netdev+bounces-83478-steffen.klassert=secunet.com@...r.kernel.org; receiver=steffen.klassert@...unet.com
DKIM-Filter: OpenDKIM Filter v2.11.0 a.mx.secunet.com D1BDB208AC
Received: from am.mirrors.kernel.org (am.mirrors.kernel.org [147.75.80.249])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(No client certificate requested)
by a.mx.secunet.com (Postfix) with ESMTPS id D1BDB208AC
for <steffen.klassert@...unet.com>; Fri, 29 Mar 2024 23:30:11 +0100 (CET)
Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(No client certificate requested)
by am.mirrors.kernel.org (Postfix) with ESMTPS id 5B4B71F22D96
for <steffen.klassert@...unet.com>; Fri, 29 Mar 2024 22:30:11 +0000 (UTC)
Received: from localhost.localdomain (localhost.localdomain [127.0.0.1])
by smtp.subspace.kernel.org (Postfix) with ESMTP id 5B38413DDA5;
Fri, 29 Mar 2024 22:29:56 +0000 (UTC)
Authentication-Results: smtp.subspace.kernel.org;
dkim=pass (2048-bit key) header.d=kernel.org header.i=@...nel.org header.b="af3tEf4r"
X-Original-To: netdev@...r.kernel.org
Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(No client certificate requested)
by smtp.subspace.kernel.org (Postfix) with ESMTPS id BF6BB28DC1
for <netdev@...r.kernel.org>; Fri, 29 Mar 2024 22:29:55 +0000 (UTC)
Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201
ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;
t=1711751395; cv=none; b=r8+B1IFFag2HuI6zBBZXeH+ixu4+v7LcY5wOF3/6wgJ223E0kn3xcKcwo+b9S0QAED6F64X45+Ly5CTR1T3QpysOskVw+gmCEHA7C6kqyn9w3eNJ9i4Hl/Myvb/UKIYrlUrLJA2ZIcn/zPzyZPRsgS1BxBM9vsbq2bHqgBZeDjM=
ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org;
s=arc-20240116; t=1711751395; c=relaxed/simple;
bh=YclD2gFKAd0KYU/nqrMwp6tntz6Bp0xkpGNnD7iuj3c=;
h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References:
MIME-Version:Content-Type; b=rJ7Bn5B+eJPtxb4RNqsOXTzMjoxUUJ5pI/JOpQlNhT4ZcDDv6O01CZ1g3k27TriDuD2V9f4K/PGRphgNiz/gM/TFCcH5mAojrujO3pTOIJTI+aIIUz1rLn0diYOJV7K7HUs8cBglYDPH5ri6aPJNGmrNMWJbh0ZerjwDrcQhuoc=
ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@...nel.org header.b=af3tEf4r; arc=none smtp.client-ip=10.30.226.201
Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2A38DC433F1;
Fri, 29 Mar 2024 22:29:55 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;
s=k20201202; t=1711751395;
bh=YclD2gFKAd0KYU/nqrMwp6tntz6Bp0xkpGNnD7iuj3c=;
h=Date:From:To:Cc:Subject:In-Reply-To:References:From;
b=af3tEf4riCb4f2NQ149pjvDrIXXmmP43YUOkyHbXZ+M94QTqDI0JCGEF6C9SwDi2v
UbNo6lur4NhXefe9RSrYlvWkEgyygoEXlsnzAgBuwTthmMcxP2nKYOexYi7y8EYgAU
s+LYfSGZY1szJRSJJk68i3GvMqw/Vxj3slvg7t75/MisPpwS+jb6RoyLsnYv0RKoVL
12qu5ji4XYH50ruUZsUfcfxQseOwTzwtSilm9SNsMlGhgFPnOmb3sh5+EZ9kkw1axQ
GhY5mxcMFxbnq+OPRpafTOvZpCjxq7fMQ4RgncWl/e6+tXnFeaTluLTysE2h8/dy/H
aDL1nOLZRHWXA==
Date: Fri, 29 Mar 2024 15:29:54 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Wojciech Drewek <wojciech.drewek@...el.com>
Cc: netdev@...r.kernel.org, intel-wired-lan@...ts.osuosl.org,
simon.horman@...igine.com, anthony.l.nguyen@...el.com, edumazet@...gle.com,
pabeni@...hat.com, idosch@...dia.com, przemyslaw.kitszel@...el.com,
marcin.szycik@...ux.intel.com
Subject: Re: [PATCH net-next 2/3] ethtool: Introduce max power support
Message-ID: <20240329152954.26a7ce75@...nel.org>
In-Reply-To: <20240329092321.16843-3-wojciech.drewek@...el.com>
References: <20240329092321.16843-1-wojciech.drewek@...el.com>
<20240329092321.16843-3-wojciech.drewek@...el.com>
Precedence: bulk
X-Mailing-List: netdev@...r.kernel.org
List-Id: <netdev.vger.kernel.org>
List-Subscribe: <mailto:netdev+subscribe@...r.kernel.org>
List-Unsubscribe: <mailto:netdev+unsubscribe@...r.kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit
Return-Path: netdev+bounces-83478-steffen.klassert=secunet.com@...r.kernel.org
X-MS-Exchange-Organization-OriginalArrivalTime: 29 Mar 2024 22:30:12.5869
(UTC)
X-MS-Exchange-Organization-Network-Message-Id: b874bb8e-c3ad-4cd8-d138-08dc503fcc9e
X-MS-Exchange-Organization-OriginalClientIPAddress: 62.96.220.36
X-MS-Exchange-Organization-OriginalServerIPAddress: 10.53.40.202
X-MS-Exchange-Organization-Cross-Premises-Headers-Processed: cas-essen-02.secunet.de
X-MS-Exchange-Organization-OrderedPrecisionLatencyInProgress: LSRV=mbx-essen-01.secunet.de:TOTAL-HUB=0.416|SMR=0.329(SMRDE=0.004|SMRC=0.325(SMRCL=0.104|X-SMRCR=0.325))|CAT=0.086(CATRESL=0.022
(CATRESLP2R=0.018)|CATORES=0.058(CATRS=0.058(CATRS-Transport Rule
Agent=0.001|CATRS-Index Routing Agent=0.056
))|CATORT=0.001(CATRT=0.001));2024-03-29T22:30:13.029Z
X-MS-Exchange-Forest-ArrivalHubServer: mbx-essen-01.secunet.de
X-MS-Exchange-Organization-AuthSource: cas-essen-02.secunet.de
X-MS-Exchange-Organization-AuthAs: Anonymous
X-MS-Exchange-Organization-FromEntityHeader: Internet
X-MS-Exchange-Organization-OriginalSize: 12132
X-MS-Exchange-Organization-HygienePolicy: Standard
X-MS-Exchange-Organization-MessageLatency: SRV=cas-essen-02.secunet.de:TOTAL-FE=25.002|SMR=0.025(SMRPI=0.022(SMRPI-FrontendProxyAgent=0.022))|SMS=0.002
X-MS-Exchange-Organization-Recipient-Limit-Verified: True
X-MS-Exchange-Organization-TotalRecipientCount: 1
X-MS-Exchange-Organization-Rules-Execution-History: 0b0cf904-14ac-4724-8bdf-482ee6223cf2%%%fd34672d-751c-45ae-a963-ed177fcabe23%%%d8080257-b0c3-47b4-b0db-23bc0c8ddb3c%%%95e591a2-5d7d-4afa-b1d0-7573d6c0a5d9%%%f7d0f6bc-4dcc-4876-8c5d-b3d6ddbb3d55%%%16355082-c50b-4214-9c7d-d39575f9f79b
X-MS-Exchange-Forest-RulesExecuted: mbx-essen-01
X-MS-Exchange-Organization-RulesExecuted: mbx-essen-01
X-MS-Exchange-Forest-IndexAgent-0: AQ0CZW4AAWoMAAAPAAADH4sIAAAAAAAEAK1YCXPbxhVe3odEyXKcZH
J2k6a2KJG0rlqybNnWxGqijmx5ZLVpppPhgMSSQgQCLACaUpP8tP63
vmNBAiRIyW0wGnGx+659x7dv8Z/NU0f+xbNqcuuxfGV4cmtja0dubu
xvbe9vbcj1jc2NDfmD+3PbUu0L+dJTQ3Uph54bqP1K+Zl86/aU7Lnm
wFa+HPhKOq7jB4ZjGp4p++5QedJW75TtN+Sh+fPAD6QKLgLXtZGZ+a
TV69uqp5zACCzXkYEr/UG/73qBdNRQGkHgWa1BAPKDCyOQQ8u2pWHb
7hD1eSgHONoXhtMFS4wrqzfoseYGrOHyoWkCq6cUyeuqICKTNnF0/v
356elJ87D56vTl306Omq8O/9F8c/rD0Vnz7dG5XAU9puy4nvSR15dD
ZdtVWZftgeeB2fY1LqAgGTdAWrCbCyXbRldVypXyZlUePzDlpVJ9mp
/Syzpl31Md66om2+BLyw9IRQMFbFXlOfCRVT3jElxiBdJ3B44pbetS
ScORRhud2JDfwnioZNcFhwUXlTLYNkMdbFY+l6ferPWT41fH589Jwo
/Kr80lIyp6Ziu7meb4NdEM3YFt6u2BS4LgGrbjmhjuniJ/JIXu+LUW
c3hyAr8vIU49y4kEhXIHAsrB4bBhhKSnMOlgpXUtTfXOaqsbkiOiIR
b299MwStGhSwnq/x8J2p1IUKqKGzIT/YnjrvVOOfKdYQ+UXO1BlVlD
MMOv3mDA2RGaUIe9oeFxLTATWE5Xa2A7TNUxBnbAivT2z8AXACxmvX
W9jyDUBvPe/vu6bV3Kpz16bfj0+sK2nMFVw3ICZTfabo+431pdB3jd
Tof4J8Hq6VBPNEyaeBHnrtfrZJjltO2BqR6ShocapRoXMvb8Kjd35T
o+E1wDo2/FWZuOCmDiEkT8KuUOcBEDzIYUDxn/Gm05+fwqd3dYTeKj
dUdFjZVNiZJbcp0ZdmTHQpzmtDBrcm8XNgAgipDhr65XawD8ECBb8U
S9quNjWp0OOKoLaGM8nOWo1qwVlGA5prqSnW2j86i1vdl+3NlsNHZ3
lLn3+PHuzs62AYo3Hu3s6IDM1oIE4IH5yl68kPU/b27UYDPr9LuHU3
7gDdqj86fJzm8CFntuT/7CybkmX/Rd22pDGiHQchYDJQ5xWioHyqzN
FYxFc+HCkYaVhy99e9CtQ+rquI5EogAW6PaVR6ecYUeFu50kfvn2gh
CwpTBuNikNZUqNH9L0oGw9X8LBicU/UuCT+nVSbznN/tBraljan4uH
86CQpRlXE9LmYN+tpQFSTEqKna7xo3SCmaBnHxEocD01QpjYNrXjHt
JvcioQXbNveEbPDxMCH+WAkERaDF5TZwb/PLktGwX+CW8Fn8H2lpwI
1ORq3PEzVsET0ZU9GXPSEzw1W2CQfM7nZ2JlzwWz1q3IxjW/3dl7vL
G7Yxgbj9qNRme3be7sPGptGXvbM2v+JsFxALiJGtFgb2+r9kiu4w+A
AsxQcCIxnjrgvj86fHl0VpuE04drALtQ8nUJpEzTXAvTam5Tc/oShy
fH3/5YCyVBdG7JWZuwYcS5nswZ6w9qMU7Ilfmskw1U7fask53R/8JK
/URtaquVcsc2umHaYkx3dymkj2q7jO4Ae23IuUDDZxMxE7K+b1+v6n
r3L5utAeT7mn/Zqo39nvhg7x1EgMLRspqmERhyjcctw1fUH0VTKcap
bYmy0v8DqTd+dvTm5eH54WpE4ERlB6rXp7oNen1o1PGVm2K4ELnQCg
+g1aHuH1cQ/QeAgQCXYEegrgI8XDQCauwLHUWvvDWABg1RqxNb/lfT
giMPN8yjWrizroJ1XsP/U264BcYyzh/IX3578v58UORDYqsnxouY3b
4v1+DftHhAiKY+QdfgF4wIN1h/Bu8JDDb25HJtLWgBMRPijB+J1m1t
wCT1YjhNkAxbfaIbLn6Q9QBPzPqziLSQhvc98kVDn0MHaGoTeoHmYG
81aP3zZjz6qRqxDewCEaCm/qw7Sgrt/nanuwrW1OR9eq9pN0CSGe3L
qBCrI1dR0FO5UZ2uM1gZeE7ogfFWkGt6Owf6Ms/v1bHPxqSRs0+G5M
nnYYK3otKZckzORTDoQ8mqJtTi6v1EpbUxx9STFIEYOv8E7gQvR6se
HYFTWE6V8jE0PnCY4W2+DZd6/MiCBDcLrsoL5UG3dKJ8XyrPgy4Vel
1HUb/aGRBOGKZphe3i9C7iEYm6+dmEm3VTUmUESPbF6xM0q3l0Btn3
9rvVaPLU5NdvPPedZUKTyDdQy5cXVhc2gF99nFFrqPV8XSVEjEtsHp
6fn61W8bbZd7HGjIB6xpZhji/UifucyMz60fHrvx+ekIqjs8PX3x09
T+L6TSpbByPZSU9DJ8X7ut/VSTZhKPtI97tRH916p1ObG0/9Nq8quH
CihcEz82pCJpfFNDBNl0a8IveSCpIa3Tnq5xcOtR8RxVh/nmpfuo7+
wuHzpcxxA7yYja47HfzyObrJaVs4TH6lbECpIfOq1YB7XQ8vjXhkw9
2G72/SdJXvPAhGXz7pruM6HatbjeUeJttXaNpMWN0IQXXMMMNH8v79
ZBDVwPOq12vI790hphl9eu26jmPIoetdPqfbrLYdlruu/syjvwGFkq
BpceUQeB/4kfscbwzgx4Q7Ln98wk+5KEhd9VUbr4okyzGhAOd/+zqQ
G3Np+PPUgdwkU2jf5NC/DvxgHweh+27GVE4N8NqY6eZ8IiBGPYeOKa
/dAWE4pIWFGYzI7kMadejaSj40nGuw0rDJzYlgNR8hXk1+gAOJpk5d
UK3Tlj4Yanj0jR4YYvVUImBEs2oKD0YkYcvg39AyYKbNbhvGjQF2Dt
Dr4mhfbjIJqMUPX/1hveXCbb9NDa0QaZHJiFw6JRbgD8fZbCpXTImi
EMVUKSdETuSLopQXhZwoZ0WuIIpAsyAWYT4vijBfEAvwf0ksgyheLY
sFoIQ/oMmKIswTWTEnKrAKk/CfyJYLKfEl6BFpIAiXkJEJgBHYS6Kc
ESViz6e1hcAiUjwmYpDzcSgnJwpg2GJqJaTJsWTgJXtyZE+R5ZOKEr
B/OmYHG5AYJj+PTObEAoiamlye4q1MzRSJEtxVYO1p0vh1fOMFUeY9
FshjsGUgBs+TteX3pK+wA9/Lz3mxGHMymcrE4FIWIuNCcqEBUTPodb
GI6QTGZPMUjnxKfIEDdhr6BGYqJAdoKMGKwPKJzjQguDMSm0qtpDFR
l2HMSQWWLIsiLeUoyiXMAcrGVOoeEX/DM0RzD2lSJaJME3uJaDLl1D
IPSuKDLBkDBGO9Ih1/RdWLYmkxVckLkRdL8dVS7DUFpoK4HMpPZcZj
SgOw/0OyNi9WOBU5M2GGIzhKbMp2UdBJeydLtcPVNwpQhABlcrmlxS
IXHdOUxb0MOi1LbinFQ5aNZM7y7ckWdILlmIzqNBtSVjIY3yxtrcCB
ILK7vP3PyFQqirtZKisZzrAfwDM8/0lkPgt6QebYwruJr6FhS2jM2A
yAr8VolCdzKQz31Hxy3KfISsnzYSZQEqajmZCFJI8AV158mk1BoaXo
9cNoyYduWU5TGX4Wm7+XTi7wOxmSP4UbN8xP1fgdwKtsCjwvMqks72
I8pmABJZTzCuEbA10aczJD6fpHnoH936Hah0kYl3HwIS8xLy9l6XWJ
zwtNdocyCmpc5wanIvxBIfChQ9i+SEtFVpTRyIN6l+gVjcFzDQcIxR
AwFIJmTOUk/E+aRBYQtZQLq3gBixfqdAlVo7VZKuexIl7KY9WgLnDm
NC/nKtX7SoaQOVxdCTWC2BLLHx1bSenxAbMD5T3afnjUIoDTru+msU
DyFLh75C6A1mxES5nMQ70V5k1liiKlhTDZeGYpah4xfsTOJ8m5JCF/
YMunchUGy2ibKDNEM4zw6TCDhbMlFx4ouN+S+JK3M8f4RZopiG/SmB
iZ0SlJ/ikVaSMModztFABGUuKDGGWBe4mRkSVxn7AXGqEvCJZ/Hxs4
rGTDNyR/J4eW5PlIjTj/TzPyAQB5OYdARCUZt6pMNuSEZN6pqodyK4
2k5fVJrWttdPRksMyzGZ32WcrnXAjgGvZpFTAKlj7KpwqUe8szjCkl
+a3MLRzrZZTgcMeiEDmPOHxwHpUJH0bNDB9eWQLnyXTSRZHkirDbJH
VwiKyMcnLKaR/PyNXSKMRF6tNGtUk49kliGvCpmtGHF0dw7HlmDP35
+czC10mS4/InmOKdjnAA0zvPt4D/AsaXde/OJAAAAQrOAjw/eG1sIH
ZlcnNpb249IjEuMCIgZW5jb2Rpbmc9InV0Zi0xNiI/Pg0KPEVtYWls
U2V0Pg0KICA8VmVyc2lvbj4xNS4wLjAuMDwvVmVyc2lvbj4NCiAgPE
VtYWlscz4NCiAgICA8RW1haWwgU3RhcnRJbmRleD0iMTE4MyI+DQog
ICAgICA8RW1haWxTdHJpbmc+bWFyY2luLnN6eWNpa0BsaW51eC5pbn
RlbC5jb208L0VtYWlsU3RyaW5nPg0KICAgIDwvRW1haWw+DQogICAg
PEVtYWlsIFN0YXJ0SW5kZXg9IjEyNDkiPg0KICAgICAgPEVtYWlsU3
RyaW5nPndvamNpZWNoLmRyZXdla0BpbnRlbC5jb208L0VtYWlsU3Ry
aW5nPg0KICAgIDwvRW1haWw+DQogIDwvRW1haWxzPg0KPC9FbWFpbF
NldD4BDIUHPD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0idXRm
LTE2Ij8+DQo8Q29udGFjdFNldD4NCiAgPFZlcnNpb24+MTUuMC4wLj
A8L1ZlcnNpb24+DQogIDxDb250YWN0cz4NCiAgICA8Q29udGFjdCBT
dGFydEluZGV4PSIxMTY4Ij4NCiAgICAgIDxQZXJzb24gU3RhcnRJbm
RleD0iMTE2OCI+DQogICAgICAgIDxQZXJzb25TdHJpbmc+TWFyY2lu
PC9QZXJzb25TdHJpbmc+DQogICAgICA8L1BlcnNvbj4NCiAgICAgID
xFbWFpbHM+DQogICAgICAgIDxFbWFpbCBTdGFydEluZGV4PSIxMTgz
Ij4NCiAgICAgICAgICA8RW1haWxTdHJpbmc+bWFyY2luLnN6eWNpa0
BsaW51eC5pbnRlbC5jb208L0VtYWlsU3RyaW5nPg0KICAgICAgICA8
L0VtYWlsPg0KICAgICAgPC9FbWFpbHM+DQogICAgICA8Q29udGFjdF
N0cmluZz5NYXJjaW4gU3p5Y2lrICZsdDttYXJjaW4uc3p5Y2lrQGxp
bnV4LmludGVsLmNvbTwvQ29udGFjdFN0cmluZz4NCiAgICA8L0Nvbn
RhY3Q+DQogICAgPENvbnRhY3QgU3RhcnRJbmRleD0iMTIzMiI+DQog
ICAgICA8UGVyc29uIFN0YXJ0SW5kZXg9IjEyMzIiPg0KICAgICAgIC
A8UGVyc29uU3RyaW5nPldvamNpZWNoIERyZXdlazwvUGVyc29uU3Ry
aW5nPg0KICAgICAgPC9QZXJzb24+DQogICAgICA8RW1haWxzPg0KIC
AgICAgICA8RW1haWwgU3RhcnRJbmRleD0iMTI0OSI+DQogICAgICAg
ICAgPEVtYWlsU3RyaW5nPndvamNpZWNoLmRyZXdla0BpbnRlbC5jb2
08L0VtYWlsU3RyaW5nPg0KICAgICAgICA8L0VtYWlsPg0KICAgICAg
PC9FbWFpbHM+DQogICAgICA8Q29udGFjdFN0cmluZz5Xb2pjaWVjaC
BEcmV3ZWsgJmx0O3dvamNpZWNoLmRyZXdla0BpbnRlbC5jb208L0Nv
bnRhY3RTdHJpbmc+DQogICAgPC9Db250YWN0Pg0KICA8L0NvbnRhY3
RzPg0KPC9Db250YWN0U2V0PgEOzwFSZXRyaWV2ZXJPcGVyYXRvciwx
MCwyO1JldHJpZXZlck9wZXJhdG9yLDExLDM7UG9zdERvY1BhcnNlck
9wZXJhdG9yLDEwLDE7UG9zdERvY1BhcnNlck9wZXJhdG9yLDExLDA7
UG9zdFdvcmRCcmVha2VyRGlhZ25vc3RpY09wZXJhdG9yLDEwLDc7UG
9zdFdvcmRCcmVha2VyRGlhZ25vc3RpY09wZXJhdG9yLDExLDA7VHJh
bnNwb3J0V3JpdGVyUHJvZHVjZXIsMjAsMjg=
X-MS-Exchange-Forest-IndexAgent: 1 4643
X-MS-Exchange-Forest-EmailMessageHash: 10B105DC
X-MS-Exchange-Forest-Language: en
X-MS-Exchange-Organization-Processed-By-Journaling: Journal Agent
On Fri, 29 Mar 2024 10:23:20 +0100 Wojciech Drewek wrote:
> Some modules use nonstandard power levels. Adjust ethtool
> module implementation to support new attributes that will allow user
> to change maximum power.
>
> Add three new get attributes:
> ETHTOOL_A_MODULE_MAX_POWER_SET (used for set as well) - currently set
> maximum power in the cage
1) I'd keep the ETHTOOL_A_MODULE_POWER_ prefix, consistently.
2) The _SET makes it sound like an action. Can we go with
ETHTOOL_A_MODULE_POWER_MAX ? Or ETHTOOL_A_MODULE_POWER_LIMIT?
Yes, ETHTOOL_A_MODULE_POWER_LIMIT
ETHTOOL_A_MODULE_POWER_MAX
ETHTOOL_A_MODULE_POWER_MIN
would sound pretty good to me.
> ETHTOOL_A_MODULE_MIN_POWER_ALLOWED - minimum power allowed in the
> cage reported by device
> ETHTOOL_A_MODULE_MAX_POWER_ALLOWED - maximum power allowed in the
> cage reported by device
>
> Add two new set attributes:
> ETHTOOL_A_MODULE_MAX_POWER_SET (used for get as well) - change
> maximum power in the cage to the given value (milliwatts)
> ETHTOOL_A_MODULE_MAX_POWER_RESET - reset maximum power setting to the
> default value
>
> Reviewed-by: Marcin Szycik <marcin.szycik@...ux.intel.com>
> Signed-off-by: Wojciech Drewek <wojciech.drewek@...el.com>
> ---
> include/linux/ethtool.h | 17 +++++--
> include/uapi/linux/ethtool_netlink.h | 4 ++
> net/ethtool/module.c | 74 ++++++++++++++++++++++++++--
> net/ethtool/netlink.h | 2 +-
> 4 files changed, 87 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index f3af6b31c9f1..74ed8997443a 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -510,10 +510,18 @@ struct ethtool_module_eeprom {
> * @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.
> + * @min_pwr_allowed: minimum power allowed in the cage reported by device
> + * @max_pwr_allowed: maximum power allowed in the cage reported by device
> + * @max_pwr_set: maximum power currently set in the cage
> + * @max_pwr_reset: restore default minimum power
> */
> struct ethtool_module_power_params {
> enum ethtool_module_power_mode_policy policy;
> enum ethtool_module_power_mode mode;
> + u32 min_pwr_allowed;
> + u32 max_pwr_allowed;
> + u32 max_pwr_set;
> + u8 max_pwr_reset;
bool ?
> diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
> index 3f89074aa06c..f7cd446b2a83 100644
> --- a/include/uapi/linux/ethtool_netlink.h
> +++ b/include/uapi/linux/ethtool_netlink.h
> @@ -882,6 +882,10 @@ enum {
> ETHTOOL_A_MODULE_HEADER, /* nest - _A_HEADER_* */
> ETHTOOL_A_MODULE_POWER_MODE_POLICY, /* u8 */
> ETHTOOL_A_MODULE_POWER_MODE, /* u8 */
> + ETHTOOL_A_MODULE_MAX_POWER_SET, /* u32 */
> + ETHTOOL_A_MODULE_MIN_POWER_ALLOWED, /* u32 */
> + ETHTOOL_A_MODULE_MAX_POWER_ALLOWED, /* u32 */
> + ETHTOOL_A_MODULE_MAX_POWER_RESET, /* u8 */
flag ?
> @@ -77,6 +86,7 @@ static int module_fill_reply(struct sk_buff *skb,
> const struct ethnl_reply_data *reply_base)
> {
> const struct module_reply_data *data = MODULE_REPDATA(reply_base);
> + u32 temp;
tmp ? temp sounds too much like temperature in context of power
> static int
> ethnl_set_module(struct ethnl_req_info *req_info, struct genl_info *info)
> {
> struct ethtool_module_power_params power = {};
> struct ethtool_module_power_params power_new;
> - const struct ethtool_ops *ops;
> struct net_device *dev = req_info->dev;
> struct nlattr **tb = info->attrs;
> + const struct ethtool_ops *ops;
> int ret;
> + bool mod;
>
> ops = dev->ethtool_ops;
>
> - power_new.policy = nla_get_u8(tb[ETHTOOL_A_MODULE_POWER_MODE_POLICY]);
> ret = ops->get_module_power_cfg(dev, &power, info->extack);
> if (ret < 0)
> return ret;
>
> - if (power_new.policy == power.policy)
> + power_new.max_pwr_set = power.max_pwr_set;
> + power_new.policy = power.policy;
> +
> + ethnl_update_u32(&power_new.max_pwr_set,
> + tb[ETHTOOL_A_MODULE_MAX_POWER_SET], &mod);
> + if (mod) {
I think we can use if (tb[ETHTOOL_A_MODULE_MAX_POWER_SET]) here
Less error prone for future additions.
> + if (power_new.max_pwr_set > power.max_pwr_allowed) {
> + NL_SET_ERR_MSG(info->extack, "Provided value is higher than maximum allowed");
NL_SET_ERR_MSG_ATTR() to point at the bad attribute.
> + return -EINVAL;
ERANGE?
> + } else if (power_new.max_pwr_set < power.min_pwr_allowed) {
> + NL_SET_ERR_MSG(info->extack, "Provided value is lower than minimum allowed");
> + return -EINVAL;
> + }
> + }
> +
> + ethnl_update_policy(&power_new.policy,
> + tb[ETHTOOL_A_MODULE_POWER_MODE_POLICY], &mod);
> + ethnl_update_u8(&power_new.max_pwr_reset,
> + tb[ETHTOOL_A_MODULE_MAX_POWER_RESET], &mod);
I reckon reset should not be allowed if none of the max_pwr values
are set (i.e. most likely driver doesn't support the config)?
> + if (!mod)
> return 0;
>
> + if (power_new.max_pwr_reset && power_new.max_pwr_set) {
Mmm. How is that gonna work? The driver is going to set max_pwr_set
to what's currently configured. So the user is expected to send
ETHTOOL_A_MODULE_MAX_POWER_SET = 0
ETHTOOL_A_MODULE_MAX_POWER_RESET = 1
to reset?
Just:
if (tb[ETHTOOL_A_MODULE_MAX_POWER_RESET] &&
tb[ETHTOOL_A_MODULE_MAX_POWER_SET])
And you can validate this before doing any real work.
> + NL_SET_ERR_MSG(info->extack, "Maximum power set and reset cannot be used at the same time");
> + return 0;
> + }
> +
> ret = ops->set_module_power_cfg(dev, &power_new, info->extack);
> return ret < 0 ? ret : 1;
> }
--
pw-bot: cr
Powered by blists - more mailing lists