From: Jiri Pirko <jiri@resnulli.us>
To: Jacob Keller <jacob.e.keller@intel.com>
Cc: netdev@vger.kernel.org, valex@mellanox.com,
linyunsheng@huawei.com, lihong.yang@intel.com
Subject: Re: [PATCH 01/15] devlink: prepare to support region operations
Date: Mon, 3 Feb 2020 12:35:29 +0100 [thread overview]
Message-ID: <20200203113529.GC2260@nanopsycho> (raw)
In-Reply-To: <20200130225913.1671982-2-jacob.e.keller@intel.com>
Thu, Jan 30, 2020 at 11:58:56PM CET, jacob.e.keller@intel.com wrote:
>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.
>
>Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
>---
> drivers/net/ethernet/mellanox/mlx4/crdump.c | 25 ++++++++++++---------
> drivers/net/netdevsim/dev.c | 6 ++++-
> include/net/devlink.h | 17 ++++++++++----
> net/core/devlink.c | 23 ++++++++++---------
> 4 files changed, 45 insertions(+), 26 deletions(-)
>
>diff --git a/drivers/net/ethernet/mellanox/mlx4/crdump.c b/drivers/net/ethernet/mellanox/mlx4/crdump.c
>index 64ed725aec28..4cea64033919 100644
>--- a/drivers/net/ethernet/mellanox/mlx4/crdump.c
>+++ b/drivers/net/ethernet/mellanox/mlx4/crdump.c
>@@ -38,8 +38,13 @@
> #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";
Just leave these as are and use in ops and messages. It is odd to use
ops.name in the message.
>+static const struct devlink_region_ops region_cr_space_ops = {
>+ .name = "cr-space",
>+};
>+
>+static const struct devlink_region_ops region_fw_health_ops = {
>+ .name = "fw-health",
>+};
>
> /* Set to true in case cr enable bit was set to true before crdump */
> static bool crdump_enbale_bit_set;
>@@ -103,10 +108,10 @@ static void mlx4_crdump_collect_crspace(struct mlx4_dev *dev,
> if (err) {
> kvfree(crspace_data);
> mlx4_warn(dev, "crdump: devlink create %s snapshot id %d err %d\n",
>- region_cr_space_str, id, err);
>+ region_cr_space_ops.name, id, err);
> } else {
> mlx4_info(dev, "crdump: added snapshot %d to devlink region %s\n",
>- id, region_cr_space_str);
>+ id, region_cr_space_ops.name);
> }
> } else {
> mlx4_err(dev, "crdump: Failed to allocate crspace buffer\n");
>@@ -142,10 +147,10 @@ static void mlx4_crdump_collect_fw_health(struct mlx4_dev *dev,
> if (err) {
> kvfree(health_data);
> mlx4_warn(dev, "crdump: devlink create %s snapshot id %d err %d\n",
>- region_fw_health_str, id, err);
>+ region_fw_health_ops.name, id, err);
> } else {
> mlx4_info(dev, "crdump: added snapshot %d to devlink region %s\n",
>- id, region_fw_health_str);
>+ id, region_fw_health_ops.name);
> }
> } else {
> mlx4_err(dev, "crdump: Failed to allocate health buffer\n");
>@@ -205,23 +210,23 @@ 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))
> mlx4_warn(dev, "crdump: create devlink region %s err %ld\n",
>- region_cr_space_str,
>+ region_cr_space_ops.name,
> PTR_ERR(crdump->region_crspace));
>
> /* 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))
> mlx4_warn(dev, "crdump: create devlink region %s err %ld\n",
>- region_fw_health_str,
>+ region_fw_health_ops.name,
> PTR_ERR(crdump->region_fw_health));
>
> return 0;
>diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
>index b53fbc06e104..d521b7bfe007 100644
>--- a/drivers/net/netdevsim/dev.c
>+++ b/drivers/net/netdevsim/dev.c
>@@ -242,11 +242,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 ce5cea428fdc..4a0baa6903cb 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -495,6 +495,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;
>
>@@ -949,10 +957,11 @@ 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,
No need to wrap here.
>+ 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 ca1df0ec3c97..d1f7bfbf81da 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;
>@@ -3687,7 +3687,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;
>
>@@ -3733,7 +3733,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;
>
>@@ -7530,21 +7530,22 @@ 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,
No need to wrap here.
>+ 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;
> }
>@@ -7557,7 +7558,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);
>--
>2.25.0.rc1
>
next prev parent reply other threads:[~2020-02-03 11:35 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-30 22:58 [RFC PATCH 00/13] devlink direct region reading Jacob Keller
2020-01-30 22:58 ` [PATCH 01/15] devlink: prepare to support region operations Jacob Keller
2020-01-31 18:07 ` Jakub Kicinski
2020-02-03 11:35 ` Jiri Pirko [this message]
2020-02-03 16:48 ` Jacob Keller
2020-02-03 17:07 ` Jacob Keller
2020-02-03 17:10 ` Jacob Keller
2020-02-03 17:14 ` Jacob Keller
2020-02-03 17:17 ` Jacob Keller
2020-01-30 22:58 ` [PATCH 02/15] devlink: add functions to take snapshot while locked Jacob Keller
2020-01-31 18:07 ` Jakub Kicinski
2020-01-31 18:09 ` Jacob Keller
2020-02-03 11:39 ` Jiri Pirko
2020-02-03 16:45 ` Jacob Keller
2020-01-30 22:58 ` [PATCH 03/15] devlink: add operation to take an immediate snapshot Jacob Keller
2020-01-31 18:07 ` Jakub Kicinski
2020-02-03 8:19 ` Yunsheng Lin
2020-02-03 11:50 ` Jiri Pirko
2020-02-03 11:50 ` Jiri Pirko
2020-02-03 16:33 ` Jacob Keller
2020-02-03 19:32 ` Jacob Keller
2020-02-03 21:30 ` Jiri Pirko
2020-02-04 19:20 ` Jacob Keller
2020-01-30 22:58 ` [PATCH 04/15] netdevsim: support taking immediate snapshot via devlink Jacob Keller
2020-01-31 18:07 ` Jakub Kicinski
2020-01-31 18:12 ` Jacob Keller
2020-01-30 22:59 ` [PATCH 05/15] ice: use __le16 types for explicitly Little Endian values Jacob Keller
2020-01-30 22:59 ` [PATCH 06/15] ice: create function to read a section of the NVM and Shadow RAM Jacob Keller
2020-01-30 22:59 ` [PATCH 07/15] ice: implement full NVM read from ETHTOOL_GEEPROM Jacob Keller
2020-01-30 22:59 ` [PATCH 08/15] devlink: add devres managed devlinkm_alloc and devlinkm_free Jacob Keller
2020-01-31 18:07 ` Jakub Kicinski
2020-01-31 18:16 ` Jacob Keller
2020-01-31 18:07 ` Jakub Kicinski
2020-02-01 0:51 ` Jacob Keller
2020-02-01 17:43 ` Jakub Kicinski
2020-02-03 16:35 ` Jacob Keller
2020-02-03 11:29 ` Jiri Pirko
2020-02-03 16:56 ` Jacob Keller
2020-01-30 22:59 ` [PATCH 09/15] ice: enable initial devlink support Jacob Keller
2020-01-30 22:59 ` [PATCH 10/15] ice: add basic handler for devlink .info_get Jacob Keller
2020-01-31 18:07 ` Jakub Kicinski
2020-01-31 18:25 ` Jacob Keller
2020-01-30 22:59 ` [PATCH 11/15] ice: add board identifier info to " Jacob Keller
2020-01-31 18:07 ` Jakub Kicinski
2020-01-31 18:26 ` Jacob Keller
2020-01-30 22:59 ` [PATCH 12/15] ice: add a devlink region to dump shadow RAM contents Jacob Keller
2020-01-30 22:59 ` [PATCH 13/15] devlink: support directly reading from region memory Jacob Keller
2020-01-31 18:07 ` Jakub Kicinski
2020-01-31 18:27 ` Jacob Keller
2020-01-31 19:15 ` Jacob Keller
2020-02-03 13:44 ` Jiri Pirko
2020-02-03 16:40 ` Jacob Keller
2020-01-30 22:59 ` [PATCH 14/15] ice: support direct read of the shadow ram region Jacob Keller
2020-01-30 22:59 ` [PATCH 15/15] ice: add ice.rst devlink documentation file Jacob Keller
2020-01-31 18:07 ` Jakub Kicinski
2020-01-31 18:28 ` Jacob Keller
2020-01-30 22:59 ` [RFC PATCH 1/3] Update kernel headers Jacob Keller
2020-01-30 22:59 ` [RFC PATCH 2/3] devlink: add support for DEVLINK_CMD_REGION_TAKE_SNAPSHOT Jacob Keller
2020-01-30 22:59 ` [RFC PATCH 3/3] devlink: stop requiring snapshot for regions Jacob Keller
2020-01-30 23:03 ` [RFC PATCH 00/13] devlink direct region reading Jacob Keller
2020-01-31 18:06 ` Jakub Kicinski
2020-01-31 18:09 ` Jacob Keller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200203113529.GC2260@nanopsycho \
--to=jiri@resnulli.us \
--cc=jacob.e.keller@intel.com \
--cc=lihong.yang@intel.com \
--cc=linyunsheng@huawei.com \
--cc=netdev@vger.kernel.org \
--cc=valex@mellanox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).