[PATCH iproute2-next v2 0/4] Support for nexthop group statistics

[PATCH iproute2-next v2 0/4] Support for nexthop group statistics

[Petr Machata]

Next hop group stats allow verification of balancedness of a next hop
group. The feature was merged in kernel commit 7cf497e5a122 ("Merge branch
'nexthop-group-stats'"). This patchset adds to ip the corresponding
support.

NH group stats come in two flavors: as statistics for SW and for HW
datapaths. The former is shown when -s is given to "ip nexthop". The latter
demands more work from the kernel, and possibly driver and HW, and might
not be always necessary. Therefore tie it to -s -s, similarly to how ip
link shows more detailed stats when -s is given twice.

Here's an example usage:

 # ip link add name gre1 up type gre \
      local 172.16.1.1 remote 172.16.1.2 tos inherit
 # ip nexthop replace id 1001 dev gre1
 # ip nexthop replace id 1002 dev gre1
 # ip nexthop replace id 1111 group 1001/1002 hw_stats on
 # ip -s -s -j -p nexthop show id 1111
 [ {
 	[ ...snip... ]
         "hw_stats": {
             "enabled": true,
             "used": true
         },
         "group_stats": [ {
                 "id": 1001,
                 "packets": 0,
                 "packets_hw": 0
             },{
                 "id": 1002,
                 "packets": 0,
                 "packets_hw": 0
             } ]
     } ]

hw_stats.enabled shows whether hw_stats have been requested for the given
group. hw_stats.used shows whether any driver actually implemented the
counter. group_stats[].packets show the total stats, packets_hw only the
HW-datapath stats.

v2:
- Patch #1:
    - have rta_getattr_uint() support 8- and 16-bit quantities as well
- Patch #2:
    - Use print_nl() for the newlines

Petr Machata (4):
  libnetlink: Add rta_getattr_uint()
  ip: ipnexthop: Support dumping next hop group stats
  ip: ipnexthop: Support dumping next hop group HW stats
  ip: ipnexthop: Allow toggling collection of nexthop group HW
    statistics

 include/libnetlink.h  |  14 +++++
 ip/ipnexthop.c        | 129 ++++++++++++++++++++++++++++++++++++++++++
 ip/nh_common.h        |  11 ++++
 man/man8/ip-nexthop.8 |   2 +
 4 files changed, 156 insertions(+)

-- 
2.43.0

[PATCH iproute2-next v2 1/4] libnetlink: Add rta_getattr_uint()

[Petr Machata]

NLA_UINT attributes have a 4-byte payload if possible, and an 8-byte one if
necessary. Add a function to extract these. Since we need to dispatch on
length anyway, make the getter truly universal by supporting also u8 and
u16.

Signed-off-by: Petr Machata <petrm@nvidia.com>
---

Notes:
    v2:
    - have rta_getattr_uint() support 8- and 16-bit quantities as well

 include/libnetlink.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/libnetlink.h b/include/libnetlink.h
index ad7e7127..35a9bb57 100644
--- a/include/libnetlink.h
+++ b/include/libnetlink.h
@@ -260,6 +260,20 @@ static inline __u64 rta_getattr_u64(const struct rtattr *rta)
 	memcpy(&tmp, RTA_DATA(rta), sizeof(__u64));
 	return tmp;
 }
+static inline __u64 rta_getattr_uint(const struct rtattr *rta)
+{
+	switch (RTA_PAYLOAD(rta)) {
+	case sizeof(__u8):
+		return rta_getattr_u8(rta);
+	case sizeof(__u16):
+		return rta_getattr_u16(rta);
+	case sizeof(__u32):
+		return rta_getattr_u32(rta);
+	case sizeof(__u64):
+		return rta_getattr_u64(rta);
+	}
+	return -1ULL;
+}
 static inline __s32 rta_getattr_s32(const struct rtattr *rta)
 {
 	return *(__s32 *)RTA_DATA(rta);
-- 
2.43.0

[PATCH iproute2-next v2 2/4] ip: ipnexthop: Support dumping next hop group stats

[Petr Machata]

Next hop group stats allow verification of balancedness of a next hop
group. The feature was merged in kernel commit 7cf497e5a122 ("Merge branch
'nexthop-group-stats'"). Add to ip the corresponding support. The
statistics are requested if "ip nexthop" is started with -s.

Signed-off-by: Petr Machata <petrm@nvidia.com>
---

Notes:
    v2:
    - Use print_nl() for the newlines

 ip/ipnexthop.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++
 ip/nh_common.h |  6 ++++
 2 files changed, 95 insertions(+)

diff --git a/ip/ipnexthop.c b/ip/ipnexthop.c
index e946d6f9..cba3d934 100644
--- a/ip/ipnexthop.c
+++ b/ip/ipnexthop.c
@@ -25,6 +25,7 @@ static struct {
 	unsigned int fdb;
 	unsigned int id;
 	unsigned int nhid;
+	unsigned int op_flags;
 } filter;
 
 enum {
@@ -92,6 +93,14 @@ static int nh_dump_filter(struct nlmsghdr *nlh, int reqlen)
 			return err;
 	}
 
+	if (filter.op_flags) {
+		__u32 op_flags = filter.op_flags;
+
+		err = addattr32(nlh, reqlen, NHA_OP_FLAGS, op_flags);
+		if (err)
+			return err;
+	}
+
 	return 0;
 }
 
@@ -296,6 +305,31 @@ static void parse_nh_res_group_rta(const struct rtattr *res_grp_attr,
 	}
 }
 
