Thread (23 messages) 23 messages, 4 authors, 2024-03-04

Re: [PATCH net-next v2 4/7] net: nexthop: Expose nexthop group stats to user space

From: Eric Dumazet <edumazet@google.com>
Date: 2024-03-01 17:32:41

On Thu, Feb 29, 2024 at 7:20 PM Petr Machata [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Ido Schimmel <idosch@nvidia.com>

Add netlink support for reading NH group stats.

This data is only for statistics of the traffic in the SW datapath. HW
nexthop group statistics will be added in the following patches.

Emission of the stats is keyed to a new op_stats flag to avoid cluttering
the netlink message with stats if the user doesn't need them:
NHA_OP_FLAG_DUMP_STATS.

Co-developed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---

Notes:
    v2:
    - Use uint to encode NHA_GROUP_STATS_ENTRY_PACKETS
    - Rename jump target in nla_put_nh_group_stats() to avoid
      having to rename further in the patchset.

 include/uapi/linux/nexthop.h | 30 ++++++++++++
 net/ipv4/nexthop.c           | 92 ++++++++++++++++++++++++++++++++----
 2 files changed, 114 insertions(+), 8 deletions(-)
diff --git a/include/uapi/linux/nexthop.h b/include/uapi/linux/nexthop.h
index 086444e2946c..f4db63c17085 100644
--- a/include/uapi/linux/nexthop.h
+++ b/include/uapi/linux/nexthop.h
@@ -30,6 +30,8 @@ enum {

 #define NEXTHOP_GRP_TYPE_MAX (__NEXTHOP_GRP_TYPE_MAX - 1)

+#define NHA_OP_FLAG_DUMP_STATS         BIT(0)
+
 enum {
        NHA_UNSPEC,
        NHA_ID,         /* u32; id for nexthop. id == 0 means auto-assign */
@@ -63,6 +65,9 @@ enum {
        /* u32; operation-specific flags */
        NHA_OP_FLAGS,

+       /* nested; nexthop group stats */
+       NHA_GROUP_STATS,
+
        __NHA_MAX,
 };
@@ -104,4 +109,29 @@ enum {

 #define NHA_RES_BUCKET_MAX     (__NHA_RES_BUCKET_MAX - 1)

+enum {
+       NHA_GROUP_STATS_UNSPEC,
+
+       /* nested; nexthop group entry stats */
+       NHA_GROUP_STATS_ENTRY,
+
+       __NHA_GROUP_STATS_MAX,
+};
+
+#define NHA_GROUP_STATS_MAX    (__NHA_GROUP_STATS_MAX - 1)
+
+enum {
+       NHA_GROUP_STATS_ENTRY_UNSPEC,
+
+       /* u32; nexthop id of the nexthop group entry */
+       NHA_GROUP_STATS_ENTRY_ID,
+
+       /* uint; number of packets forwarded via the nexthop group entry */
+       NHA_GROUP_STATS_ENTRY_PACKETS,
+
+       __NHA_GROUP_STATS_ENTRY_MAX,
+};
+
+#define NHA_GROUP_STATS_ENTRY_MAX      (__NHA_GROUP_STATS_ENTRY_MAX - 1)
+
 #endif
diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index 4be66622e24f..0ede8777bd66 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -41,7 +41,8 @@ static const struct nla_policy rtm_nh_policy_new[] = {

 static const struct nla_policy rtm_nh_policy_get[] = {
        [NHA_ID]                = { .type = NLA_U32 },
-       [NHA_OP_FLAGS]          = NLA_POLICY_MASK(NLA_U32, 0),
+       [NHA_OP_FLAGS]          = NLA_POLICY_MASK(NLA_U32,
+                                                 NHA_OP_FLAG_DUMP_STATS),
 };

 static const struct nla_policy rtm_nh_policy_del[] = {
@@ -53,7 +54,8 @@ static const struct nla_policy rtm_nh_policy_dump[] = {
        [NHA_GROUPS]            = { .type = NLA_FLAG },
        [NHA_MASTER]            = { .type = NLA_U32 },
        [NHA_FDB]               = { .type = NLA_FLAG },
-       [NHA_OP_FLAGS]          = NLA_POLICY_MASK(NLA_U32, 0),
+       [NHA_OP_FLAGS]          = NLA_POLICY_MASK(NLA_U32,
+                                                 NHA_OP_FLAG_DUMP_STATS),
 };

 static const struct nla_policy rtm_nh_res_policy_new[] = {
@@ -661,8 +663,77 @@ static int nla_put_nh_group_res(struct sk_buff *skb, struct nh_group *nhg)
        return -EMSGSIZE;
 }

-static int nla_put_nh_group(struct sk_buff *skb, struct nh_group *nhg)
+static void nh_grp_entry_stats_read(struct nh_grp_entry *nhge,
+                                   struct nh_grp_entry_stats *stats)
 {
+       int i;
+
+       memset(stats, 0, sizeof(*stats));
+       for_each_possible_cpu(i) {
+               struct nh_grp_entry_stats *cpu_stats;
+               unsigned int start;
+               u64 packets;
+
+               cpu_stats = per_cpu_ptr(nhge->stats, i);
+               do {
+                       start = u64_stats_fetch_begin(&cpu_stats->syncp);
+                       packets = cpu_stats->packets;
This is not safe, even on 64bit arches.

You should use u64_stats_t, u64_stats_read(), u64_stats_add() ...
+               } while (u64_stats_fetch_retry(&cpu_stats->syncp, start));
+
+               stats->packets += packets;
+       }
+}
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help