Skip to contentSkip to conversation

Support for nexthop group statistics

PATCH Applied Petr Machata 3 participants 8 messages Mar 14, 2024 – Mar 15, 2024

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Petr Machata

author
#0 lore raw

v2 cover Support for nexthop group statistics

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(+)
Signature · 1 line
2.43.0
author
#1 lore raw

PATCH v2 1/4 libnetlink: Add rta_getattr_uint()

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 <[email protected]>

---

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

 include/libnetlink.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
include/libnetlink.h +14 -0
index ad7e7127..35a9bb57 100644
@@ -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);
Signature · 1 line
2.43.0
author
#2 lore raw

PATCH v2 2/4 ip: ipnexthop: Support dumping next hop group stats

In reply to 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 <[email protected]>

---

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

 ip/ipnexthop.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++
 ip/nh_common.h |  6 ++++
 2 files changed, 95 insertions(+)
ip/ipnexthop.c +89 -0
index e946d6f9..cba3d934 100644
@@ -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);
ip/nh_common.h +6 -0
index 4d6677e6..e2f74ec5 100644
@@ -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,
Signature · 1 line
2.43.0
#3 lore raw

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

Quoted 6 lines · On Thu, 14 Mar 2024 15:52:13 +0100 Petr Machata <[email protected]> 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 <[email protected]>

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

author
#4 lore raw

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

Quoted 18 lines · Stephen Hemminger
Quoted 6 lines · On Thu, 14 Mar 2024 15:52:13 +0100 Petr Machata <[email protected]> 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 <[email protected]>

Checkpatch complains:

WARNING: braces {} are not necessary for single statement blocks #273: FILE: ip/ipnexthop.c:568:

ip/ipnexthop.c +3 -0
+	if (show_stats) {
+		op_flags |= NHA_OP_FLAG_DUMP_STATS;
+	}
Quoted 1 line

total: 0 erro

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

author
#5 lore raw

PATCH v2 3/4 ip: ipnexthop: Support dumping next hop group HW stats

In reply to 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 <[email protected]>
---
 ip/ipnexthop.c | 28 ++++++++++++++++++++++++++++
 ip/nh_common.h |  5 +++++
 2 files changed, 33 insertions(+)
ip/ipnexthop.c +28 -0
index cba3d934..6c5d524b 100644
@@ -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;
ip/nh_common.h +5 -0
index e2f74ec5..33b1d847 100644
@@ -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;
Signature · 1 line
2.43.0
author
#6 lore raw

PATCH v2 4/4 ip: ipnexthop: Allow toggling collection of nexthop group HW statistics

In reply to 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 <[email protected]>
---
 ip/ipnexthop.c        | 12 ++++++++++++
 man/man8/ip-nexthop.8 |  2 ++
 2 files changed, 14 insertions(+)
ip/ipnexthop.c +12 -0
index 6c5d524b..8aa06de0 100644
@@ -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 {
man/man8/ip-nexthop.8 +2 -0
index f81a5910..aad68696 100644
@@ -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 " ] ] }"
Signature · 1 line
2.43.0
CI Applied
#7 lore raw
In reply to Petr Machata
This series was applied to iproute2/iproute2-next.git (main) by David Ahern <[email protected]>:

Hello:

This series was applied to iproute2/iproute2-next.git (main) by David Ahern <[email protected]>:

Quoted 5 lines · 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.

Quoted 7 lines

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!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html