+static void parse_nh_group_stats_rta(const struct rtattr *grp_stats_attr,
+				     struct nh_entry *nhe)
+{
+	const struct rtattr *pos;
+	int i = 0;
+
+	rtattr_for_each_nested(pos, grp_stats_attr) {
+		struct nh_grp_stats *nh_grp_stats = &nhe->nh_grp_stats[i++];
+		struct rtattr *tb[NHA_GROUP_STATS_ENTRY_MAX + 1];
+		struct rtattr *rta;
+
+		parse_rtattr_nested(tb, NHA_GROUP_STATS_ENTRY_MAX, pos);
+
+		if (tb[NHA_GROUP_STATS_ENTRY_ID]) {
+			rta = tb[NHA_GROUP_STATS_ENTRY_ID];
+			nh_grp_stats->nh_id = rta_getattr_u32(rta);
+		}
+
+		if (tb[NHA_GROUP_STATS_ENTRY_PACKETS]) {
+			rta = tb[NHA_GROUP_STATS_ENTRY_PACKETS];
+			nh_grp_stats->packets = rta_getattr_uint(rta);
+		}
+	}
+}
+
 static void print_nh_res_group(const struct nha_res_grp *res_grp)
 {
 	struct timeval tv;
@@ -343,8 +377,35 @@ static void print_nh_res_bucket(FILE *fp, const struct rtattr *res_bucket_attr)
 	close_json_object();
 }
 
+static void print_nh_grp_stats(const struct nh_entry *nhe)
+{
+	int i;
+
+	if (!show_stats)
+		return;
+
+	open_json_array(PRINT_JSON, "group_stats");
+	print_nl();
+	print_string(PRINT_FP, NULL, "  stats:", NULL);
+	print_nl();
+	for (i = 0; i < nhe->nh_groups_cnt; i++) {
+		open_json_object(NULL);
+
+		print_uint(PRINT_ANY, "id", "    id %u",
+			   nhe->nh_grp_stats[i].nh_id);
+		print_u64(PRINT_ANY, "packets", " packets %llu",
+			  nhe->nh_grp_stats[i].packets);
+
+		if (i != nhe->nh_groups_cnt - 1)
+			print_nl();
+		close_json_object();
+	}
+	close_json_array(PRINT_JSON, NULL);
+}
+
 static void ipnh_destroy_entry(struct nh_entry *nhe)
 {
+	free(nhe->nh_grp_stats);
 	free(nhe->nh_encap);
 	free(nhe->nh_groups);
 }
@@ -418,6 +479,16 @@ static int ipnh_parse_nhmsg(FILE *fp, const struct nhmsg *nhm, int len,
 		nhe->nh_has_res_grp = true;
 	}
 
+	if (tb[NHA_GROUP_STATS]) {
+		nhe->nh_grp_stats = calloc(nhe->nh_groups_cnt,
+					   sizeof(*nhe->nh_grp_stats));
+		if (!nhe->nh_grp_stats) {
+			err = -ENOMEM;
+			goto out_err;
+		}
+		parse_nh_group_stats_rta(tb[NHA_GROUP_STATS], nhe);
+	}
+
 	nhe->nh_blackhole = !!tb[NHA_BLACKHOLE];
 	nhe->nh_fdb = !!tb[NHA_FDB];
 
