devlink: restart dump based on devlink instance ids (simple)

xarray gives each devlink instance an id and allows us to restart
walk based on that id quite neatly. This is nice both from the
perspective of code brevity and from the stability of the dump
(devlink instances disappearing from before the resumption point
will not cause inconsistent dumps).

This patch takes care of simple cases where state->idx counts
devlink instances only.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/devlink/core.c b/net/devlink/core.c
index 3a99bf84632e564ecbc40441d0198632c569a4ce..371d6821315de658a2e9195f33a3c5aa43495d3b 100644 (file)
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -91,7 +91,7 @@ void devlink_put(struct devlink *devlink)
                call_rcu(&devlink->rcu, __devlink_put_rcu);
 }
 
-static struct devlink *
+struct devlink *
 devlinks_xa_find_get(struct net *net, unsigned long *indexp,
                     void * (*xa_find_fn)(struct xarray *, unsigned long *,
                                          unsigned long, xa_mark_t))

diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
index 86b321aef3ebca66b474d4af640fef3ae05dff31..b9bb6f73a7c184c3e45e277157803ba33b667504 100644 (file)
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -87,6 +87,10 @@ extern struct genl_family devlink_nl_family;
             devlink; devlink = devlinks_xa_find_get_next(net, &index))
 
 struct devlink *
+devlinks_xa_find_get(struct net *net, unsigned long *indexp,
+                    void * (*xa_find_fn)(struct xarray *, unsigned long *,
+                                         unsigned long, xa_mark_t));
+struct devlink *
 devlinks_xa_find_get_first(struct net *net, unsigned long *indexp);
 struct devlink *
 devlinks_xa_find_get_next(struct net *net, unsigned long *indexp);
@@ -104,6 +108,7 @@ enum devlink_multicast_groups {
 
 /* state held across netlink dumps */
 struct devlink_nl_dump_state {
+       unsigned long instance;
        int idx;
        union {
                /* DEVLINK_CMD_REGION_READ */
@@ -117,6 +122,17 @@ struct devlink_nl_dump_state {
        };
 };
 
+/* Iterate over registered devlink instances for devlink dump.
+ * devlink_put() needs to be called for each iterated devlink pointer
+ * in loop body in order to release the reference.
+ * Note: this is NOT a generic iterator, it makes assumptions about the use
+ *      of @state and can only be used once per dumpit implementation.
+ */
+#define devlink_dump_for_each_instance_get(msg, state, devlink)                \
+       for (; (devlink = devlinks_xa_find_get(sock_net(msg->sk),       \
+                                              &state->instance, xa_find)); \
+            state->instance++)
+
 extern const struct genl_small_ops devlink_nl_ops[56];
 
 struct devlink *devlink_get_from_attrs(struct net *net, struct nlattr **attrs);

diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index 83cd7bd55941b54f7c9c9bb0b8cbd4e8844a95e2..db7c095a0d75c2f2ef209b097468afd0b6a73ac7 100644 (file)
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -1319,17 +1319,9 @@ static int devlink_nl_cmd_get_dumpit(struct sk_buff *msg,
 {
        struct devlink_nl_dump_state *state = devlink_dump_state(cb);
        struct devlink *devlink;
-       unsigned long index;
-       int idx = 0;
        int err;
 
-       devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
-               if (idx < state->idx) {
-                       idx++;
-                       devlink_put(devlink);
-                       continue;
-               }
-
+       devlink_dump_for_each_instance_get(msg, state, devlink) {
                devl_lock(devlink);
                err = devlink_nl_fill(msg, devlink, DEVLINK_CMD_NEW,
                                      NETLINK_CB(cb->skb).portid,
@@ -1339,10 +1331,8 @@ static int devlink_nl_cmd_get_dumpit(struct sk_buff *msg,
 
                if (err)
                        goto out;
-               idx++;
        }
 out:
-       state->idx = idx;
        return msg->len;
 }
 
@@ -4872,13 +4862,13 @@ static int devlink_nl_cmd_selftests_get_dumpit(struct sk_buff *msg,
 {
        struct devlink_nl_dump_state *state = devlink_dump_state(cb);
        struct devlink *devlink;
-       unsigned long index;
-       int idx = 0;
        int err = 0;
 
-       devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
-               if (idx < state->idx || !devlink->ops->selftest_check)
-                       goto inc;
+       devlink_dump_for_each_instance_get(msg, state, devlink) {
+               if (!devlink->ops->selftest_check) {
+                       devlink_put(devlink);
+                       continue;
+               }
 
                devl_lock(devlink);
                err = devlink_nl_selftests_fill(msg, devlink,
@@ -4890,15 +4880,13 @@ static int devlink_nl_cmd_selftests_get_dumpit(struct sk_buff *msg,
                        devlink_put(devlink);
                        break;
                }
-inc:
-               idx++;
+
                devlink_put(devlink);
        }
 
        if (err != -EMSGSIZE)
                return err;
 
-       state->idx = idx;
        return msg->len;
 }
 
@@ -6747,14 +6735,9 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
 {
        struct devlink_nl_dump_state *state = devlink_dump_state(cb);
        struct devlink *devlink;
-       unsigned long index;
-       int idx = 0;
        int err = 0;
 
-       devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
-               if (idx < state->idx)
-                       goto inc;
-
+       devlink_dump_for_each_instance_get(msg, state, devlink) {
                devl_lock(devlink);
                err = devlink_nl_info_fill(msg, devlink, DEVLINK_CMD_INFO_GET,
                                           NETLINK_CB(cb->skb).portid,
@@ -6767,15 +6750,12 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
                        devlink_put(devlink);
                        break;
                }
-inc:
-               idx++;
                devlink_put(devlink);
        }
 
        if (err != -EMSGSIZE)
                return err;
 
-       state->idx = idx;
        return msg->len;
 }