[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20200501164042.1430604-3-kuba@kernel.org>
Date: Fri, 1 May 2020 09:40:41 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: davem@...emloft.net, jiri@...nulli.us
Cc: netdev@...r.kernel.org, kernel-team@...com,
jacob.e.keller@...el.com, Jakub Kicinski <kuba@...nel.org>
Subject: [PATCH net-next v4 2/3] devlink: let kernel allocate region snapshot id
Currently users have to choose a free snapshot id before
calling DEVLINK_CMD_REGION_NEW. This is potentially racy
and inconvenient.
Make the DEVLINK_ATTR_REGION_SNAPSHOT_ID optional and try
to allocate id automatically. Send a message back to the
caller with the snapshot info.
Example use:
$ devlink region new netdevsim/netdevsim1/dummy
netdevsim/netdevsim1/dummy: snapshot 1
$ id=$(devlink -j region new netdevsim/netdevsim1/dummy | \
jq '.[][][][]')
$ devlink region dump netdevsim/netdevsim1/dummy snapshot $id
[...]
$ devlink region del netdevsim/netdevsim1/dummy snapshot $id
v4:
- inline the notification code
v3:
- send the notification only once snapshot creation completed.
v2:
- don't wrap the line containing extack;
- add a few sentences to the docs.
Signed-off-by: Jakub Kicinski <kuba@...nel.org>
---
.../networking/devlink/devlink-region.rst | 7 ++-
net/core/devlink.c | 57 ++++++++++++++-----
.../drivers/net/netdevsim/devlink.sh | 13 +++++
3 files changed, 62 insertions(+), 15 deletions(-)
diff --git a/Documentation/networking/devlink/devlink-region.rst b/Documentation/networking/devlink/devlink-region.rst
index 04e04d1ff627..daf35427fce1 100644
--- a/Documentation/networking/devlink/devlink-region.rst
+++ b/Documentation/networking/devlink/devlink-region.rst
@@ -23,7 +23,9 @@ 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.
+in its ``devlink_region_ops`` structure. If snapshot id is not set in
+the ``DEVLINK_CMD_REGION_NEW`` request kernel will allocate one and send
+the snapshot information to user space.
example usage
-------------
@@ -45,7 +47,8 @@ example usage
$ 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
+ $ devlink region new pci/0000:00:05.0/cr-space
+ pci/0000:00:05.0/cr-space: snapshot 5
# Dump a snapshot:
$ devlink region dump pci/0000:00:05.0/fw-health snapshot 1
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 92afb85bad89..cf3084208724 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -4086,6 +4086,8 @@ static int
devlink_nl_cmd_region_new(struct sk_buff *skb, struct genl_info *info)
{
struct devlink *devlink = info->user_ptr[0];
+ struct devlink_snapshot *snapshot;
+ struct nlattr *snapshot_id_attr;
struct devlink_region *region;
const char *region_name;
u32 snapshot_id;
@@ -4097,11 +4099,6 @@ devlink_nl_cmd_region_new(struct sk_buff *skb, struct genl_info *info)
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) {
@@ -4119,16 +4116,25 @@ devlink_nl_cmd_region_new(struct sk_buff *skb, struct genl_info *info)
return -ENOSPC;
}
- snapshot_id = nla_get_u32(info->attrs[DEVLINK_ATTR_REGION_SNAPSHOT_ID]);
+ snapshot_id_attr = info->attrs[DEVLINK_ATTR_REGION_SNAPSHOT_ID];
+ if (snapshot_id_attr) {
+ snapshot_id = nla_get_u32(snapshot_id_attr);
- 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;
- }
+ 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 = __devlink_snapshot_id_insert(devlink, snapshot_id);
+ if (err)
+ return err;
+ } else {
+ err = __devlink_region_snapshot_id_get(devlink, &snapshot_id);
+ if (err) {
+ NL_SET_ERR_MSG_MOD(info->extack, "Failed to allocate a new snapshot id");
+ return err;
+ }
+ }
err = region->ops->snapshot(devlink, info->extack, &data);
if (err)
@@ -4138,6 +4144,27 @@ devlink_nl_cmd_region_new(struct sk_buff *skb, struct genl_info *info)
if (err)
goto err_snapshot_create;
+ if (!snapshot_id_attr) {
+ struct sk_buff *msg;
+
+ snapshot = devlink_region_snapshot_get_by_id(region,
+ snapshot_id);
+ if (WARN_ON(!snapshot))
+ return -EINVAL;
+
+ msg = devlink_nl_region_notify_build(region, snapshot,
+ DEVLINK_CMD_REGION_NEW,
+ info->snd_portid,
+ info->snd_seq);
+ err = PTR_ERR_OR_ZERO(msg);
+ if (err)
+ goto err_notify;
+
+ err = genlmsg_reply(msg, info);
+ if (err)
+ goto err_notify;
+ }
+
return 0;
err_snapshot_create:
@@ -4145,6 +4172,10 @@ devlink_nl_cmd_region_new(struct sk_buff *skb, struct genl_info *info)
err_snapshot_capture:
__devlink_snapshot_id_decrement(devlink, snapshot_id);
return err;
+
+err_notify:
+ devlink_region_snapshot_del(region, snapshot);
+ return err;
}
static int devlink_nl_cmd_region_read_chunk_fill(struct sk_buff *msg,
diff --git a/tools/testing/selftests/drivers/net/netdevsim/devlink.sh b/tools/testing/selftests/drivers/net/netdevsim/devlink.sh
index 1d15afd62941..de4b32fc4223 100755
--- a/tools/testing/selftests/drivers/net/netdevsim/devlink.sh
+++ b/tools/testing/selftests/drivers/net/netdevsim/devlink.sh
@@ -166,6 +166,19 @@ regions_test()
check_region_snapshot_count dummy post-second-delete 2
+ sid=$(devlink -j region new $DL_HANDLE/dummy | jq '.[][][][]')
+ check_err $? "Failed to create a new snapshot with id allocated by the kernel"
+
+ check_region_snapshot_count dummy post-first-request 3
+
+ devlink region dump $DL_HANDLE/dummy snapshot $sid >> /dev/null
+ check_err $? "Failed to dump a snapshot with id allocated by the kernel"
+
+ devlink region del $DL_HANDLE/dummy snapshot $sid
+ check_err $? "Failed to delete snapshot with id allocated by the kernel"
+
+ check_region_snapshot_count dummy post-first-request 2
+
log_test "regions test"
}
--
2.25.4
Powered by blists - more mailing lists