@@ -484,9 +555,23 @@ static void __print_nexthop_entry(FILE *fp, const char *jsobj,
 	if (nhe->nh_fdb)
 		print_null(PRINT_ANY, "fdb", "fdb", NULL);
 
+	if (nhe->nh_grp_stats)
+		print_nh_grp_stats(nhe);
+
 	close_json_object();
 }
 
+static __u32 ipnh_get_op_flags(void)
+{
+	__u32 op_flags = 0;
+
+	if (show_stats) {
+		op_flags |= NHA_OP_FLAG_DUMP_STATS;
+	}
+
+	return op_flags;
+}
+
 static int  __ipnh_get_id(struct rtnl_handle *rthp, __u32 nh_id,
 			  struct nlmsghdr **answer)
 {
@@ -500,8 +585,10 @@ static int  __ipnh_get_id(struct rtnl_handle *rthp, __u32 nh_id,
 		.n.nlmsg_type	= RTM_GETNEXTHOP,
 		.nhm.nh_family	= preferred_family,
 	};
+	__u32 op_flags = ipnh_get_op_flags();
 
 	addattr32(&req.n, sizeof(req), NHA_ID, nh_id);
+	addattr32(&req.n, sizeof(req), NHA_OP_FLAGS, op_flags);
 
 	return rtnl_talk(rthp, &req.n, answer);
 }
@@ -1093,6 +1180,8 @@ static int ipnh_list_flush(int argc, char **argv, int action)
 		argc--; argv++;
 	}
 
+	filter.op_flags = ipnh_get_op_flags();
+
 	if (action == IPNH_FLUSH)
 		return ipnh_flush(all);
 
diff --git a/ip/nh_common.h b/ip/nh_common.h
index 4d6677e6..e2f74ec5 100644
--- a/ip/nh_common.h
+++ b/ip/nh_common.h
@@ -13,6 +13,11 @@ struct nha_res_grp {
 	__u64			unbalanced_time;
 };
 
+struct nh_grp_stats {
+	__u32			nh_id;
+	__u64			packets;
+};
+
 struct nh_entry {
 	struct hlist_node	nh_hash;
 
@@ -44,6 +49,7 @@ struct nh_entry {
 
 	int			nh_groups_cnt;
 	struct nexthop_grp	*nh_groups;
+	struct nh_grp_stats	*nh_grp_stats;
 };
 
 void print_cache_nexthop_id(FILE *fp, const char *fp_prefix, const char *jsobj,
-- 
2.43.0

[PATCH iproute2-next v2 3/4] ip: ipnexthop: Support dumping next hop group HW stats

[Petr Machata]

Besides SW datapath stats, the kernel also support collecting statistics
from HW datapath, for nexthop groups offloaded to HW. Request that these be
collected when ip is given "-s -s", similarly to how "ip link" shows more
statistics in that case.

Besides the statistics themselves, also show whether the collection of HW
statistics was in fact requested, and whether any driver actually
implemented the request.

Signed-off-by: Petr Machata <petrm@nvidia.com>
---
 ip/ipnexthop.c | 28 ++++++++++++++++++++++++++++
 ip/nh_common.h |  5 +++++
 2 files changed, 33 insertions(+)

diff --git a/ip/ipnexthop.c b/ip/ipnexthop.c
index cba3d934..6c5d524b 100644
--- a/ip/ipnexthop.c
+++ b/ip/ipnexthop.c
@@ -327,6 +327,11 @@ static void parse_nh_group_stats_rta(const struct rtattr *grp_stats_attr,
 			rta = tb[NHA_GROUP_STATS_ENTRY_PACKETS];
 			nh_grp_stats->packets = rta_getattr_uint(rta);
 		}
+
+		if (tb[NHA_GROUP_STATS_ENTRY_PACKETS_HW]) {
+			rta = tb[NHA_GROUP_STATS_ENTRY_PACKETS_HW];
+			nh_grp_stats->packets_hw = rta_getattr_uint(rta);
+		}
 	}
 }
 
@@ -395,6 +400,9 @@ static void print_nh_grp_stats(const struct nh_entry *nhe)
 			   nhe->nh_grp_stats[i].nh_id);
 		print_u64(PRINT_ANY, "packets", " packets %llu",
 			  nhe->nh_grp_stats[i].packets);
+		if (show_stats > 1)
+			print_u64(PRINT_ANY, "packets_hw", " packets_hw %llu",
+				  nhe->nh_grp_stats[i].packets_hw);
 
 		if (i != nhe->nh_groups_cnt - 1)
 			print_nl();
