From patchwork Fri Nov 20 13:40:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Moshe Shemesh X-Patchwork-Id: 11920327 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,URIBL_BLOCKED, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 12D09C5519F for ; Fri, 20 Nov 2020 13:41:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C445722272 for ; Fri, 20 Nov 2020 13:41:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727778AbgKTNkv (ORCPT ); Fri, 20 Nov 2020 08:40:51 -0500 Received: from mail-il-dmz.mellanox.com ([193.47.165.129]:60955 "EHLO mellanox.co.il" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726541AbgKTNkv (ORCPT ); Fri, 20 Nov 2020 08:40:51 -0500 Received: from Internal Mail-Server by MTLPINE1 (envelope-from moshe@mellanox.com) with SMTP; 20 Nov 2020 15:40:46 +0200 Received: from vnc1.mtl.labs.mlnx (vnc1.mtl.labs.mlnx [10.7.2.1]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id 0AKDekKo029539; Fri, 20 Nov 2020 15:40:46 +0200 Received: from vnc1.mtl.labs.mlnx (localhost [127.0.0.1]) by vnc1.mtl.labs.mlnx (8.14.4/8.14.4) with ESMTP id 0AKDejTi006178; Fri, 20 Nov 2020 15:40:45 +0200 Received: (from moshe@localhost) by vnc1.mtl.labs.mlnx (8.14.4/8.14.4/Submit) id 0AKDeh0O006172; Fri, 20 Nov 2020 15:40:43 +0200 From: Moshe Shemesh To: "David S. Miller" , Jakub Kicinski Cc: Jiri Pirko , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Moshe Shemesh Subject: [PATCH net] devlink: Fix reload stats structure Date: Fri, 20 Nov 2020 15:40:37 +0200 Message-Id: <1605879637-6114-1-git-send-email-moshe@mellanox.com> X-Mailer: git-send-email 1.8.4.3 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org Fix reload stats structure exposed to the user. Change stats structure hierarchy to have the reload action as a parent of the stat entry and then stat entry includes value per limit. This will also help to avoid string concatenation on iproute2 output. Reload stats structure before this fix: "stats": { "reload": { "driver_reinit": 2, "fw_activate": 1, "fw_activate_no_reset": 0 } } After this fix: "stats": { "reload": { "driver_reinit": { "unspecified": 2 }, "fw_activate": { "unspecified": 1, "no_reset": 0 } } Fixes: a254c264267e ("devlink: Add reload stats") Signed-off-by: Moshe Shemesh Reviewed-by: Jiri Pirko --- include/uapi/linux/devlink.h | 2 ++ net/core/devlink.c | 48 +++++++++++++++++++++++------------- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h index 0113bc4db9f5..5203f54a2be1 100644 --- a/include/uapi/linux/devlink.h +++ b/include/uapi/linux/devlink.h @@ -526,6 +526,8 @@ enum devlink_attr { DEVLINK_ATTR_RELOAD_STATS_LIMIT, /* u8 */ DEVLINK_ATTR_RELOAD_STATS_VALUE, /* u32 */ DEVLINK_ATTR_REMOTE_RELOAD_STATS, /* nested */ + DEVLINK_ATTR_RELOAD_ACTION_INFO, /* nested */ + DEVLINK_ATTR_RELOAD_ACTION_STATS, /* nested */ /* add new attributes above here, update the policy in devlink.c */ diff --git a/net/core/devlink.c b/net/core/devlink.c index ab4b1368904f..34d38abd74ee 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -517,8 +517,7 @@ devlink_reload_limit_is_supported(struct devlink *devlink, enum devlink_reload_l return test_bit(limit, &devlink->ops->reload_limits); } -static int devlink_reload_stat_put(struct sk_buff *msg, enum devlink_reload_action action, - enum devlink_reload_limit limit, u32 value) +static int devlink_reload_stat_put(struct sk_buff *msg, enum devlink_reload_limit limit, u32 value) { struct nlattr *reload_stats_entry; @@ -526,8 +525,7 @@ static int devlink_reload_stat_put(struct sk_buff *msg, enum devlink_reload_acti if (!reload_stats_entry) return -EMSGSIZE; - if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_ACTION, action) || - nla_put_u8(msg, DEVLINK_ATTR_RELOAD_STATS_LIMIT, limit) || + if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_STATS_LIMIT, limit) || nla_put_u32(msg, DEVLINK_ATTR_RELOAD_STATS_VALUE, value)) goto nla_put_failure; nla_nest_end(msg, reload_stats_entry); @@ -540,7 +538,7 @@ static int devlink_reload_stat_put(struct sk_buff *msg, enum devlink_reload_acti static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink, bool is_remote) { - struct nlattr *reload_stats_attr; + struct nlattr *reload_stats_attr, *action_info_attr, *action_stats_attr; int i, j, stat_idx; u32 value; @@ -552,17 +550,27 @@ static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink if (!reload_stats_attr) return -EMSGSIZE; - for (j = 0; j <= DEVLINK_RELOAD_LIMIT_MAX; j++) { - /* Remote stats are shown even if not locally supported. Stats - * of actions with unspecified limit are shown though drivers - * don't need to register unspecified limit. - */ - if (!is_remote && j != DEVLINK_RELOAD_LIMIT_UNSPEC && - !devlink_reload_limit_is_supported(devlink, j)) + for (i = 0; i <= DEVLINK_RELOAD_ACTION_MAX; i++) { + if ((!is_remote && !devlink_reload_action_is_supported(devlink, i)) || + i == DEVLINK_RELOAD_ACTION_UNSPEC) continue; - for (i = 0; i <= DEVLINK_RELOAD_ACTION_MAX; i++) { - if ((!is_remote && !devlink_reload_action_is_supported(devlink, i)) || - i == DEVLINK_RELOAD_ACTION_UNSPEC || + action_info_attr = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_ACTION_INFO); + if (!action_info_attr) + goto nla_put_failure; + + if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_ACTION, i)) + goto action_info_nest_cancel; + action_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_ACTION_STATS); + if (!action_stats_attr) + goto action_info_nest_cancel; + + for (j = 0; j <= DEVLINK_RELOAD_LIMIT_MAX; j++) { + /* Remote stats are shown even if not locally supported. Stats + * of actions with unspecified limit are shown though drivers + * don't need to register unspecified limit. + */ + if ((!is_remote && j != DEVLINK_RELOAD_LIMIT_UNSPEC && + !devlink_reload_limit_is_supported(devlink, j)) || devlink_reload_combination_is_invalid(i, j)) continue; @@ -571,13 +579,19 @@ static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink value = devlink->stats.reload_stats[stat_idx]; else value = devlink->stats.remote_reload_stats[stat_idx]; - if (devlink_reload_stat_put(msg, i, j, value)) - goto nla_put_failure; + if (devlink_reload_stat_put(msg, j, value)) + goto action_stats_nest_cancel; } + nla_nest_end(msg, action_stats_attr); + nla_nest_end(msg, action_info_attr); } nla_nest_end(msg, reload_stats_attr); return 0; +action_stats_nest_cancel: + nla_nest_cancel(msg, action_stats_attr); +action_info_nest_cancel: + nla_nest_cancel(msg, action_info_attr); nla_put_failure: nla_nest_cancel(msg, reload_stats_attr); return -EMSGSIZE;