From patchwork Fri Jan 29 00:43:23 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tony Nguyen X-Patchwork-Id: 12054827 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,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 9A3FBC433E6 for ; Fri, 29 Jan 2021 00:45:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5FAF964DEB for ; Fri, 29 Jan 2021 00:45:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231678AbhA2Apb (ORCPT ); Thu, 28 Jan 2021 19:45:31 -0500 Received: from mga02.intel.com ([134.134.136.20]:27200 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231547AbhA2AoO (ORCPT ); Thu, 28 Jan 2021 19:44:14 -0500 IronPort-SDR: adRtpQwl+Nfb8WAPhZEEytErGAmhYIF6ER2jTN/yhqdSPBAWK3IIwRpyIVNrasWYGrZtaT+bW8 4cs6b+8Usghg== X-IronPort-AV: E=McAfee;i="6000,8403,9878"; a="167438966" X-IronPort-AV: E=Sophos;i="5.79,384,1602572400"; d="scan'208";a="167438966" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Jan 2021 16:42:52 -0800 IronPort-SDR: UjQ15AgMlrERwf365EVQ2MLE5Eyxqryj+qcTOLWQQxkI/YXrZR7dWebWRhfzs0KlIWnXRdGJIc Q5EaRG67CS+w== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.79,384,1602572400"; d="scan'208";a="430778697" Received: from anguy11-desk2.jf.intel.com ([10.166.244.147]) by orsmga001.jf.intel.com with ESMTP; 28 Jan 2021 16:42:51 -0800 From: Tony Nguyen To: davem@davemloft.net, kuba@kernel.org Cc: Jacob Keller , netdev@vger.kernel.org, sassmann@redhat.com, anthony.l.nguyen@intel.com, Tony Brelinski Subject: [PATCH net-next 06/15] ice: introduce context struct for info report Date: Thu, 28 Jan 2021 16:43:23 -0800 Message-Id: <20210129004332.3004826-7-anthony.l.nguyen@intel.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210129004332.3004826-1-anthony.l.nguyen@intel.com> References: <20210129004332.3004826-1-anthony.l.nguyen@intel.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org From: Jacob Keller The ice driver uses an array of structures which link an info name with a function that formats the associated version data into a string. All existing format functions simply format already captured static data from the driver hw structure. Future changes will introduce format functions for reporting the versions of flash sections stored but not yet applied. This type of version data is not stored as a member of the hw structure. This is because (a) it might not yet exist in the case there is no pending flash update, and (b) even if it does, it might change such as if an update is canceled or replaced by a new update before finalizing. We could simply have each format function gather its own data upon being called. However, in some cases the raw binary version data is a combination of multiple different reported fields. Additionally, the current interface doesn't have a way for the function to indicate that the version doesn't exist. Refactor this function interface to take a new ice_info_ctx structure instead of the buffer pointer and length. This context structure allows for future extensions to pre-gather version data that is stored within the context struct instead of the hw struct. Allocate this context structure initially at the start of ice_devlink_info_get. We use dynamic allocation instead of a local stack variable in order to avoid using too much kernel stack once we extend it with additional data structures. Modify the main loop that drives the info reporting so that the version buffer string is always cleared between each format. Explicitly check that the format function actually filled in a version string of non-zero length. If the string is not provided, simply skip this version without reporting an error. This allows for introducing format functions of versions which may or may not be present, such as the version of a pending update that has not yet been activated. Signed-off-by: Jacob Keller Tested-by: Tony Brelinski Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_devlink.c | 117 ++++++++++++------- 1 file changed, 72 insertions(+), 45 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c index 8af33ef70f76..917fdbb20b7b 100644 --- a/drivers/net/ethernet/intel/ice/ice_devlink.c +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c @@ -6,144 +6,159 @@ #include "ice_devlink.h" #include "ice_fw_update.h" -static void ice_info_get_dsn(struct ice_pf *pf, char *buf, size_t len) +/* context for devlink info version reporting */ +struct ice_info_ctx { + char buf[128]; +}; + +/* The following functions are used to format specific strings for various + * devlink info versions. The ctx parameter is used to provide the storage + * buffer, as well as any ancillary information calculated when the info + * request was made. + * + * If a version does not exist, for example a "stored" version that does not + * exist because no update is pending, the function should leave the buffer in + * the ctx structure empty and return 0. + */ + +static void ice_info_get_dsn(struct ice_pf *pf, struct ice_info_ctx *ctx) { u8 dsn[8]; /* Copy the DSN into an array in Big Endian format */ put_unaligned_be64(pci_get_dsn(pf->pdev), dsn); - snprintf(buf, len, "%8phD", dsn); + snprintf(ctx->buf, sizeof(ctx->buf), "%8phD", dsn); } -static int ice_info_pba(struct ice_pf *pf, char *buf, size_t len) +static int ice_info_pba(struct ice_pf *pf, struct ice_info_ctx *ctx) { struct ice_hw *hw = &pf->hw; enum ice_status status; - status = ice_read_pba_string(hw, (u8 *)buf, len); + status = ice_read_pba_string(hw, (u8 *)ctx->buf, sizeof(ctx->buf)); if (status) return -EIO; return 0; } -static int ice_info_fw_mgmt(struct ice_pf *pf, char *buf, size_t len) +static int ice_info_fw_mgmt(struct ice_pf *pf, struct ice_info_ctx *ctx) { struct ice_hw *hw = &pf->hw; - snprintf(buf, len, "%u.%u.%u", hw->fw_maj_ver, hw->fw_min_ver, + snprintf(ctx->buf, sizeof(ctx->buf), "%u.%u.%u", hw->fw_maj_ver, hw->fw_min_ver, hw->fw_patch); return 0; } -static int ice_info_fw_api(struct ice_pf *pf, char *buf, size_t len) +static int ice_info_fw_api(struct ice_pf *pf, struct ice_info_ctx *ctx) { struct ice_hw *hw = &pf->hw; - snprintf(buf, len, "%u.%u", hw->api_maj_ver, hw->api_min_ver); + snprintf(ctx->buf, sizeof(ctx->buf), "%u.%u", hw->api_maj_ver, hw->api_min_ver); return 0; } -static int ice_info_fw_build(struct ice_pf *pf, char *buf, size_t len) +static int ice_info_fw_build(struct ice_pf *pf, struct ice_info_ctx *ctx) { struct ice_hw *hw = &pf->hw; - snprintf(buf, len, "0x%08x", hw->fw_build); + snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", hw->fw_build); return 0; } -static int ice_info_fw_srev(struct ice_pf *pf, char *buf, size_t len) +static int ice_info_fw_srev(struct ice_pf *pf, struct ice_info_ctx *ctx) { struct ice_nvm_info *nvm = &pf->hw.flash.nvm; - snprintf(buf, len, "%u", nvm->srev); + snprintf(ctx->buf, sizeof(ctx->buf), "%u", nvm->srev); return 0; } -static int ice_info_orom_ver(struct ice_pf *pf, char *buf, size_t len) +static int ice_info_orom_ver(struct ice_pf *pf, struct ice_info_ctx *ctx) { struct ice_orom_info *orom = &pf->hw.flash.orom; - snprintf(buf, len, "%u.%u.%u", orom->major, orom->build, orom->patch); + snprintf(ctx->buf, sizeof(ctx->buf), "%u.%u.%u", orom->major, orom->build, orom->patch); return 0; } -static int ice_info_orom_srev(struct ice_pf *pf, char *buf, size_t len) +static int ice_info_orom_srev(struct ice_pf *pf, struct ice_info_ctx *ctx) { struct ice_orom_info *orom = &pf->hw.flash.orom; - snprintf(buf, len, "%u", orom->srev); + snprintf(ctx->buf, sizeof(ctx->buf), "%u", orom->srev); return 0; } -static int ice_info_nvm_ver(struct ice_pf *pf, char *buf, size_t len) +static int ice_info_nvm_ver(struct ice_pf *pf, struct ice_info_ctx *ctx) { struct ice_nvm_info *nvm = &pf->hw.flash.nvm; - snprintf(buf, len, "%x.%02x", nvm->major, nvm->minor); + snprintf(ctx->buf, sizeof(ctx->buf), "%x.%02x", nvm->major, nvm->minor); return 0; } -static int ice_info_eetrack(struct ice_pf *pf, char *buf, size_t len) +static int ice_info_eetrack(struct ice_pf *pf, struct ice_info_ctx *ctx) { struct ice_nvm_info *nvm = &pf->hw.flash.nvm; - snprintf(buf, len, "0x%08x", nvm->eetrack); + snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", nvm->eetrack); return 0; } -static int ice_info_ddp_pkg_name(struct ice_pf *pf, char *buf, size_t len) +static int ice_info_ddp_pkg_name(struct ice_pf *pf, struct ice_info_ctx *ctx) { struct ice_hw *hw = &pf->hw; - snprintf(buf, len, "%s", hw->active_pkg_name); + snprintf(ctx->buf, sizeof(ctx->buf), "%s", hw->active_pkg_name); return 0; } -static int ice_info_ddp_pkg_version(struct ice_pf *pf, char *buf, size_t len) +static int ice_info_ddp_pkg_version(struct ice_pf *pf, struct ice_info_ctx *ctx) { struct ice_pkg_ver *pkg = &pf->hw.active_pkg_ver; - snprintf(buf, len, "%u.%u.%u.%u", pkg->major, pkg->minor, pkg->update, + snprintf(ctx->buf, sizeof(ctx->buf), "%u.%u.%u.%u", pkg->major, pkg->minor, pkg->update, pkg->draft); return 0; } -static int ice_info_ddp_pkg_bundle_id(struct ice_pf *pf, char *buf, size_t len) +static int ice_info_ddp_pkg_bundle_id(struct ice_pf *pf, struct ice_info_ctx *ctx) { - snprintf(buf, len, "0x%08x", pf->hw.active_track_id); + snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", pf->hw.active_track_id); return 0; } -static int ice_info_netlist_ver(struct ice_pf *pf, char *buf, size_t len) +static int ice_info_netlist_ver(struct ice_pf *pf, struct ice_info_ctx *ctx) { struct ice_netlist_info *netlist = &pf->hw.flash.netlist; /* The netlist version fields are BCD formatted */ - snprintf(buf, len, "%x.%x.%x-%x.%x.%x", netlist->major, netlist->minor, + snprintf(ctx->buf, sizeof(ctx->buf), "%x.%x.%x-%x.%x.%x", netlist->major, netlist->minor, netlist->type >> 16, netlist->type & 0xFFFF, netlist->rev, netlist->cust_ver); return 0; } -static int ice_info_netlist_build(struct ice_pf *pf, char *buf, size_t len) +static int ice_info_netlist_build(struct ice_pf *pf, struct ice_info_ctx *ctx) { struct ice_netlist_info *netlist = &pf->hw.flash.netlist; - snprintf(buf, len, "0x%08x", netlist->hash); + snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", netlist->hash); return 0; } @@ -160,7 +175,7 @@ enum ice_version_type { static const struct ice_devlink_version { enum ice_version_type type; const char *key; - int (*getter)(struct ice_pf *pf, char *buf, size_t len); + int (*getter)(struct ice_pf *pf, struct ice_info_ctx *ctx); } ice_devlink_versions[] = { fixed(DEVLINK_INFO_VERSION_GENERIC_BOARD_ID, ice_info_pba), running(DEVLINK_INFO_VERSION_GENERIC_FW_MGMT, ice_info_fw_mgmt), @@ -194,60 +209,72 @@ static int ice_devlink_info_get(struct devlink *devlink, struct netlink_ext_ack *extack) { struct ice_pf *pf = devlink_priv(devlink); - char buf[100]; + struct ice_info_ctx *ctx; size_t i; int err; + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); + if (!ctx) + return -ENOMEM; + err = devlink_info_driver_name_put(req, KBUILD_MODNAME); if (err) { NL_SET_ERR_MSG_MOD(extack, "Unable to set driver name"); - return err; + goto out_free_ctx; } - ice_info_get_dsn(pf, buf, sizeof(buf)); + ice_info_get_dsn(pf, ctx); - err = devlink_info_serial_number_put(req, buf); + err = devlink_info_serial_number_put(req, ctx->buf); if (err) { NL_SET_ERR_MSG_MOD(extack, "Unable to set serial number"); - return err; + goto out_free_ctx; } for (i = 0; i < ARRAY_SIZE(ice_devlink_versions); i++) { enum ice_version_type type = ice_devlink_versions[i].type; const char *key = ice_devlink_versions[i].key; - err = ice_devlink_versions[i].getter(pf, buf, sizeof(buf)); + memset(ctx->buf, 0, sizeof(ctx->buf)); + + err = ice_devlink_versions[i].getter(pf, ctx); if (err) { NL_SET_ERR_MSG_MOD(extack, "Unable to obtain version info"); - return err; + goto out_free_ctx; } + /* Do not report missing versions */ + if (ctx->buf[0] == '\0') + continue; + switch (type) { case ICE_VERSION_FIXED: - err = devlink_info_version_fixed_put(req, key, buf); + err = devlink_info_version_fixed_put(req, key, ctx->buf); if (err) { NL_SET_ERR_MSG_MOD(extack, "Unable to set fixed version"); - return err; + goto out_free_ctx; } break; case ICE_VERSION_RUNNING: - err = devlink_info_version_running_put(req, key, buf); + err = devlink_info_version_running_put(req, key, ctx->buf); if (err) { NL_SET_ERR_MSG_MOD(extack, "Unable to set running version"); - return err; + goto out_free_ctx; } break; case ICE_VERSION_STORED: - err = devlink_info_version_stored_put(req, key, buf); + err = devlink_info_version_stored_put(req, key, ctx->buf); if (err) { NL_SET_ERR_MSG_MOD(extack, "Unable to set stored version"); - return err; + goto out_free_ctx; } break; } } - return 0; +out_free_ctx: + kfree(ctx); + return err; } enum ice_devlink_param_id {