@@ -479,6 +487,15 @@ static int ipnh_parse_nhmsg(FILE *fp, const struct nhmsg *nhm, int len,
 		nhe->nh_has_res_grp = true;
 	}
 
+	if (tb[NHA_HW_STATS_ENABLE]) {
+		nhe->nh_hw_stats_supported = true;
+		nhe->nh_hw_stats_enabled =
+			!!rta_getattr_u32(tb[NHA_HW_STATS_ENABLE]);
+	}
+
+	if (tb[NHA_HW_STATS_USED])
+		nhe->nh_hw_stats_used = !!rta_getattr_u32(tb[NHA_HW_STATS_USED]);
+
 	if (tb[NHA_GROUP_STATS]) {
 		nhe->nh_grp_stats = calloc(nhe->nh_groups_cnt,
 					   sizeof(*nhe->nh_grp_stats));
@@ -555,6 +572,15 @@ static void __print_nexthop_entry(FILE *fp, const char *jsobj,
 	if (nhe->nh_fdb)
 		print_null(PRINT_ANY, "fdb", "fdb", NULL);
 
+	if ((show_details > 0 || show_stats) && nhe->nh_hw_stats_supported) {
+		open_json_object("hw_stats");
+		print_on_off(PRINT_ANY, "enabled", "hw_stats %s ",
+			     nhe->nh_hw_stats_enabled);
+		print_on_off(PRINT_ANY, "used", "used %s ",
+			     nhe->nh_hw_stats_used);
+		close_json_object();
+	}
+
 	if (nhe->nh_grp_stats)
 		print_nh_grp_stats(nhe);
 
@@ -567,6 +593,8 @@ static __u32 ipnh_get_op_flags(void)
 
 	if (show_stats) {
 		op_flags |= NHA_OP_FLAG_DUMP_STATS;
+		if (show_stats > 1)
+			op_flags |= NHA_OP_FLAG_DUMP_HW_STATS;
 	}
 
 	return op_flags;
diff --git a/ip/nh_common.h b/ip/nh_common.h
index e2f74ec5..33b1d847 100644
--- a/ip/nh_common.h
+++ b/ip/nh_common.h
@@ -16,6 +16,7 @@ struct nha_res_grp {
 struct nh_grp_stats {
 	__u32			nh_id;
 	__u64			packets;
+	__u64			packets_hw;
 };
 
 struct nh_entry {
@@ -32,6 +33,10 @@ struct nh_entry {
 	bool			nh_blackhole;
 	bool			nh_fdb;
 
+	bool			nh_hw_stats_supported;
+	bool			nh_hw_stats_enabled;
+	bool			nh_hw_stats_used;
+
 	int			nh_gateway_len;
 	union {
 		__be32		ipv4;
-- 
2.43.0

[PATCH iproute2-next v2 4/4] ip: ipnexthop: Allow toggling collection of nexthop group HW statistics

[Petr Machata]

Besides SW datapath stats, the kernel also support collecting statistics
from HW datapath, for nexthop groups offloaded to HW. Since collection of
these statistics may consume HW resources, there is an interface to request
that the HW stats be recorded. Add this toggle to "ip nexthop".

Signed-off-by: Petr Machata <petrm@nvidia.com>
---
 ip/ipnexthop.c        | 12 ++++++++++++
 man/man8/ip-nexthop.8 |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/ip/ipnexthop.c b/ip/ipnexthop.c
index 6c5d524b..8aa06de0 100644
--- a/ip/ipnexthop.c
+++ b/ip/ipnexthop.c
@@ -56,6 +56,7 @@ static void usage(void)
 		"        [ encap ENCAPTYPE ENCAPHDR ] |\n"
 		"        group GROUP [ fdb ] [ type TYPE [ TYPE_ARGS ] ] }\n"
 		"GROUP := [ <id[,weight]>/<id[,weight]>/... ]\n"
+		"         [ hw_stats {off|on} ]\n"
 		"TYPE := { mpath | resilient }\n"
 		"TYPE_ARGS := [ RESILIENT_ARGS ]\n"
 		"RESILIENT_ARGS := [ buckets BUCKETS ] [ idle_timer IDLE ]\n"
@@ -1102,6 +1103,17 @@ static int ipnh_modify(int cmd, unsigned int flags, int argc, char **argv)
 			if (rtnl_rtprot_a2n(&prot, *argv))
 				invarg("\"protocol\" value is invalid\n", *argv);
 			req.nhm.nh_protocol = prot;
+		} else if (!strcmp(*argv, "hw_stats")) {
+			bool hw_stats;
+			int ret;
+
+			NEXT_ARG();
+			hw_stats = parse_on_off("hw_stats", *argv, &ret);
+			if (ret)
+				return ret;
+
+			addattr32(&req.n, sizeof(req), NHA_HW_STATS_ENABLE,
+				  hw_stats);
 		} else if (strcmp(*argv, "help") == 0) {
 			usage();
 		} else {
diff --git a/man/man8/ip-nexthop.8 b/man/man8/ip-nexthop.8
index f81a5910..aad68696 100644
--- a/man/man8/ip-nexthop.8
+++ b/man/man8/ip-nexthop.8
@@ -68,6 +68,8 @@ ip-nexthop \- nexthop object management
 .BR fdb " ] | "
 .B  group
 .IR GROUP " [ "
+.BR hw_stats " { "
+.BR on " | " off " }  ] [ "
 .BR fdb " ] [ "
 .B type
 .IR TYPE " [ " TYPE_ARGS " ] ] }"
-- 
2.43.0

Re: [PATCH iproute2-next v2 2/4] ip: ipnexthop: Support dumping next hop group stats

[Stephen Hemminger]

On Thu, 14 Mar 2024 15:52:13 +0100
Petr Machata <petrm@nvidia.com> wrote:

> Next hop group stats allow verification of balancedness of a next hop
> group. The feature was merged in kernel commit 7cf497e5a122 ("Merge branch
> 'nexthop-group-stats'"). Add to ip the corresponding support. The
> statistics are requested if "ip nexthop" is started with -s.
> 
> Signed-off-by: Petr Machata <petrm@nvidia.com>

Checkpatch complains:

WARNING: braces {} are not necessary for single statement blocks
#273: FILE: ip/ipnexthop.c:568:
+	if (show_stats) {
+		op_flags |= NHA_OP_FLAG_DUMP_STATS;
+	}

total: 0 erro

Re: [PATCH iproute2-next v2 2/4] ip: ipnexthop: Support dumping next hop group stats

[Petr Machata]

Stephen Hemminger <stephen@networkplumber.org> writes:

> On Thu, 14 Mar 2024 15:52:13 +0100
> Petr Machata <petrm@nvidia.com> wrote:
>
>> Next hop group stats allow verification of balancedness of a next hop
>> group. The feature was merged in kernel commit 7cf497e5a122 ("Merge branch
>> 'nexthop-group-stats'"). Add to ip the corresponding support. The
>> statistics are requested if "ip nexthop" is started with -s.
>> 
>> Signed-off-by: Petr Machata <petrm@nvidia.com>
>
> Checkpatch complains:
>
> WARNING: braces {} are not necessary for single statement blocks
> #273: FILE: ip/ipnexthop.c:568:
> +	if (show_stats) {
> +		op_flags |= NHA_OP_FLAG_DUMP_STATS;
> +	}
>
> total: 0 erro

Yes. The next patch adds more code to that branch.

Re: [PATCH iproute2-next v2 0/4] Support for nexthop group statistics

[patchwork-bot+netdevbpf]

Hello:

This series was applied to iproute2/iproute2-next.git (main)
by David Ahern <dsahern@kernel.org>:

On Thu, 14 Mar 2024 15:52:11 +0100 you wrote:
> Next hop group stats allow verification of balancedness of a next hop
> group. The feature was merged in kernel commit 7cf497e5a122 ("Merge branch
> 'nexthop-group-stats'"). This patchset adds to ip the corresponding
> support.
> 
> NH group stats come in two flavors: as statistics for SW and for HW
> datapaths. The former is shown when -s is given to "ip nexthop". The latter
> demands more work from the kernel, and possibly driver and HW, and might
> not be always necessary. Therefore tie it to -s -s, similarly to how ip
> link shows more detailed stats when -s is given twice.
> 
> [...]

Here is the summary with links:
  - [iproute2-next,v2,1/4] libnetlink: Add rta_getattr_uint()
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=95836fbf35d3
  - [iproute2-next,v2,2/4] ip: ipnexthop: Support dumping next hop group stats
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=529ada74c401
  - [iproute2-next,v2,3/4] ip: ipnexthop: Support dumping next hop group HW stats
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=a50655e730ff
  - [iproute2-next,v2,4/4] ip: ipnexthop: Allow toggling collection of nexthop group HW statistics
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=69d1c2c4aae8

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html