From patchwork Thu Mar 26 18:37:08 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Keller, Jacob E" X-Patchwork-Id: 1262179 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=intel.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 48pDHT55M1z9sSM for ; Fri, 27 Mar 2020 05:37:25 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728525AbgCZShY (ORCPT ); Thu, 26 Mar 2020 14:37:24 -0400 Received: from mga09.intel.com ([134.134.136.24]:43770 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727446AbgCZShX (ORCPT ); Thu, 26 Mar 2020 14:37:23 -0400 IronPort-SDR: 4juFxP8fI87TRRmGeQ2ujKhrh2lx46n5IZ+uQfvy9a0C1FgsSPC8gdaeID8WJ539/sL7kDT5e5 bO8j0EkQZJEQ== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Mar 2020 11:37:22 -0700 IronPort-SDR: u10JPTukHLk0itj6lMQDwFxMejRwi2BNRmJkg66MJSMCwHCWcJyKHxpihVhgyw/ZtDykdWRMY3 ek4/PIFns/pw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,309,1580803200"; d="scan'208";a="358241606" Received: from jekeller-desk.amr.corp.intel.com ([10.166.241.33]) by fmsmga001.fm.intel.com with ESMTP; 26 Mar 2020 11:37:22 -0700 From: Jacob Keller To: netdev@vger.kernel.org Cc: Jakub Kicinski , Jiri Pirko , Jacob Keller , Jiri Pirko Subject: [PATCH net-next v3 01/11] devlink: prepare to support region operations Date: Thu, 26 Mar 2020 11:37:08 -0700 Message-Id: <20200326183718.2384349-2-jacob.e.keller@intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200326183718.2384349-1-jacob.e.keller@intel.com> References: <20200326183718.2384349-1-jacob.e.keller@intel.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Modify the devlink region code in preparation for adding new operations on regions. Create a devlink_region_ops structure, and move the name pointer from within the devlink_region structure into the ops structure (similar to the devlink_health_reporter_ops). This prepares the regions to enable support of additional operations in the future such as requesting snapshots, or accessing the region directly without a snapshot. In order to re-use the constant strings in the mlx4 driver their declaration must be changed to 'const char * const' to ensure the compiler realizes that both the data and the pointer cannot change. Signed-off-by: Jacob Keller Reviewed-by: Jakub Kicinski Reviewed-by: Jiri Pirko --- Changes since RFC * Picked up Jiri's Reviewed-by drivers/net/ethernet/mellanox/mlx4/crdump.c | 16 +++++++++++---- drivers/net/netdevsim/dev.c | 6 +++++- include/net/devlink.h | 16 +++++++++++---- net/core/devlink.c | 22 ++++++++++----------- 4 files changed, 40 insertions(+), 20 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx4/crdump.c b/drivers/net/ethernet/mellanox/mlx4/crdump.c index 64ed725aec28..cc2bf596c74b 100644 --- a/drivers/net/ethernet/mellanox/mlx4/crdump.c +++ b/drivers/net/ethernet/mellanox/mlx4/crdump.c @@ -38,8 +38,16 @@ #define CR_ENABLE_BIT_OFFSET 0xF3F04 #define MAX_NUM_OF_DUMPS_TO_STORE (8) -static const char *region_cr_space_str = "cr-space"; -static const char *region_fw_health_str = "fw-health"; +static const char * const region_cr_space_str = "cr-space"; +static const char * const region_fw_health_str = "fw-health"; + +static const struct devlink_region_ops region_cr_space_ops = { + .name = region_cr_space_str, +}; + +static const struct devlink_region_ops region_fw_health_ops = { + .name = region_fw_health_str, +}; /* Set to true in case cr enable bit was set to true before crdump */ static bool crdump_enbale_bit_set; @@ -205,7 +213,7 @@ int mlx4_crdump_init(struct mlx4_dev *dev) /* Create cr-space region */ crdump->region_crspace = devlink_region_create(devlink, - region_cr_space_str, + ®ion_cr_space_ops, MAX_NUM_OF_DUMPS_TO_STORE, pci_resource_len(pdev, 0)); if (IS_ERR(crdump->region_crspace)) @@ -216,7 +224,7 @@ int mlx4_crdump_init(struct mlx4_dev *dev) /* Create fw-health region */ crdump->region_fw_health = devlink_region_create(devlink, - region_fw_health_str, + ®ion_fw_health_ops, MAX_NUM_OF_DUMPS_TO_STORE, HEALTH_BUFFER_SIZE); if (IS_ERR(crdump->region_fw_health)) diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c index 7bfd0622cef1..47a8f8c570c4 100644 --- a/drivers/net/netdevsim/dev.c +++ b/drivers/net/netdevsim/dev.c @@ -340,11 +340,15 @@ static void nsim_devlink_param_load_driverinit_values(struct devlink *devlink) #define NSIM_DEV_DUMMY_REGION_SNAPSHOT_MAX 16 +static const struct devlink_region_ops dummy_region_ops = { + .name = "dummy", +}; + static int nsim_dev_dummy_region_init(struct nsim_dev *nsim_dev, struct devlink *devlink) { nsim_dev->dummy_region = - devlink_region_create(devlink, "dummy", + devlink_region_create(devlink, &dummy_region_ops, NSIM_DEV_DUMMY_REGION_SNAPSHOT_MAX, NSIM_DEV_DUMMY_REGION_SIZE); return PTR_ERR_OR_ZERO(nsim_dev->dummy_region); diff --git a/include/net/devlink.h b/include/net/devlink.h index 37230e23b5b0..85db5dd5184d 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -498,6 +498,14 @@ struct devlink_info_req; typedef void devlink_snapshot_data_dest_t(const void *data); +/** + * struct devlink_region_ops - Region operations + * @name: region name + */ +struct devlink_region_ops { + const char *name; +}; + struct devlink_fmsg; struct devlink_health_reporter; @@ -963,10 +971,10 @@ void devlink_port_param_value_changed(struct devlink_port *devlink_port, u32 param_id); void devlink_param_value_str_fill(union devlink_param_value *dst_val, const char *src); -struct devlink_region *devlink_region_create(struct devlink *devlink, - const char *region_name, - u32 region_max_snapshots, - u64 region_size); +struct devlink_region * +devlink_region_create(struct devlink *devlink, + const struct devlink_region_ops *ops, + u32 region_max_snapshots, u64 region_size); void devlink_region_destroy(struct devlink_region *region); u32 devlink_region_snapshot_id_get(struct devlink *devlink); int devlink_region_snapshot_create(struct devlink_region *region, diff --git a/net/core/devlink.c b/net/core/devlink.c index 73bb8fbe3393..ca5362530567 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -344,7 +344,7 @@ devlink_sb_tc_index_get_from_info(struct devlink_sb *devlink_sb, struct devlink_region { struct devlink *devlink; struct list_head list; - const char *name; + const struct devlink_region_ops *ops; struct list_head snapshot_list; u32 max_snapshots; u32 cur_snapshots; @@ -365,7 +365,7 @@ devlink_region_get_by_name(struct devlink *devlink, const char *region_name) struct devlink_region *region; list_for_each_entry(region, &devlink->region_list, list) - if (!strcmp(region->name, region_name)) + if (!strcmp(region->ops->name, region_name)) return region; return NULL; @@ -3695,7 +3695,7 @@ static int devlink_nl_region_fill(struct sk_buff *msg, struct devlink *devlink, if (err) goto nla_put_failure; - err = nla_put_string(msg, DEVLINK_ATTR_REGION_NAME, region->name); + err = nla_put_string(msg, DEVLINK_ATTR_REGION_NAME, region->ops->name); if (err) goto nla_put_failure; @@ -3741,7 +3741,7 @@ static void devlink_nl_region_notify(struct devlink_region *region, goto out_cancel_msg; err = nla_put_string(msg, DEVLINK_ATTR_REGION_NAME, - region->name); + region->ops->name); if (err) goto out_cancel_msg; @@ -7647,21 +7647,21 @@ EXPORT_SYMBOL_GPL(devlink_param_value_str_fill); * devlink_region_create - create a new address region * * @devlink: devlink - * @region_name: region name + * @ops: region operations and name * @region_max_snapshots: Maximum supported number of snapshots for region * @region_size: size of region */ -struct devlink_region *devlink_region_create(struct devlink *devlink, - const char *region_name, - u32 region_max_snapshots, - u64 region_size) +struct devlink_region * +devlink_region_create(struct devlink *devlink, + const struct devlink_region_ops *ops, + u32 region_max_snapshots, u64 region_size) { struct devlink_region *region; int err = 0; mutex_lock(&devlink->lock); - if (devlink_region_get_by_name(devlink, region_name)) { + if (devlink_region_get_by_name(devlink, ops->name)) { err = -EEXIST; goto unlock; } @@ -7674,7 +7674,7 @@ struct devlink_region *devlink_region_create(struct devlink *devlink, region->devlink = devlink; region->max_snapshots = region_max_snapshots; - region->name = region_name; + region->ops = ops; region->size = region_size; INIT_LIST_HEAD(®ion->snapshot_list); list_add_tail(®ion->list, &devlink->region_list); From patchwork Thu Mar 26 18:37:09 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Keller, Jacob E" X-Patchwork-Id: 1262180 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=intel.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 48pDHV1cwdz9sRY for ; Fri, 27 Mar 2020 05:37:26 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728502AbgCZShY (ORCPT ); Thu, 26 Mar 2020 14:37:24 -0400 Received: from mga09.intel.com ([134.134.136.24]:43770 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726359AbgCZShX (ORCPT ); Thu, 26 Mar 2020 14:37:23 -0400 IronPort-SDR: uXqXmOEzi0tapyf2QftQ/QZ6Ml3J/OJNye08H3ivDRjISijrkkqfm7zxZMgv2c0Ul8f1YOnWlj qivv5orPBX9Q== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Mar 2020 11:37:22 -0700 IronPort-SDR: trc47C/Gh71Ie5aHYfl//y018J19AihjbkGbw7keRjpQ/1Cx4Omj7BW2iVRcI3XcSH1dt2hvSH yrlAdH1iUZdg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,309,1580803200"; d="scan'208";a="358241609" Received: from jekeller-desk.amr.corp.intel.com ([10.166.241.33]) by fmsmga001.fm.intel.com with ESMTP; 26 Mar 2020 11:37:22 -0700 From: Jacob Keller To: netdev@vger.kernel.org Cc: Jakub Kicinski , Jiri Pirko , Jacob Keller , Jiri Pirko Subject: [PATCH net-next v3 02/11] devlink: convert snapshot destructor callback to region op Date: Thu, 26 Mar 2020 11:37:09 -0700 Message-Id: <20200326183718.2384349-3-jacob.e.keller@intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200326183718.2384349-1-jacob.e.keller@intel.com> References: <20200326183718.2384349-1-jacob.e.keller@intel.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org It does not makes sense that two snapshots for a given region would use different destructors. Simplify snapshot creation by adding a .destructor op for regions. This operation will replace the data_destructor for the snapshot creation, and makes snapshot creation easier. Noticed-by: Jakub Kicinski Signed-off-by: Jacob Keller Reviewed-by: Jiri Pirko --- Changes since RFC: * Picked up Jiri's Reviewed-by drivers/net/ethernet/mellanox/mlx4/crdump.c | 6 ++++-- drivers/net/netdevsim/dev.c | 3 ++- include/net/devlink.h | 7 +++---- net/core/devlink.c | 11 +++++------ 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx4/crdump.c b/drivers/net/ethernet/mellanox/mlx4/crdump.c index cc2bf596c74b..c3f90c0f9554 100644 --- a/drivers/net/ethernet/mellanox/mlx4/crdump.c +++ b/drivers/net/ethernet/mellanox/mlx4/crdump.c @@ -43,10 +43,12 @@ static const char * const region_fw_health_str = "fw-health"; static const struct devlink_region_ops region_cr_space_ops = { .name = region_cr_space_str, + .destructor = &kvfree, }; static const struct devlink_region_ops region_fw_health_ops = { .name = region_fw_health_str, + .destructor = &kvfree, }; /* Set to true in case cr enable bit was set to true before crdump */ @@ -107,7 +109,7 @@ static void mlx4_crdump_collect_crspace(struct mlx4_dev *dev, readl(cr_space + offset); err = devlink_region_snapshot_create(crdump->region_crspace, - crspace_data, id, &kvfree); + crspace_data, id); if (err) { kvfree(crspace_data); mlx4_warn(dev, "crdump: devlink create %s snapshot id %d err %d\n", @@ -146,7 +148,7 @@ static void mlx4_crdump_collect_fw_health(struct mlx4_dev *dev, readl(health_buf_start + offset); err = devlink_region_snapshot_create(crdump->region_fw_health, - health_data, id, &kvfree); + health_data, id); if (err) { kvfree(health_data); mlx4_warn(dev, "crdump: devlink create %s snapshot id %d err %d\n", diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c index 47a8f8c570c4..f7621ccb7b88 100644 --- a/drivers/net/netdevsim/dev.c +++ b/drivers/net/netdevsim/dev.c @@ -56,7 +56,7 @@ static ssize_t nsim_dev_take_snapshot_write(struct file *file, id = devlink_region_snapshot_id_get(priv_to_devlink(nsim_dev)); err = devlink_region_snapshot_create(nsim_dev->dummy_region, - dummy_data, id, kfree); + dummy_data, id); if (err) { pr_err("Failed to create region snapshot\n"); kfree(dummy_data); @@ -342,6 +342,7 @@ static void nsim_devlink_param_load_driverinit_values(struct devlink *devlink) static const struct devlink_region_ops dummy_region_ops = { .name = "dummy", + .destructor = &kfree, }; static int nsim_dev_dummy_region_init(struct nsim_dev *nsim_dev, diff --git a/include/net/devlink.h b/include/net/devlink.h index 85db5dd5184d..8869ad75b965 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -496,14 +496,14 @@ enum devlink_param_generic_id { struct devlink_region; struct devlink_info_req; -typedef void devlink_snapshot_data_dest_t(const void *data); - /** * struct devlink_region_ops - Region operations * @name: region name + * @destructor: callback used to free snapshot memory when deleting */ struct devlink_region_ops { const char *name; + void (*destructor)(const void *data); }; struct devlink_fmsg; @@ -978,8 +978,7 @@ devlink_region_create(struct devlink *devlink, void devlink_region_destroy(struct devlink_region *region); u32 devlink_region_snapshot_id_get(struct devlink *devlink); int devlink_region_snapshot_create(struct devlink_region *region, - u8 *data, u32 snapshot_id, - devlink_snapshot_data_dest_t *data_destructor); + u8 *data, u32 snapshot_id); int devlink_info_serial_number_put(struct devlink_info_req *req, const char *sn); int devlink_info_driver_name_put(struct devlink_info_req *req, diff --git a/net/core/devlink.c b/net/core/devlink.c index ca5362530567..84d74fbcff62 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -354,7 +354,6 @@ struct devlink_region { struct devlink_snapshot { struct list_head list; struct devlink_region *region; - devlink_snapshot_data_dest_t *data_destructor; u8 *data; u32 id; }; @@ -3775,7 +3774,7 @@ static void devlink_region_snapshot_del(struct devlink_region *region, devlink_nl_region_notify(region, snapshot, DEVLINK_CMD_REGION_DEL); region->cur_snapshots--; list_del(&snapshot->list); - (*snapshot->data_destructor)(snapshot->data); + region->ops->destructor(snapshot->data); kfree(snapshot); } @@ -7659,6 +7658,9 @@ devlink_region_create(struct devlink *devlink, struct devlink_region *region; int err = 0; + if (WARN_ON(!ops) || WARN_ON(!ops->destructor)) + return ERR_PTR(-EINVAL); + mutex_lock(&devlink->lock); if (devlink_region_get_by_name(devlink, ops->name)) { @@ -7745,11 +7747,9 @@ EXPORT_SYMBOL_GPL(devlink_region_snapshot_id_get); * @region: devlink region of the snapshot * @data: snapshot data * @snapshot_id: snapshot id to be created - * @data_destructor: pointer to destructor function to free data */ int devlink_region_snapshot_create(struct devlink_region *region, - u8 *data, u32 snapshot_id, - devlink_snapshot_data_dest_t *data_destructor) + u8 *data, u32 snapshot_id) { struct devlink *devlink = region->devlink; struct devlink_snapshot *snapshot; @@ -7777,7 +7777,6 @@ int devlink_region_snapshot_create(struct devlink_region *region, snapshot->id = snapshot_id; snapshot->region = region; snapshot->data = data; - snapshot->data_destructor = data_destructor; list_add_tail(&snapshot->list, ®ion->snapshot_list); From patchwork Thu Mar 26 18:37:10 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Keller, Jacob E" X-Patchwork-Id: 1262181 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=intel.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 48pDHh3NYZz9sR4 for ; Fri, 27 Mar 2020 05:37:36 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728236AbgCZShf (ORCPT ); Thu, 26 Mar 2020 14:37:35 -0400 Received: from mga09.intel.com ([134.134.136.24]:43777 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726163AbgCZShf (ORCPT ); Thu, 26 Mar 2020 14:37:35 -0400 IronPort-SDR: TY4dKEntE936j500B+9EV6gWUDa75HgaGdKjVxScSVXkpSnf0FXCoYFWPuxxlFsokVU9q3CZrO pMEO+qh1fhXA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Mar 2020 11:37:22 -0700 IronPort-SDR: W5XN3ncv0P1yypSj0E0QiOI6qFEAJrYYHqWbrYfyqpxo5QnWjOSwpV7b3vIVjPbdHVj9t7kn+H wvTc1NbjFdbg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,309,1580803200"; d="scan'208";a="358241613" Received: from jekeller-desk.amr.corp.intel.com ([10.166.241.33]) by fmsmga001.fm.intel.com with ESMTP; 26 Mar 2020 11:37:22 -0700 From: Jacob Keller To: netdev@vger.kernel.org Cc: Jakub Kicinski , Jiri Pirko , Jacob Keller , Jiri Pirko Subject: [PATCH net-next v3 03/11] devlink: trivial: fix tab in function documentation Date: Thu, 26 Mar 2020 11:37:10 -0700 Message-Id: <20200326183718.2384349-4-jacob.e.keller@intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200326183718.2384349-1-jacob.e.keller@intel.com> References: <20200326183718.2384349-1-jacob.e.keller@intel.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org The function documentation comment for devlink_region_snapshot_create included a literal tab character between 'future analyses' that was difficult to spot as it happened to only display as one space wide. Fix the comment to use a space here instead of a stray tab appearing in the middle of a sentence. Signed-off-by: Jacob Keller Reviewed-by: Jiri Pirko --- Changes since RFC: * Picked up Jiri's Reviewed-by net/core/devlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/core/devlink.c b/net/core/devlink.c index 84d74fbcff62..73e66a779c13 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -7740,7 +7740,7 @@ EXPORT_SYMBOL_GPL(devlink_region_snapshot_id_get); * devlink_region_snapshot_create - create a new snapshot * This will add a new snapshot of a region. The snapshot * will be stored on the region struct and can be accessed - * from devlink. This is useful for future analyses of snapshots. + * from devlink. This is useful for future analyses of snapshots. * Multiple snapshots can be created on a region. * The @snapshot_id should be obtained using the getter function. * From patchwork Thu Mar 26 18:37:11 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Keller, Jacob E" X-Patchwork-Id: 1262182 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=intel.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 48pDHj2BPZz9sRY for ; Fri, 27 Mar 2020 05:37:37 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728335AbgCZShg (ORCPT ); Thu, 26 Mar 2020 14:37:36 -0400 Received: from mga09.intel.com ([134.134.136.24]:43777 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727446AbgCZShf (ORCPT ); Thu, 26 Mar 2020 14:37:35 -0400 IronPort-SDR: u6REnX8sVJrjlUz4T4t3r/YkAVqxFRjGi4ZA0WIa2VCNst2L74MGapUoQuj5eLxDkRzQEkkh/G zls4EiS7p66Q== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Mar 2020 11:37:23 -0700 IronPort-SDR: OTwNpUc2IEfdggVBe0B7IB6ZiRZ3tPc7twfTxPXpj40NBjPv4QT25j1M4mBcz9DnBiMW/d6JaP YrKS/v93xliQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,309,1580803200"; d="scan'208";a="358241616" Received: from jekeller-desk.amr.corp.intel.com ([10.166.241.33]) by fmsmga001.fm.intel.com with ESMTP; 26 Mar 2020 11:37:22 -0700 From: Jacob Keller To: netdev@vger.kernel.org Cc: Jakub Kicinski , Jiri Pirko , Jacob Keller , Jiri Pirko Subject: [PATCH net-next v3 04/11] devlink: add function to take snapshot while locked Date: Thu, 26 Mar 2020 11:37:11 -0700 Message-Id: <20200326183718.2384349-5-jacob.e.keller@intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200326183718.2384349-1-jacob.e.keller@intel.com> References: <20200326183718.2384349-1-jacob.e.keller@intel.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org A future change is going to add a new devlink command to request a snapshot on demand. This function will want to call the devlink_region_snapshot_create function while already holding the devlink instance lock. Extract the logic of this function into a static function prefixed by `__` to indicate that it is an internal helper function. Modify the original function to be implemented in terms of the new locked function. Signed-off-by: Jacob Keller Reviewed-by: Jiri Pirko Reviewed-by: Jakub Kicinski --- Changes since v1: * Picked up Jakub's Reviewed-by net/core/devlink.c | 78 ++++++++++++++++++++++++++++------------------ 1 file changed, 47 insertions(+), 31 deletions(-) diff --git a/net/core/devlink.c b/net/core/devlink.c index 73e66a779c13..620e9d07ac85 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -3768,6 +3768,52 @@ static void devlink_nl_region_notify(struct devlink_region *region, nlmsg_free(msg); } +/** + * __devlink_region_snapshot_create - create a new snapshot + * This will add a new snapshot of a region. The snapshot + * will be stored on the region struct and can be accessed + * from devlink. This is useful for future analyses of snapshots. + * Multiple snapshots can be created on a region. + * The @snapshot_id should be obtained using the getter function. + * + * Must be called only while holding the devlink instance lock. + * + * @region: devlink region of the snapshot + * @data: snapshot data + * @snapshot_id: snapshot id to be created + */ +static int +__devlink_region_snapshot_create(struct devlink_region *region, + u8 *data, u32 snapshot_id) +{ + struct devlink *devlink = region->devlink; + struct devlink_snapshot *snapshot; + + lockdep_assert_held(&devlink->lock); + + /* check if region can hold one more snapshot */ + if (region->cur_snapshots == region->max_snapshots) + return -ENOMEM; + + if (devlink_region_snapshot_get_by_id(region, snapshot_id)) + return -EEXIST; + + snapshot = kzalloc(sizeof(*snapshot), GFP_KERNEL); + if (!snapshot) + return -ENOMEM; + + snapshot->id = snapshot_id; + snapshot->region = region; + snapshot->data = data; + + list_add_tail(&snapshot->list, ®ion->snapshot_list); + + region->cur_snapshots++; + + devlink_nl_region_notify(region, snapshot, DEVLINK_CMD_REGION_NEW); + return 0; +} + static void devlink_region_snapshot_del(struct devlink_region *region, struct devlink_snapshot *snapshot) { @@ -7752,42 +7798,12 @@ int devlink_region_snapshot_create(struct devlink_region *region, u8 *data, u32 snapshot_id) { struct devlink *devlink = region->devlink; - struct devlink_snapshot *snapshot; int err; mutex_lock(&devlink->lock); - - /* check if region can hold one more snapshot */ - if (region->cur_snapshots == region->max_snapshots) { - err = -ENOMEM; - goto unlock; - } - - if (devlink_region_snapshot_get_by_id(region, snapshot_id)) { - err = -EEXIST; - goto unlock; - } - - snapshot = kzalloc(sizeof(*snapshot), GFP_KERNEL); - if (!snapshot) { - err = -ENOMEM; - goto unlock; - } - - snapshot->id = snapshot_id; - snapshot->region = region; - snapshot->data = data; - - list_add_tail(&snapshot->list, ®ion->snapshot_list); - - region->cur_snapshots++; - - devlink_nl_region_notify(region, snapshot, DEVLINK_CMD_REGION_NEW); + err = __devlink_region_snapshot_create(region, data, snapshot_id); mutex_unlock(&devlink->lock); - return 0; -unlock: - mutex_unlock(&devlink->lock); return err; } EXPORT_SYMBOL_GPL(devlink_region_snapshot_create); From patchwork Thu Mar 26 18:37:12 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Keller, Jacob E" X-Patchwork-Id: 1262189 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=intel.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 48pDHy0QyMz9sRY for ; Fri, 27 Mar 2020 05:37:50 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728629AbgCZShs (ORCPT ); Thu, 26 Mar 2020 14:37:48 -0400 Received: from mga09.intel.com ([134.134.136.24]:43777 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726163AbgCZShg (ORCPT ); Thu, 26 Mar 2020 14:37:36 -0400 IronPort-SDR: ff9MIxEONpuLEGR6vjunoWpn1TpBu5ScTLlH7KqNYnaCI6344JGJXAA+8w38DGy0hoNZf+nghh G3GXA6c8i0DQ== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Mar 2020 11:37:23 -0700 IronPort-SDR: vxMzHNOkLU62P6ri6Zkd9WiMAm2kES8VZaHbOFvTOH8Hljmno8OMjr0UH1bBAgzw/hs/lriF6i sZ4/BjBdeACA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,309,1580803200"; d="scan'208";a="358241624" Received: from jekeller-desk.amr.corp.intel.com ([10.166.241.33]) by fmsmga001.fm.intel.com with ESMTP; 26 Mar 2020 11:37:23 -0700 From: Jacob Keller To: netdev@vger.kernel.org Cc: Jakub Kicinski , Jiri Pirko , Jacob Keller , Jiri Pirko Subject: [PATCH net-next v3 05/11] devlink: use -ENOSPC to indicate no more room for snapshots Date: Thu, 26 Mar 2020 11:37:12 -0700 Message-Id: <20200326183718.2384349-6-jacob.e.keller@intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200326183718.2384349-1-jacob.e.keller@intel.com> References: <20200326183718.2384349-1-jacob.e.keller@intel.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org The devlink_region_snapshot_create function returns -ENOMEM when the maximum number of snapshots has been reached. This is confusing because it is not an issue of being out of memory. Change this to use -ENOSPC instead. Reported-by: Jiri Pirko Signed-off-by: Jacob Keller Reviewed-by: Jiri Pirko --- Changes since v1: * Introduced this patch net/core/devlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/core/devlink.c b/net/core/devlink.c index 620e9d07ac85..8c2c4bc009bb 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -3793,7 +3793,7 @@ __devlink_region_snapshot_create(struct devlink_region *region, /* check if region can hold one more snapshot */ if (region->cur_snapshots == region->max_snapshots) - return -ENOMEM; + return -ENOSPC; if (devlink_region_snapshot_get_by_id(region, snapshot_id)) return -EEXIST; From patchwork Thu Mar 26 18:37:13 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Keller, Jacob E" X-Patchwork-Id: 1262183 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=intel.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 48pDHl6ZNCz9sR4 for ; Fri, 27 Mar 2020 05:37:39 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728579AbgCZShj (ORCPT ); Thu, 26 Mar 2020 14:37:39 -0400 Received: from mga09.intel.com ([134.134.136.24]:43777 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728531AbgCZShg (ORCPT ); Thu, 26 Mar 2020 14:37:36 -0400 IronPort-SDR: CFQFi5UC0CErgApOd5esx2D1SllxKzDU9+3NXBYZiYGvj8eFyN2RQZ32I7LHm4VVi/ED2tY+T5 nt1G07jC2Z0w== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Mar 2020 11:37:23 -0700 IronPort-SDR: qZQUFpfPlwrV40WFCnWo9b1EPkTgxG35oRS8JQIIAMiyMbAJ99hMuC6+MpSheOQGqNiDmH9rpK oOUU3zDnOCnw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,309,1580803200"; d="scan'208";a="358241629" Received: from jekeller-desk.amr.corp.intel.com ([10.166.241.33]) by fmsmga001.fm.intel.com with ESMTP; 26 Mar 2020 11:37:23 -0700 From: Jacob Keller To: netdev@vger.kernel.org Cc: Jakub Kicinski , Jiri Pirko , Jacob Keller Subject: [PATCH net-next v3 06/11] devlink: extract snapshot id allocation to helper function Date: Thu, 26 Mar 2020 11:37:13 -0700 Message-Id: <20200326183718.2384349-7-jacob.e.keller@intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200326183718.2384349-1-jacob.e.keller@intel.com> References: <20200326183718.2384349-1-jacob.e.keller@intel.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org A future change is going to implement a new devlink command to request a snapshot on demand. As part of this, the logic for handling the snapshot ids will be refactored. To simplify the snapshot id allocation function, move it to a separate function prefixed by `__`. This helper function will assume the lock is held. While no other callers will exist, it simplifies refactoring the logic because there is no need to complicate the function with gotos to handle unlocking on failure. Signed-off-by: Jacob Keller Reviewed-by: Jiri Pirko --- Changes since RFC: * Added this patch to explain reasoning why the function is kept despite only having one caller. net/core/devlink.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/net/core/devlink.c b/net/core/devlink.c index 8c2c4bc009bb..457b8303ae59 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -3768,6 +3768,19 @@ static void devlink_nl_region_notify(struct devlink_region *region, nlmsg_free(msg); } +/** + * __devlink_region_snapshot_id_get - get snapshot ID + * @devlink: devlink instance + * + * Returns a new snapshot id. Must be called while holding the + * devlink instance lock. + */ +static u32 __devlink_region_snapshot_id_get(struct devlink *devlink) +{ + lockdep_assert_held(&devlink->lock); + return ++devlink->snapshot_id; +} + /** * __devlink_region_snapshot_create - create a new snapshot * This will add a new snapshot of a region. The snapshot @@ -7775,7 +7788,7 @@ u32 devlink_region_snapshot_id_get(struct devlink *devlink) u32 id; mutex_lock(&devlink->lock); - id = ++devlink->snapshot_id; + id = __devlink_region_snapshot_id_get(devlink); mutex_unlock(&devlink->lock); return id; From patchwork Thu Mar 26 18:37:14 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Keller, Jacob E" X-Patchwork-Id: 1262186 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=intel.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 48pDHr57vXz9sR4 for ; Fri, 27 Mar 2020 05:37:44 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728554AbgCZShi (ORCPT ); Thu, 26 Mar 2020 14:37:38 -0400 Received: from mga09.intel.com ([134.134.136.24]:43782 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728534AbgCZShg (ORCPT ); Thu, 26 Mar 2020 14:37:36 -0400 IronPort-SDR: NBSVcXP/k+ZjTa6AdryL9sXMxqYfNFwPWPGESOEUDuvO7WHs/tqZPlvtjP/IQzOe2G5nSzxnWN 3+zC+0a0Vc7A== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Mar 2020 11:37:24 -0700 IronPort-SDR: yA7zKV6Ats899XqPsOXmvO0KtofcjVcxGwjUzVnUm7kAbpw6geo3CHMSyZZ51cp9aDaaUZfNki C5jL/o0q8oiw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,309,1580803200"; d="scan'208";a="358241637" Received: from jekeller-desk.amr.corp.intel.com ([10.166.241.33]) by fmsmga001.fm.intel.com with ESMTP; 26 Mar 2020 11:37:23 -0700 From: Jacob Keller To: netdev@vger.kernel.org Cc: Jakub Kicinski , Jiri Pirko , Jacob Keller , Jiri Pirko Subject: [PATCH net-next v3 07/11] devlink: report error once U32_MAX snapshot ids have been used Date: Thu, 26 Mar 2020 11:37:14 -0700 Message-Id: <20200326183718.2384349-8-jacob.e.keller@intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200326183718.2384349-1-jacob.e.keller@intel.com> References: <20200326183718.2384349-1-jacob.e.keller@intel.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org The devlink_snapshot_id_get() function returns a snapshot id. The snapshot id is a u32, so there is no way to indicate an error code. A future change is going to possibly add additional cases where this function could fail. Refactor the function to return the snapshot id in an argument, so that it can return zero or an error value. This ensures that snapshot ids cannot be confused with error values, and aids in the future refactor of snapshot id allocation management. Because there is no current way to release previously used snapshot ids, add a simple check ensuring that an error is reported in case the snapshot_id would over flow. Signed-off-by: Jacob Keller Reviewed-by: Jiri Pirko --- Changes since RFC: * Picked up Jiri's Reviewed-by Changes since v1: * Refactored this patch to return the id in an argument, instead of conflating id and error in the return value of the function. * Allow up to U32_MAX instead of INT_MAX * Removed Jiri's Reviewed-by, since this is a complete re-write Changes since v2: * Fix commit message to avoid incorrect statement about checking snapshot ids. This referred to a previous version of the code which has been removed. * Fixed a build failure due to forgetting a '{' bracket which got lost in a rebase conflict. * Picked up Jiri's Reviewed-by again drivers/net/ethernet/mellanox/mlx4/crdump.c | 11 ++++++--- drivers/net/netdevsim/dev.c | 6 ++++- include/net/devlink.h | 2 +- net/core/devlink.c | 27 +++++++++++++++------ 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx4/crdump.c b/drivers/net/ethernet/mellanox/mlx4/crdump.c index c3f90c0f9554..792951f6df0d 100644 --- a/drivers/net/ethernet/mellanox/mlx4/crdump.c +++ b/drivers/net/ethernet/mellanox/mlx4/crdump.c @@ -169,6 +169,7 @@ int mlx4_crdump_collect(struct mlx4_dev *dev) struct pci_dev *pdev = dev->persist->pdev; unsigned long cr_res_size; u8 __iomem *cr_space; + int err; u32 id; if (!dev->caps.health_buffer_addrs) { @@ -189,10 +190,14 @@ int mlx4_crdump_collect(struct mlx4_dev *dev) return -ENODEV; } - crdump_enable_crspace_access(dev, cr_space); - /* Get the available snapshot ID for the dumps */ - id = devlink_region_snapshot_id_get(devlink); + err = devlink_region_snapshot_id_get(devlink, &id); + if (err) { + mlx4_err(dev, "crdump: devlink get snapshot id err %d\n", err); + return err; + } + + crdump_enable_crspace_access(dev, cr_space); /* Try to capture dumps */ mlx4_crdump_collect_crspace(dev, cr_space, id); diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c index f7621ccb7b88..609005f2ac85 100644 --- a/drivers/net/netdevsim/dev.c +++ b/drivers/net/netdevsim/dev.c @@ -54,7 +54,11 @@ static ssize_t nsim_dev_take_snapshot_write(struct file *file, get_random_bytes(dummy_data, NSIM_DEV_DUMMY_REGION_SIZE); - id = devlink_region_snapshot_id_get(priv_to_devlink(nsim_dev)); + err = devlink_region_snapshot_id_get(priv_to_devlink(nsim_dev), &id); + if (err) { + pr_err("Failed to get snapshot id\n"); + return err; + } err = devlink_region_snapshot_create(nsim_dev->dummy_region, dummy_data, id); if (err) { diff --git a/include/net/devlink.h b/include/net/devlink.h index 8869ad75b965..9a46bc7fed90 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -976,7 +976,7 @@ devlink_region_create(struct devlink *devlink, const struct devlink_region_ops *ops, u32 region_max_snapshots, u64 region_size); void devlink_region_destroy(struct devlink_region *region); -u32 devlink_region_snapshot_id_get(struct devlink *devlink); +int devlink_region_snapshot_id_get(struct devlink *devlink, u32 *id); int devlink_region_snapshot_create(struct devlink_region *region, u8 *data, u32 snapshot_id); int devlink_info_serial_number_put(struct devlink_info_req *req, diff --git a/net/core/devlink.c b/net/core/devlink.c index 457b8303ae59..77341c65868f 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -3771,14 +3771,22 @@ static void devlink_nl_region_notify(struct devlink_region *region, /** * __devlink_region_snapshot_id_get - get snapshot ID * @devlink: devlink instance + * @id: storage to return snapshot id * - * Returns a new snapshot id. Must be called while holding the - * devlink instance lock. + * Allocates a new snapshot id. Returns zero on success, or a negative + * error on failure. Must be called while holding the devlink instance + * lock. */ -static u32 __devlink_region_snapshot_id_get(struct devlink *devlink) +static int __devlink_region_snapshot_id_get(struct devlink *devlink, u32 *id) { lockdep_assert_held(&devlink->lock); - return ++devlink->snapshot_id; + + if (devlink->snapshot_id >= U32_MAX) + return -ENOSPC; + + *id = ++devlink->snapshot_id; + + return 0; } /** @@ -7781,17 +7789,20 @@ EXPORT_SYMBOL_GPL(devlink_region_destroy); * Driver should use the same id for multiple snapshots taken * on multiple regions at the same time/by the same trigger. * + * Returns zero on success, or a negative error code on failure. + * * @devlink: devlink + * @id: storage to return id */ -u32 devlink_region_snapshot_id_get(struct devlink *devlink) +int devlink_region_snapshot_id_get(struct devlink *devlink, u32 *id) { - u32 id; + int err; mutex_lock(&devlink->lock); - id = __devlink_region_snapshot_id_get(devlink); + err = __devlink_region_snapshot_id_get(devlink, id); mutex_unlock(&devlink->lock); - return id; + return err; } EXPORT_SYMBOL_GPL(devlink_region_snapshot_id_get); From patchwork Thu Mar 26 18:37:15 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Keller, Jacob E" X-Patchwork-Id: 1262188 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=intel.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 48pDHx0kQzz9sR4 for ; Fri, 27 Mar 2020 05:37:49 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728620AbgCZShq (ORCPT ); Thu, 26 Mar 2020 14:37:46 -0400 Received: from mga09.intel.com ([134.134.136.24]:43777 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728535AbgCZShh (ORCPT ); Thu, 26 Mar 2020 14:37:37 -0400 IronPort-SDR: d4D4Wv1As44fDbfCpOVpvDKbWG6YnQxvL9SFCWczSsNOEKJD8+NEcoppBtTQg89oKfLvZhhPJj dZ8PN6dSGPgA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Mar 2020 11:37:24 -0700 IronPort-SDR: yVcqPjrxvR6lp2VxHR6NIVA8ShVj9XHYkSmFEw3+iyog9yA+sA1nFoKDffF+210WpHdpoiHKb4 w8JGCk74fmSg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,309,1580803200"; d="scan'208";a="358241645" Received: from jekeller-desk.amr.corp.intel.com ([10.166.241.33]) by fmsmga001.fm.intel.com with ESMTP; 26 Mar 2020 11:37:24 -0700 From: Jacob Keller To: netdev@vger.kernel.org Cc: Jakub Kicinski , Jiri Pirko , Jacob Keller Subject: [PATCH net-next v3 08/11] devlink: track snapshot id usage count using an xarray Date: Thu, 26 Mar 2020 11:37:15 -0700 Message-Id: <20200326183718.2384349-9-jacob.e.keller@intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200326183718.2384349-1-jacob.e.keller@intel.com> References: <20200326183718.2384349-1-jacob.e.keller@intel.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Each snapshot created for a devlink region must have an id. These ids are supposed to be unique per "event" that caused the snapshot to be created. Drivers call devlink_region_snapshot_id_get to obtain a new id to use for a new event trigger. The id values are tracked per devlink, so that the same id number can be used if a triggering event creates multiple snapshots on different regions. There is no mechanism for snapshot ids to ever be reused. Introduce an xarray to store the count of how many snapshots are using a given id, replacing the snapshot_id field previously used for picking the next id. The devlink_region_snapshot_id_get() function will use xa_alloc to insert an initial value of 1 value at an available slot between 0 and U32_MAX. The new __devlink_snapshot_id_increment() and __devlink_snapshot_id_decrement() functions will be used to track how many snapshots currently use an id. Drivers must now call devlink_snapshot_id_put() in order to release their reference of the snapshot id after adding region snapshots. By tracking the total number of snapshots using a given id, it is possible for the decrement() function to erase the id from the xarray when it is not in use. With this method, a snapshot id can become reused again once all snapshots that referred to it have been deleted via DEVLINK_CMD_REGION_DEL, and the driver has finished adding snapshots. This work also paves the way to introduce a mechanism for userspace to request a snapshot. Signed-off-by: Jacob Keller Reviewed-by: Jiri Pirko --- Changes since RFC: * Rewrote to use xarray directly, instead of IDR Changes since v1: * Add a new devlink_region_snapshot_id_put, and have the devlink_region_snapshot_id_get start the xarray usage count at 1. This fixes a race condition where the driver might create two snapshots using the id, but the first one is freed before the second one is created. Additionally it resolves an issue where the snapshot id could be locked forever if the driver does not create a snapshot due to an error. * Add WARN_ON to checks in __devlink_snapshot_id_increment * Remove "if (err) return err" constructions that occur just after a return 0, as a direct return can be used instead. * Rename goto labels to indicate the cause of failure instead of the action taken to clean up. * Set XA_FLAGS_ALLOC so that xa_alloc can actually be used properly * Remove the unnecessary locking around xa_destroy Changes since v2: * Move devlink variable assignment * Remove a comment about placement of devlink_region_snapshot_id_put * Renamed label to use "err_" prefix drivers/net/ethernet/mellanox/mlx4/crdump.c | 3 + drivers/net/netdevsim/dev.c | 6 +- include/net/devlink.h | 4 +- net/core/devlink.c | 130 +++++++++++++++++++- 4 files changed, 135 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx4/crdump.c b/drivers/net/ethernet/mellanox/mlx4/crdump.c index 792951f6df0d..2700628f1689 100644 --- a/drivers/net/ethernet/mellanox/mlx4/crdump.c +++ b/drivers/net/ethernet/mellanox/mlx4/crdump.c @@ -203,6 +203,9 @@ int mlx4_crdump_collect(struct mlx4_dev *dev) mlx4_crdump_collect_crspace(dev, cr_space, id); mlx4_crdump_collect_fw_health(dev, cr_space, id); + /* Release reference on the snapshot id */ + devlink_region_snapshot_id_put(devlink, id); + crdump_disable_crspace_access(dev, cr_space); iounmap(cr_space); diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c index 609005f2ac85..f4f6539f1e17 100644 --- a/drivers/net/netdevsim/dev.c +++ b/drivers/net/netdevsim/dev.c @@ -44,23 +44,27 @@ static ssize_t nsim_dev_take_snapshot_write(struct file *file, size_t count, loff_t *ppos) { struct nsim_dev *nsim_dev = file->private_data; + struct devlink *devlink; void *dummy_data; int err; u32 id; + devlink = priv_to_devlink(nsim_dev); + dummy_data = kmalloc(NSIM_DEV_DUMMY_REGION_SIZE, GFP_KERNEL); if (!dummy_data) return -ENOMEM; get_random_bytes(dummy_data, NSIM_DEV_DUMMY_REGION_SIZE); - err = devlink_region_snapshot_id_get(priv_to_devlink(nsim_dev), &id); + err = devlink_region_snapshot_id_get(devlink, &id); if (err) { pr_err("Failed to get snapshot id\n"); return err; } err = devlink_region_snapshot_create(nsim_dev->dummy_region, dummy_data, id); + devlink_region_snapshot_id_put(devlink, id); if (err) { pr_err("Failed to create region snapshot\n"); kfree(dummy_data); diff --git a/include/net/devlink.h b/include/net/devlink.h index 9a46bc7fed90..fb9154060e6e 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -18,6 +18,7 @@ #include #include #include +#include struct devlink_ops; @@ -29,13 +30,13 @@ struct devlink { struct list_head resource_list; struct list_head param_list; struct list_head region_list; - u32 snapshot_id; struct list_head reporter_list; struct mutex reporters_lock; /* protects reporter_list */ struct devlink_dpipe_headers *dpipe_headers; struct list_head trap_list; struct list_head trap_group_list; const struct devlink_ops *ops; + struct xarray snapshot_ids; struct device *dev; possible_net_t _net; struct mutex lock; @@ -977,6 +978,7 @@ devlink_region_create(struct devlink *devlink, u32 region_max_snapshots, u64 region_size); void devlink_region_destroy(struct devlink_region *region); int devlink_region_snapshot_id_get(struct devlink *devlink, u32 *id); +void devlink_region_snapshot_id_put(struct devlink *devlink, u32 id); int devlink_region_snapshot_create(struct devlink_region *region, u8 *data, u32 snapshot_id); int devlink_info_serial_number_put(struct devlink_info_req *req, diff --git a/net/core/devlink.c b/net/core/devlink.c index 77341c65868f..b410fb126a66 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -3768,6 +3768,83 @@ static void devlink_nl_region_notify(struct devlink_region *region, nlmsg_free(msg); } +/** + * __devlink_snapshot_id_increment - Increment number of snapshots using an id + * @devlink: devlink instance + * @id: the snapshot id + * + * Track when a new snapshot begins using an id. Load the count for the + * given id from the snapshot xarray, increment it, and store it back. + * + * Called when a new snapshot is created with the given id. + * + * The id *must* have been previously allocated by + * devlink_region_snapshot_id_get(). + * + * Returns 0 on success, or an error on failure. + */ +static int __devlink_snapshot_id_increment(struct devlink *devlink, u32 id) +{ + unsigned long count; + void *p; + + lockdep_assert_held(&devlink->lock); + + p = xa_load(&devlink->snapshot_ids, id); + if (WARN_ON(!p)) + return -EINVAL; + + if (WARN_ON(!xa_is_value(p))) + return -EINVAL; + + count = xa_to_value(p); + count++; + + return xa_err(xa_store(&devlink->snapshot_ids, id, xa_mk_value(count), + GFP_KERNEL)); +} + +/** + * __devlink_snapshot_id_decrement - Decrease number of snapshots using an id + * @devlink: devlink instance + * @id: the snapshot id + * + * Track when a snapshot is deleted and stops using an id. Load the count + * for the given id from the snapshot xarray, decrement it, and store it + * back. + * + * If the count reaches zero, erase this id from the xarray, freeing it + * up for future re-use by devlink_region_snapshot_id_get(). + * + * Called when a snapshot using the given id is deleted, and when the + * initial allocator of the id is finished using it. + */ +static void __devlink_snapshot_id_decrement(struct devlink *devlink, u32 id) +{ + unsigned long count; + void *p; + + lockdep_assert_held(&devlink->lock); + + p = xa_load(&devlink->snapshot_ids, id); + if (WARN_ON(!p)) + return; + + if (WARN_ON(!xa_is_value(p))) + return; + + count = xa_to_value(p); + + if (count > 1) { + count--; + xa_store(&devlink->snapshot_ids, id, xa_mk_value(count), + GFP_KERNEL); + } else { + /* If this was the last user, we can erase this id */ + xa_erase(&devlink->snapshot_ids, id); + } +} + /** * __devlink_region_snapshot_id_get - get snapshot ID * @devlink: devlink instance @@ -3776,17 +3853,20 @@ static void devlink_nl_region_notify(struct devlink_region *region, * Allocates a new snapshot id. Returns zero on success, or a negative * error on failure. Must be called while holding the devlink instance * lock. + * + * Snapshot IDs are tracked using an xarray which stores the number of + * users of the snapshot id. + * + * Note that the caller of this function counts as a 'user', in order to + * avoid race conditions. The caller must release its hold on the + * snapshot by using devlink_region_snapshot_id_put. */ static int __devlink_region_snapshot_id_get(struct devlink *devlink, u32 *id) { lockdep_assert_held(&devlink->lock); - if (devlink->snapshot_id >= U32_MAX) - return -ENOSPC; - - *id = ++devlink->snapshot_id; - - return 0; + return xa_alloc(&devlink->snapshot_ids, id, xa_mk_value(1), + xa_limit_32b, GFP_KERNEL); } /** @@ -3809,6 +3889,7 @@ __devlink_region_snapshot_create(struct devlink_region *region, { struct devlink *devlink = region->devlink; struct devlink_snapshot *snapshot; + int err; lockdep_assert_held(&devlink->lock); @@ -3823,6 +3904,10 @@ __devlink_region_snapshot_create(struct devlink_region *region, if (!snapshot) return -ENOMEM; + err = __devlink_snapshot_id_increment(devlink, snapshot_id); + if (err) + goto err_snapshot_id_increment; + snapshot->id = snapshot_id; snapshot->region = region; snapshot->data = data; @@ -3833,15 +3918,24 @@ __devlink_region_snapshot_create(struct devlink_region *region, devlink_nl_region_notify(region, snapshot, DEVLINK_CMD_REGION_NEW); return 0; + +err_snapshot_id_increment: + kfree(snapshot); + return err; } static void devlink_region_snapshot_del(struct devlink_region *region, struct devlink_snapshot *snapshot) { + struct devlink *devlink = region->devlink; + + lockdep_assert_held(&devlink->lock); + devlink_nl_region_notify(region, snapshot, DEVLINK_CMD_REGION_DEL); region->cur_snapshots--; list_del(&snapshot->list); region->ops->destructor(snapshot->data); + __devlink_snapshot_id_decrement(devlink, snapshot->id); kfree(snapshot); } @@ -6494,6 +6588,7 @@ struct devlink *devlink_alloc(const struct devlink_ops *ops, size_t priv_size) if (!devlink) return NULL; devlink->ops = ops; + xa_init_flags(&devlink->snapshot_ids, XA_FLAGS_ALLOC); __devlink_net_set(devlink, &init_net); INIT_LIST_HEAD(&devlink->port_list); INIT_LIST_HEAD(&devlink->sb_list); @@ -6598,6 +6693,8 @@ void devlink_free(struct devlink *devlink) WARN_ON(!list_empty(&devlink->sb_list)); WARN_ON(!list_empty(&devlink->port_list)); + xa_destroy(&devlink->snapshot_ids); + kfree(devlink); } EXPORT_SYMBOL_GPL(devlink_free); @@ -7789,6 +7886,9 @@ EXPORT_SYMBOL_GPL(devlink_region_destroy); * Driver should use the same id for multiple snapshots taken * on multiple regions at the same time/by the same trigger. * + * The caller of this function must use devlink_region_snapshot_id_put + * when finished creating regions using this id. + * * Returns zero on success, or a negative error code on failure. * * @devlink: devlink @@ -7806,6 +7906,24 @@ int devlink_region_snapshot_id_get(struct devlink *devlink, u32 *id) } EXPORT_SYMBOL_GPL(devlink_region_snapshot_id_get); +/** + * devlink_region_snapshot_id_put - put snapshot ID reference + * + * This should be called by a driver after finishing creating snapshots + * with an id. Doing so ensures that the ID can later be released in the + * event that all snapshots using it have been destroyed. + * + * @devlink: devlink + * @id: id to release reference on + */ +void devlink_region_snapshot_id_put(struct devlink *devlink, u32 id) +{ + mutex_lock(&devlink->lock); + __devlink_snapshot_id_decrement(devlink, id); + mutex_unlock(&devlink->lock); +} +EXPORT_SYMBOL_GPL(devlink_region_snapshot_id_put); + /** * devlink_region_snapshot_create - create a new snapshot * This will add a new snapshot of a region. The snapshot From patchwork Thu Mar 26 18:37:16 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Keller, Jacob E" X-Patchwork-Id: 1262187 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=intel.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 48pDHt2ftTz9sR4 for ; Fri, 27 Mar 2020 05:37:46 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728614AbgCZShp (ORCPT ); Thu, 26 Mar 2020 14:37:45 -0400 Received: from mga09.intel.com ([134.134.136.24]:43782 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727446AbgCZShh (ORCPT ); Thu, 26 Mar 2020 14:37:37 -0400 IronPort-SDR: CxqXNnjeNrFSwYPHEm0ZayDj0rp4CPreuIbhhm8gGHyoZl3qfmu9q8qWBM4XLVTviQH35PVDSD 7mz8I+TXXwyA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Mar 2020 11:37:24 -0700 IronPort-SDR: 4VSvdDSOcrevWgU9m3v/rBGDRTj2LCuIDXSE5el7pRUBrI3Wt/1Ona9PtKXc8J3BWLdq4V43BX JJmMMDfUpp6w== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,309,1580803200"; d="scan'208";a="358241649" Received: from jekeller-desk.amr.corp.intel.com ([10.166.241.33]) by fmsmga001.fm.intel.com with ESMTP; 26 Mar 2020 11:37:24 -0700 From: Jacob Keller To: netdev@vger.kernel.org Cc: Jakub Kicinski , Jiri Pirko , Jacob Keller , Jiri Pirko Subject: [PATCH net-next v3 09/11] devlink: implement DEVLINK_CMD_REGION_NEW Date: Thu, 26 Mar 2020 11:37:16 -0700 Message-Id: <20200326183718.2384349-10-jacob.e.keller@intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200326183718.2384349-1-jacob.e.keller@intel.com> References: <20200326183718.2384349-1-jacob.e.keller@intel.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Implement support for the DEVLINK_CMD_REGION_NEW command for creating snapshots. This new command parallels the existing DEVLINK_CMD_REGION_DEL. In order for DEVLINK_CMD_REGION_NEW to work for a region, the new ".snapshot" operation must be implemented in the region's ops structure. The desired snapshot id must be provided. This helps avoid confusion on the purpose of DEVLINK_CMD_REGION_NEW, and keeps the API simpler. The requested id will be inserted into the xarray tracking the number of snapshots using each id. If this id is already used by another snapshot on any region, an error will be returned. Signed-off-by: Jacob Keller Reviewed-by: Jiri Pirko --- Changes since RFC: * Removed ability to not specify the snapshot id. Now requesting a region must always come with a snapshot id, as suggested by Jiri. * Removed unnecessary newlines on NL_SET_ERR_MSG_MOD calls Changes since v1: * Add a WARN_ON to the xa_load check in devlink_snapshot_id_insert * Remove an unnecessary "if (err) return err" construction in the insert function, as a direct return would behave the same here. * Use ENOSPC instead of ENOMEM for when the maximum number of snapshots is reached. * Remove an NL_SET_ERR_MSG_MOD message when snapshot_id_insert fails, as the only expected failures would be ENOMEM or would trigger a WARN_ON. * Rename labels to specify the cause of failure rather than the action taken to cleanup. Changes since v2: * Renamed labels again to use "err_" prefix. * Picked up Jiri's Reviewed-by * Removed { } that are no longer necessary since the NL_SET_ERR_MSG_MOD call was removed in a previous version. .../networking/devlink/devlink-region.rst | 8 ++ include/net/devlink.h | 6 ++ net/core/devlink.c | 99 +++++++++++++++++++ 3 files changed, 113 insertions(+) diff --git a/Documentation/networking/devlink/devlink-region.rst b/Documentation/networking/devlink/devlink-region.rst index 8b46e8591fe0..9d2d4c95a5c4 100644 --- a/Documentation/networking/devlink/devlink-region.rst +++ b/Documentation/networking/devlink/devlink-region.rst @@ -20,6 +20,11 @@ address regions that are otherwise inaccessible to the user. Regions may also be used to provide an additional way to debug complex error states, but see also :doc:`devlink-health` +Regions may optionally support capturing a snapshot on demand via the +``DEVLINK_CMD_REGION_NEW`` netlink message. A driver wishing to allow +requested snapshots must implement the ``.snapshot`` callback for the region +in its ``devlink_region_ops`` structure. + example usage ------------- @@ -40,6 +45,9 @@ example usage # Delete a snapshot using: $ devlink region del pci/0000:00:05.0/cr-space snapshot 1 + # Request an immediate snapshot, if supported by the region + $ devlink region new pci/0000:00:05.0/cr-space snapshot 5 + # Dump a snapshot: $ devlink region dump pci/0000:00:05.0/fw-health snapshot 1 0000000000000000 0014 95dc 0014 9514 0035 1670 0034 db30 diff --git a/include/net/devlink.h b/include/net/devlink.h index fb9154060e6e..a1a02cd5890b 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -501,10 +501,16 @@ struct devlink_info_req; * struct devlink_region_ops - Region operations * @name: region name * @destructor: callback used to free snapshot memory when deleting + * @snapshot: callback to request an immediate snapshot. On success, + * the data variable must be updated to point to the snapshot data. + * The function will be called while the devlink instance lock is + * held. */ struct devlink_region_ops { const char *name; void (*destructor)(const void *data); + int (*snapshot)(struct devlink *devlink, struct netlink_ext_ack *extack, + u8 **data); }; struct devlink_fmsg; diff --git a/net/core/devlink.c b/net/core/devlink.c index b410fb126a66..15fbb8d8102e 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -3845,6 +3845,33 @@ static void __devlink_snapshot_id_decrement(struct devlink *devlink, u32 id) } } +/** + * __devlink_snapshot_id_insert - Insert a specific snapshot ID + * @devlink: devlink instance + * @id: the snapshot id + * + * Mark the given snapshot id as used by inserting a zero value into the + * snapshot xarray. + * + * This must be called while holding the devlink instance lock. Unlike + * devlink_snapshot_id_get, the initial reference count is zero, not one. + * It is expected that the id will immediately be used before + * releasing the devlink instance lock. + * + * Returns zero on success, or an error code if the snapshot id could not + * be inserted. + */ +static int __devlink_snapshot_id_insert(struct devlink *devlink, u32 id) +{ + lockdep_assert_held(&devlink->lock); + + if (WARN_ON(xa_load(&devlink->snapshot_ids, id))) + return -EEXIST; + + return xa_err(xa_store(&devlink->snapshot_ids, id, xa_mk_value(0), + GFP_KERNEL)); +} + /** * __devlink_region_snapshot_id_get - get snapshot ID * @devlink: devlink instance @@ -4038,6 +4065,71 @@ static int devlink_nl_cmd_region_del(struct sk_buff *skb, return 0; } +static int +devlink_nl_cmd_region_new(struct sk_buff *skb, struct genl_info *info) +{ + struct devlink *devlink = info->user_ptr[0]; + struct devlink_region *region; + const char *region_name; + u32 snapshot_id; + u8 *data; + int err; + + if (!info->attrs[DEVLINK_ATTR_REGION_NAME]) { + NL_SET_ERR_MSG_MOD(info->extack, "No region name provided"); + return -EINVAL; + } + + if (!info->attrs[DEVLINK_ATTR_REGION_SNAPSHOT_ID]) { + NL_SET_ERR_MSG_MOD(info->extack, "No snapshot id provided"); + return -EINVAL; + } + + region_name = nla_data(info->attrs[DEVLINK_ATTR_REGION_NAME]); + region = devlink_region_get_by_name(devlink, region_name); + if (!region) { + NL_SET_ERR_MSG_MOD(info->extack, "The requested region does not exist"); + return -EINVAL; + } + + if (!region->ops->snapshot) { + NL_SET_ERR_MSG_MOD(info->extack, "The requested region does not support taking an immediate snapshot"); + return -EOPNOTSUPP; + } + + if (region->cur_snapshots == region->max_snapshots) { + NL_SET_ERR_MSG_MOD(info->extack, "The region has reached the maximum number of stored snapshots"); + return -ENOSPC; + } + + snapshot_id = nla_get_u32(info->attrs[DEVLINK_ATTR_REGION_SNAPSHOT_ID]); + + if (devlink_region_snapshot_get_by_id(region, snapshot_id)) { + NL_SET_ERR_MSG_MOD(info->extack, "The requested snapshot id is already in use"); + return -EEXIST; + } + + err = __devlink_snapshot_id_insert(devlink, snapshot_id); + if (err) + return err; + + err = region->ops->snapshot(devlink, info->extack, &data); + if (err) + goto err_snapshot_capture; + + err = __devlink_region_snapshot_create(region, data, snapshot_id); + if (err) + goto err_snapshot_create; + + return 0; + +err_snapshot_create: + region->ops->destructor(data); +err_snapshot_capture: + __devlink_snapshot_id_decrement(devlink, snapshot_id); + return err; +} + static int devlink_nl_cmd_region_read_chunk_fill(struct sk_buff *msg, struct devlink *devlink, u8 *chunk, u32 chunk_size, @@ -6445,6 +6537,13 @@ static const struct genl_ops devlink_nl_ops[] = { .flags = GENL_ADMIN_PERM, .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK, }, + { + .cmd = DEVLINK_CMD_REGION_NEW, + .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, + .doit = devlink_nl_cmd_region_new, + .flags = GENL_ADMIN_PERM, + .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK, + }, { .cmd = DEVLINK_CMD_REGION_DEL, .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, From patchwork Thu Mar 26 18:37:17 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Keller, Jacob E" X-Patchwork-Id: 1262184 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=intel.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 48pDHn3rVZz9sR4 for ; Fri, 27 Mar 2020 05:37:41 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728596AbgCZShk (ORCPT ); Thu, 26 Mar 2020 14:37:40 -0400 Received: from mga09.intel.com ([134.134.136.24]:43777 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728569AbgCZShj (ORCPT ); Thu, 26 Mar 2020 14:37:39 -0400 IronPort-SDR: yn6dVZISom3Zi7BxMbpRoFS6wLlCm85qHkuysXLfvY3pKjwh9oTY4VDhmrMJAVkRG8d1b45IxC o8rLrTZ9zuaA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Mar 2020 11:37:24 -0700 IronPort-SDR: JK9Ge80zClu0jPUeNZtdKwqQRGDKNaE55aS7x8ETBj7JAlZxbeCn9ufiic4h/BD/tP/8y5ggQk L4knz5b4uWPg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,309,1580803200"; d="scan'208";a="358241655" Received: from jekeller-desk.amr.corp.intel.com ([10.166.241.33]) by fmsmga001.fm.intel.com with ESMTP; 26 Mar 2020 11:37:24 -0700 From: Jacob Keller To: netdev@vger.kernel.org Cc: Jakub Kicinski , Jiri Pirko , Jacob Keller Subject: [PATCH net-next v3 10/11] netdevsim: support taking immediate snapshot via devlink Date: Thu, 26 Mar 2020 11:37:17 -0700 Message-Id: <20200326183718.2384349-11-jacob.e.keller@intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200326183718.2384349-1-jacob.e.keller@intel.com> References: <20200326183718.2384349-1-jacob.e.keller@intel.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Implement the .snapshot region operation for the dummy data region. This enables a region snapshot to be taken upon request via the new DEVLINK_CMD_REGION_SNAPSHOT command. Signed-off-by: Jacob Keller Reviewed-by: Jiri Pirko --- Changes since v1 * remove selftests for creating a region without a snapshot id, as this functionality has been removed. * Ran the selftests to ensure they pass now Changes since v2 * Moved devlink variable assignment drivers/net/netdevsim/dev.c | 28 +++++++++++++++---- .../drivers/net/netdevsim/devlink.sh | 10 +++++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c index f4f6539f1e17..2b727a7001f6 100644 --- a/drivers/net/netdevsim/dev.c +++ b/drivers/net/netdevsim/dev.c @@ -39,23 +39,38 @@ static struct dentry *nsim_dev_ddir; #define NSIM_DEV_DUMMY_REGION_SIZE (1024 * 32) +static int +nsim_dev_take_snapshot(struct devlink *devlink, struct netlink_ext_ack *extack, + u8 **data) +{ + void *dummy_data; + + dummy_data = kmalloc(NSIM_DEV_DUMMY_REGION_SIZE, GFP_KERNEL); + if (!dummy_data) + return -ENOMEM; + + get_random_bytes(dummy_data, NSIM_DEV_DUMMY_REGION_SIZE); + + *data = dummy_data; + + return 0; +} + static ssize_t nsim_dev_take_snapshot_write(struct file *file, const char __user *data, size_t count, loff_t *ppos) { struct nsim_dev *nsim_dev = file->private_data; struct devlink *devlink; - void *dummy_data; + u8 *dummy_data; int err; u32 id; devlink = priv_to_devlink(nsim_dev); - dummy_data = kmalloc(NSIM_DEV_DUMMY_REGION_SIZE, GFP_KERNEL); - if (!dummy_data) - return -ENOMEM; - - get_random_bytes(dummy_data, NSIM_DEV_DUMMY_REGION_SIZE); + err = nsim_dev_take_snapshot(devlink, NULL, &dummy_data); + if (err) + return err; err = devlink_region_snapshot_id_get(devlink, &id); if (err) { @@ -351,6 +366,7 @@ static void nsim_devlink_param_load_driverinit_values(struct devlink *devlink) static const struct devlink_region_ops dummy_region_ops = { .name = "dummy", .destructor = &kfree, + .snapshot = nsim_dev_take_snapshot, }; static int nsim_dev_dummy_region_init(struct nsim_dev *nsim_dev, diff --git a/tools/testing/selftests/drivers/net/netdevsim/devlink.sh b/tools/testing/selftests/drivers/net/netdevsim/devlink.sh index 025a84c2ab5a..32cb2a159c70 100755 --- a/tools/testing/selftests/drivers/net/netdevsim/devlink.sh +++ b/tools/testing/selftests/drivers/net/netdevsim/devlink.sh @@ -141,6 +141,16 @@ regions_test() check_region_snapshot_count dummy post-first-delete 2 + devlink region new $DL_HANDLE/dummy snapshot 25 + check_err $? "Failed to create a new snapshot with id 25" + + check_region_snapshot_count dummy post-first-request 3 + + devlink region del $DL_HANDLE/dummy snapshot 25 + check_err $? "Failed to delete snapshot with id 25" + + check_region_snapshot_count dummy post-second-delete 2 + log_test "regions test" } From patchwork Thu Mar 26 18:37:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Keller, Jacob E" X-Patchwork-Id: 1262185 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=intel.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 48pDHp6rjlz9sR4 for ; Fri, 27 Mar 2020 05:37:42 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728607AbgCZShl (ORCPT ); Thu, 26 Mar 2020 14:37:41 -0400 Received: from mga09.intel.com ([134.134.136.24]:43782 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728578AbgCZShj (ORCPT ); Thu, 26 Mar 2020 14:37:39 -0400 IronPort-SDR: SvLEMD6PNoAPZcVa2JZh5hTwytWC+GQfcIin+4U4eLUn34+8Uob4Y7cMeeAeFiYV6X1TcgWriU 8zoujgm8tstQ== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Mar 2020 11:37:24 -0700 IronPort-SDR: DpW8aKKxBjnEoW7Lra3maV4uo6y39Lq1vbHHZjhbEOdeBkm9jN2cwVn1kwpuGoWSReStqnA7JT RXoOaHfaP4vg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,309,1580803200"; d="scan'208";a="358241661" Received: from jekeller-desk.amr.corp.intel.com ([10.166.241.33]) by fmsmga001.fm.intel.com with ESMTP; 26 Mar 2020 11:37:24 -0700 From: Jacob Keller To: netdev@vger.kernel.org Cc: Jakub Kicinski , Jiri Pirko , Jacob Keller Subject: [PATCH net-next v3 11/11] ice: add a devlink region for dumping NVM contents Date: Thu, 26 Mar 2020 11:37:18 -0700 Message-Id: <20200326183718.2384349-12-jacob.e.keller@intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200326183718.2384349-1-jacob.e.keller@intel.com> References: <20200326183718.2384349-1-jacob.e.keller@intel.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Add a devlink region for exposing the device's Non Volatime Memory flash contents. Support the recently added .snapshot operation, enabling userspace to request a snapshot of the NVM contents via DEVLINK_CMD_REGION_NEW. Signed-off-by: Jacob Keller Reviewed-by: Jiri Pirko --- Changes since RFC: * Capture the entire NVM instead of just Shadow RAM * Use vmalloc instead of kmalloc, since the memory does not need to be physically contiguous. * Remove the direct reading function, as this will be sent in a separate series Changes since v1: * use dev_err instead of dev_warn when a region fails to be created Changes since v2: * Removed redundant "out of memory" extack message * Fixed up function declaration alignment to avoid checkpatch.pl warnings Documentation/networking/devlink/ice.rst | 26 ++++++ drivers/net/ethernet/intel/ice/ice.h | 2 + drivers/net/ethernet/intel/ice/ice_devlink.c | 96 ++++++++++++++++++++ drivers/net/ethernet/intel/ice/ice_devlink.h | 3 + drivers/net/ethernet/intel/ice/ice_main.c | 4 + 5 files changed, 131 insertions(+) diff --git a/Documentation/networking/devlink/ice.rst b/Documentation/networking/devlink/ice.rst index 37fbbd40a5e5..f3d6a3b50342 100644 --- a/Documentation/networking/devlink/ice.rst +++ b/Documentation/networking/devlink/ice.rst @@ -69,3 +69,29 @@ The ``ice`` driver reports the following versions - The version of the DDP package that is active in the device. Note that both the name (as reported by ``fw.app.name``) and version are required to uniquely identify the package. + +Regions +======= + +The ``ice`` driver enables access to the contents of the Non Volatile Memory +flash chip via the ``nvm-flash`` region. + +Users can request an immediate capture of a snapshot via the +``DEVLINK_CMD_REGION_NEW`` + +.. code:: shell + + $ devlink region new pci/0000:01:00.0/nvm-flash snapshot 1 + $ devlink region dump pci/0000:01:00.0/nvm-flash snapshot 1 + + $ devlink region dump pci/0000:01:00.0/nvm-flash snapshot 1 + 0000000000000000 0014 95dc 0014 9514 0035 1670 0034 db30 + 0000000000000010 0000 0000 ffff ff04 0029 8c00 0028 8cc8 + 0000000000000020 0016 0bb8 0016 1720 0000 0000 c00f 3ffc + 0000000000000030 bada cce5 bada cce5 bada cce5 bada cce5 + + $ devlink region read pci/0000:01:00.0/nvm-flash snapshot 1 address 0 + length 16 + 0000000000000000 0014 95dc 0014 9514 0035 1670 0034 db30 + + $ devlink region delete pci/0000:01:00.0/nvm-flash snapshot 1 diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h index 8ce3afcfeca0..5c11448bfbb3 100644 --- a/drivers/net/ethernet/intel/ice/ice.h +++ b/drivers/net/ethernet/intel/ice/ice.h @@ -351,6 +351,8 @@ struct ice_pf { /* devlink port data */ struct devlink_port devlink_port; + struct devlink_region *nvm_region; + /* OS reserved IRQ details */ struct msix_entry *msix_entries; struct ice_res_tracker *irq_tracker; diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c index 27c5034c039a..c6833944b90a 100644 --- a/drivers/net/ethernet/intel/ice/ice_devlink.c +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c @@ -318,3 +318,99 @@ void ice_devlink_destroy_port(struct ice_pf *pf) devlink_port_type_clear(&pf->devlink_port); devlink_port_unregister(&pf->devlink_port); } + +/** + * ice_devlink_nvm_snapshot - Capture a snapshot of the Shadow RAM contents + * @devlink: the devlink instance + * @extack: extended ACK response structure + * @data: on exit points to snapshot data buffer + * + * This function is called in response to the DEVLINK_CMD_REGION_TRIGGER for + * the shadow-ram devlink region. It captures a snapshot of the shadow ram + * contents. This snapshot can later be viewed via the devlink-region + * interface. + * + * @returns zero on success, and updates the data pointer. Returns a non-zero + * error code on failure. + */ +static int ice_devlink_nvm_snapshot(struct devlink *devlink, + struct netlink_ext_ack *extack, u8 **data) +{ + struct ice_pf *pf = devlink_priv(devlink); + struct device *dev = ice_pf_to_dev(pf); + struct ice_hw *hw = &pf->hw; + enum ice_status status; + void *nvm_data; + u32 nvm_size; + + nvm_size = hw->nvm.flash_size; + nvm_data = vzalloc(nvm_size); + if (!nvm_data) + return -ENOMEM; + + status = ice_acquire_nvm(hw, ICE_RES_READ); + if (status) { + dev_dbg(dev, "ice_acquire_nvm failed, err %d aq_err %d\n", + status, hw->adminq.sq_last_status); + NL_SET_ERR_MSG_MOD(extack, "Failed to acquire NVM semaphore"); + vfree(nvm_data); + return -EIO; + } + + status = ice_read_flat_nvm(hw, 0, &nvm_size, nvm_data, false); + if (status) { + dev_dbg(dev, "ice_read_flat_nvm failed after reading %u bytes, err %d aq_err %d\n", + nvm_size, status, hw->adminq.sq_last_status); + NL_SET_ERR_MSG_MOD(extack, "Failed to read NVM contents"); + ice_release_nvm(hw); + vfree(nvm_data); + return -EIO; + } + + ice_release_nvm(hw); + + *data = nvm_data; + + return 0; +} + +static const struct devlink_region_ops ice_nvm_region_ops = { + .name = "nvm-flash", + .destructor = vfree, + .snapshot = ice_devlink_nvm_snapshot, +}; + +/** + * ice_devlink_init_regions - Initialize devlink regions + * @pf: the PF device structure + * + * Create devlink regions used to enable access to dump the contents of the + * flash memory on the device. + */ +void ice_devlink_init_regions(struct ice_pf *pf) +{ + struct devlink *devlink = priv_to_devlink(pf); + struct device *dev = ice_pf_to_dev(pf); + u64 nvm_size; + + nvm_size = pf->hw.nvm.flash_size; + pf->nvm_region = devlink_region_create(devlink, &ice_nvm_region_ops, 1, + nvm_size); + if (IS_ERR(pf->nvm_region)) { + dev_err(dev, "failed to create NVM devlink region, err %ld\n", + PTR_ERR(pf->nvm_region)); + pf->nvm_region = NULL; + } +} + +/** + * ice_devlink_destroy_regions - Destroy devlink regions + * @pf: the PF device structure + * + * Remove previously created regions for this PF. + */ +void ice_devlink_destroy_regions(struct ice_pf *pf) +{ + if (pf->nvm_region) + devlink_region_destroy(pf->nvm_region); +} diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.h b/drivers/net/ethernet/intel/ice/ice_devlink.h index f94dc93c24c5..6e806a08dc23 100644 --- a/drivers/net/ethernet/intel/ice/ice_devlink.h +++ b/drivers/net/ethernet/intel/ice/ice_devlink.h @@ -11,4 +11,7 @@ void ice_devlink_unregister(struct ice_pf *pf); int ice_devlink_create_port(struct ice_pf *pf); void ice_devlink_destroy_port(struct ice_pf *pf); +void ice_devlink_init_regions(struct ice_pf *pf); +void ice_devlink_destroy_regions(struct ice_pf *pf); + #endif /* _ICE_DEVLINK_H_ */ diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c index 359ff8544773..306a4e5b2320 100644 --- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c @@ -3276,6 +3276,8 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent) goto err_init_pf_unroll; } + ice_devlink_init_regions(pf); + pf->num_alloc_vsi = hw->func_caps.guar_num_vsi; if (!pf->num_alloc_vsi) { err = -EIO; @@ -3385,6 +3387,7 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent) devm_kfree(dev, pf->vsi); err_init_pf_unroll: ice_deinit_pf(pf); + ice_devlink_destroy_regions(pf); ice_deinit_hw(hw); err_exit_unroll: ice_devlink_unregister(pf); @@ -3427,6 +3430,7 @@ static void ice_remove(struct pci_dev *pdev) ice_vsi_free_q_vectors(pf->vsi[i]); } ice_deinit_pf(pf); + ice_devlink_destroy_regions(pf); ice_deinit_hw(&pf->hw); ice_devlink_unregister(pf);