From patchwork Thu Mar 26 03:51:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacob Keller X-Patchwork-Id: 221836 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=-9.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS, 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 95C37C2D0EA for ; Thu, 26 Mar 2020 03:52:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6BAD120737 for ; Thu, 26 Mar 2020 03:52:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727695AbgCZDwO (ORCPT ); Wed, 25 Mar 2020 23:52:14 -0400 Received: from mga12.intel.com ([192.55.52.136]:29070 "EHLO mga12.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727639AbgCZDwN (ORCPT ); Wed, 25 Mar 2020 23:52:13 -0400 IronPort-SDR: Vsls5P4vnSOYlbjsrFapBvbmSUEygsdFOYrpN7BNsrbp/kJc8sMmgbVZFdTvp7wyMMvGuOQTwZ VfIzqIqU53qA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Mar 2020 20:52:13 -0700 IronPort-SDR: kJ8PBJyRZqwIFHgf6j0cUZYj9FPx6X5Wo+L99kDJTLcxBWlLKYGBUS/6mmYsSXvEhFLkWMnUlw j00os19wsefQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,306,1580803200"; d="scan'208";a="271028097" Received: from jekeller-desk.amr.corp.intel.com ([10.166.241.33]) by fmsmga004.fm.intel.com with ESMTP; 25 Mar 2020 20:52:11 -0700 From: Jacob Keller To: netdev@vger.kernel.org Cc: Jakub Kicinski , Jiri Pirko , Jacob Keller , Jiri Pirko Subject: [net-next v2 02/11] devlink: convert snapshot destructor callback to region op Date: Wed, 25 Mar 2020 20:51:48 -0700 Message-Id: <20200326035157.2211090-3-jacob.e.keller@intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200326035157.2211090-1-jacob.e.keller@intel.com> References: <20200326035157.2211090-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 --- 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 03:51:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacob Keller X-Patchwork-Id: 221835 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=-9.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS, 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 83CF5C43331 for ; Thu, 26 Mar 2020 03:52:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5A95820719 for ; Thu, 26 Mar 2020 03:52:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727721AbgCZDwR (ORCPT ); Wed, 25 Mar 2020 23:52:17 -0400 Received: from mga12.intel.com ([192.55.52.136]:29078 "EHLO mga12.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727705AbgCZDwP (ORCPT ); Wed, 25 Mar 2020 23:52:15 -0400 IronPort-SDR: Mqw/u1KDWIZ432Pw0an0iTlZfiSEdNDHBKYu55tKdNeeTbaR4UVWfWcW3kCu55iBISHCO+1nls BhXiWAkby8bg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Mar 2020 20:52:14 -0700 IronPort-SDR: twOKjs7AL5S+3nB3RAZS0zITxuUSaUu2NN5rzEBnvTFk3FsILPdtBSGnC9bxkBZlYK1TPuDiVp ZC0rLTIuK+rA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,306,1580803200"; d="scan'208";a="271028110" Received: from jekeller-desk.amr.corp.intel.com ([10.166.241.33]) by fmsmga004.fm.intel.com with ESMTP; 25 Mar 2020 20:52:13 -0700 From: Jacob Keller To: netdev@vger.kernel.org Cc: Jakub Kicinski , Jiri Pirko , Jacob Keller Subject: [net-next v2 06/11] devlink: extract snapshot id allocation to helper function Date: Wed, 25 Mar 2020 20:51:52 -0700 Message-Id: <20200326035157.2211090-7-jacob.e.keller@intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200326035157.2211090-1-jacob.e.keller@intel.com> References: <20200326035157.2211090-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 --- 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 03:51:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacob Keller X-Patchwork-Id: 221834 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=-9.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS, 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 02422C43331 for ; Thu, 26 Mar 2020 03:52:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D1ED020719 for ; Thu, 26 Mar 2020 03:52:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727743AbgCZDwV (ORCPT ); Wed, 25 Mar 2020 23:52:21 -0400 Received: from mga12.intel.com ([192.55.52.136]:29078 "EHLO mga12.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727666AbgCZDwQ (ORCPT ); Wed, 25 Mar 2020 23:52:16 -0400 IronPort-SDR: zEVELhQ7p9TgpyDQpXf90WY17K3mpQyjeOT/lJIlDELc6NyZqasDvKxRgdyILRMvSTfKQHVcBS elAZ78rWSNvA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Mar 2020 20:52:14 -0700 IronPort-SDR: J4d/QIZqUEJT9ihgvApT5xjVcCVhb4XEOzz4g63uylrTV27ji5FWbt5lV2lefmmXoJa3vXvADw T1sltJ1ht2Zg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,306,1580803200"; d="scan'208";a="271028115" Received: from jekeller-desk.amr.corp.intel.com ([10.166.241.33]) by fmsmga004.fm.intel.com with ESMTP; 25 Mar 2020 20:52:13 -0700 From: Jacob Keller To: netdev@vger.kernel.org Cc: Jakub Kicinski , Jiri Pirko , Jacob Keller Subject: [net-next v2 07/11] devlink: report error once U32_MAX snapshot ids have been used Date: Wed, 25 Mar 2020 20:51:53 -0700 Message-Id: <20200326035157.2211090-8-jacob.e.keller@intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200326035157.2211090-1-jacob.e.keller@intel.com> References: <20200326035157.2211090-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. Indeed, the two current callers of devlink_snapshot_id_get() assume that a negative value is an error. 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 --- 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..b851fe63a75d 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 03:51:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacob Keller X-Patchwork-Id: 221833 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=-9.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS, 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 8AF6FC2D0EA for ; Thu, 26 Mar 2020 03:52:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 604FA20719 for ; Thu, 26 Mar 2020 03:52:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727766AbgCZDwZ (ORCPT ); Wed, 25 Mar 2020 23:52:25 -0400 Received: from mga12.intel.com ([192.55.52.136]:29079 "EHLO mga12.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727708AbgCZDwQ (ORCPT ); Wed, 25 Mar 2020 23:52:16 -0400 IronPort-SDR: IQNahPEcruiR8lZiUBq8wA0W1C5yA2Waoa5pTtOBOQh49cNj0nzSIVyTbQqTDqFuVbEvMQeKN3 lsOjIAyzJzFg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Mar 2020 20:52:15 -0700 IronPort-SDR: qFuB22mdnHe+sW0ggBFJMJlPfoGXFY9I4e77jdL6ICXqjfjqY/nfeDhQfLrd2Al/MBisvChdhh MflhBvIR3YEA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,306,1580803200"; d="scan'208";a="271028122" Received: from jekeller-desk.amr.corp.intel.com ([10.166.241.33]) by fmsmga004.fm.intel.com with ESMTP; 25 Mar 2020 20:52:14 -0700 From: Jacob Keller To: netdev@vger.kernel.org Cc: Jakub Kicinski , Jiri Pirko , Jacob Keller Subject: [net-next v2 09/11] devlink: implement DEVLINK_CMD_REGION_NEW Date: Wed, 25 Mar 2020 20:51:55 -0700 Message-Id: <20200326035157.2211090-10-jacob.e.keller@intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200326035157.2211090-1-jacob.e.keller@intel.com> References: <20200326035157.2211090-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 --- .../networking/devlink/devlink-region.rst | 8 ++ include/net/devlink.h | 6 ++ net/core/devlink.c | 100 ++++++++++++++++++ 3 files changed, 114 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 d89eaac0a101..a0960ba139ec 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,72 @@ 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 snapshot_capture_failure; + + err = __devlink_region_snapshot_create(region, data, snapshot_id); + if (err) + goto snapshot_create_failure; + + return 0; + +snapshot_create_failure: + region->ops->destructor(data); +snapshot_capture_failure: + __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 +6538,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 03:51:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacob Keller X-Patchwork-Id: 221832 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=-9.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS, 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 C3283C43331 for ; Thu, 26 Mar 2020 03:52:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A49EF20719 for ; Thu, 26 Mar 2020 03:52:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727738AbgCZDwU (ORCPT ); Wed, 25 Mar 2020 23:52:20 -0400 Received: from mga12.intel.com ([192.55.52.136]:29079 "EHLO mga12.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727718AbgCZDwQ (ORCPT ); Wed, 25 Mar 2020 23:52:16 -0400 IronPort-SDR: oWeCRG2bWI3laf4x6JnWcrC91mKu0BGej4qg5Zq0FzHUgd55rK7HM9oEMMw1eOJFeaB4Txrm/G N8dcaQ2pTRFA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Mar 2020 20:52:15 -0700 IronPort-SDR: Rx64fEQiO1WJ6ZOjwZhS/7HaInYIIpLI1yLwEeab50JyvbrLyuKo/yK//bY1JcfLgEY7WG8olz UCkGlecoj4oA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,306,1580803200"; d="scan'208";a="271028126" Received: from jekeller-desk.amr.corp.intel.com ([10.166.241.33]) by fmsmga004.fm.intel.com with ESMTP; 25 Mar 2020 20:52:14 -0700 From: Jacob Keller To: netdev@vger.kernel.org Cc: Jakub Kicinski , Jiri Pirko , Jacob Keller Subject: [net-next v2 10/11] netdevsim: support taking immediate snapshot via devlink Date: Wed, 25 Mar 2020 20:51:56 -0700 Message-Id: <20200326035157.2211090-11-jacob.e.keller@intel.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200326035157.2211090-1-jacob.e.keller@intel.com> References: <20200326035157.2211090-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 --- drivers/net/netdevsim/dev.c | 30 ++++++++++++++----- .../drivers/net/netdevsim/devlink.sh | 10 +++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c index 53ec891659eb..ffc295c7653e 100644 --- a/drivers/net/netdevsim/dev.c +++ b/drivers/net/netdevsim/dev.c @@ -39,24 +39,39 @@ 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 = priv_to_devlink(nsim_dev); - void *dummy_data; + u8 *dummy_data; int err; u32 id; - 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) + if (err) { pr_err("Failed to get snapshot id\n"); return err; } @@ -350,6 +365,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" }