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@kernel.org>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/Documentation/networking/devlink/devlink-region.rst b/Documentation/networking/devlink/devlink-region.rst
index 04e04d1..daf3542 100644 (file)
--- 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 2b7c60c..43a9d5b 100644 (file)
--- 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 @@ err_snapshot_create:
 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 9f97414..ad539ec 100755 (executable)
--- a/tools/testing/selftests/drivers/net/netdevsim/devlink.sh
+++ b/tools/testing/selftests/drivers/net/netdevsim/devlink.sh
@@ -151,6 +151,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"
 }