* [PATCH iproute2 v3 1/2] ip: ipstats: Merge statistics split across several netlink messages
2026-09-10 9:20 [PATCH iproute2 v3 0/2] ip: ipstats: Fix statistics split across netlink messages Alexander Zubkov
@ 2026-09-10 9:20 ` Alexander Zubkov
2026-09-10 9:20 ` [PATCH iproute2 v3 2/2] ip: ipstats: Do not hide HW statistics when hw_stats_info is missing Alexander Zubkov
1 sibling, 0 replies; 3+ messages in thread
From: Alexander Zubkov @ 2026-09-10 9:20 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, Petr Machata, Ido Schimmel, Alexander Zubkov
The kernel can split the statistics of one interface across several
netlink messages. ipstats_dump_one() formats every message on its
own, so the interface is shown twice and the attributes of the second
message are dropped:
109: vlan859: group offload subgroup l3_stats on used on
109: vlan859: group offload subgroup l3_stats
Buffer the message instead, merge into it the following messages with
the same ifindex, and format it once another ifindex shows up or the
dump ends.
Fixes: 54d82b0699a0 ("ip: Add a new family of commands, "stats"")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Alexander Zubkov <green@qrator.net>
---
v3:
- Merge rewritten along the lines Stephen suggested: no attribute
counting, no classifier, no refusal. Only the top-level nest and,
for bridge xstats, the one inside it can be reopened.
- The invariants moved from the cover letter into the comment above
ipstats_merge_attrs().
- Mask rta_type with NLA_TYPE_MASK, drop the dead addattr_nest() NULL
checks, malloc instead of calloc, no err out-parameter and no
separate ipstats_msg_merge().
- One deviation from suggested version: the depth == 2 test is moved
above the two scans. At that depth there might be no nested
attributes.
v2: https://lore.kernel.org/netdev/20260908181225.31712-1-green@qrator.net/
- Trimmed the comments and the commit message.
- Dropped IPSTATS_MERGE_MAX_DEPTH.
- Only the boundary attributes are merged.
- All of the merging decision is in ipstats_merge_classify().
- Do not index ipstats_stat_ifla_max[] with attribute type 0 (padding).
- Use addraw_l() to copy attributes.
v1: https://lore.kernel.org/netdev/20260830181949.1096-1-green@qrator.net/
Notes:
- Reproduced on Linux 7.1.5 / iproute2-7.0.0, Mellanox MSN4600C with 121
netdevices, 31 with l3_stats; the split is visible in strace sa two
RTM_NEWSTATS messages with the same ifindex.
- The merge was also exercised by Claude out of tree against offload
xstats cut between HW_S_INFO and L3_STATS, bridge per-VLAN xstats cut
between two BRIDGE_XSTATS_VLAN entries, and a leaf attribute in both
messages.
ip/ipstats.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 133 insertions(+), 7 deletions(-)
diff --git a/ip/ipstats.c b/ip/ipstats.c
index 8a2ac85e..1d44fe47 100644
--- a/ip/ipstats.c
+++ b/ip/ipstats.c
@@ -850,12 +850,68 @@ ipstats_show_one(int ifindex, struct ipstats_stat_enabled *enabled)
return err;
}
-static int ipstats_dump_one(struct nlmsghdr *n, void *arg)
+/* The kernel resumes a split dump by reopening the top-level nest it stopped
+ * in and, for bridge xstats, the nest inside that. Everything below is whole
+ * and nothing at those two levels repeats, so join the two halves at the
+ * boundary and copy the rest.
+ */
+static int ipstats_merge_attrs(struct nlmsghdr *n, int maxlen,
+ struct rtattr *a, int alen,
+ struct rtattr *b, int blen, int depth)
+{
+ struct rtattr *last = NULL, *rta, *nest;
+ unsigned short type;
+ int rem;
+
+ if (depth == 2)
+ goto copy;
+
+ /* type 0 is 64-bit padding, possibly on either side of the split */
+ for (rta = a, rem = alen; RTA_OK(rta, rem); rta = RTA_NEXT(rta, rem))
+ if (rta->rta_type)
+ last = rta;
+ while (RTA_OK(b, blen) && !b->rta_type)
+ b = RTA_NEXT(b, blen);
+ if (!last || !RTA_OK(b, blen))
+ goto copy;
+
+ type = last->rta_type & NLA_TYPE_MASK;
+ if (type != (b->rta_type & NLA_TYPE_MASK) ||
+ (!depth && (type > IFLA_STATS_MAX || !ipstats_stat_ifla_max[type])))
+ goto copy;
+
+ if (addraw_l(n, maxlen, a, (char *)last - (char *)a))
+ return -EMSGSIZE;
+
+ nest = addattr_nest(n, maxlen, last->rta_type);
+ if (ipstats_merge_attrs(n, maxlen, RTA_DATA(last), RTA_PAYLOAD(last),
+ RTA_DATA(b), RTA_PAYLOAD(b), depth + 1))
+ return -EMSGSIZE;
+ addattr_nest_end(n, nest);
+
+ b = RTA_NEXT(b, blen);
+ return addraw_l(n, maxlen, b, blen) ? -EMSGSIZE : 0;
+copy:
+ if (addraw_l(n, maxlen, a, alen) || addraw_l(n, maxlen, b, blen))
+ return -EMSGSIZE;
+ return 0;
+}
+
+struct ipstats_dump_ctx {
+ struct ipstats_stat_enabled *enabled;
+ struct nlmsghdr *pending;
+};
+
+static int ipstats_dump_pending(struct ipstats_dump_ctx *ctx)
{
- struct ipstats_stat_enabled *enabled = arg;
int rc;
- rc = ipstats_process_ifsm(stdout, n, enabled);
+ if (ctx->pending == NULL)
+ return 0;
+
+ rc = ipstats_process_ifsm(stdout, ctx->pending, ctx->enabled);
+ free(ctx->pending);
+ ctx->pending = NULL;
if (rc)
return rc;
@@ -863,9 +919,78 @@ static int ipstats_dump_one(struct nlmsghdr *n, void *arg)
return 0;
}
+#define IFSM_PAYLOAD(n) NLMSG_PAYLOAD(n, sizeof(struct if_stats_msg))
+
+static int ipstats_dump_merge(struct ipstats_dump_ctx *ctx, struct nlmsghdr *n)
+{
+ int hdrlen = NLMSG_LENGTH(sizeof(struct if_stats_msg));
+ int maxlen = ctx->pending->nlmsg_len + n->nlmsg_len;
+ struct nlmsghdr *merged;
+ int rc;
+
+ merged = malloc(maxlen);
+ if (merged == NULL) {
+ fprintf(stderr, "Error merging netlink answer: %s\n",
+ strerror(errno));
+ return -errno;
+ }
+
+ memcpy(merged, ctx->pending, hdrlen);
+ merged->nlmsg_len = hdrlen;
+
+ rc = ipstats_merge_attrs(merged, maxlen,
+ IFLA_STATS_RTA(NLMSG_DATA(ctx->pending)),
+ IFSM_PAYLOAD(ctx->pending),
+ IFLA_STATS_RTA(NLMSG_DATA(n)),
+ IFSM_PAYLOAD(n), 0);
+ if (rc) {
+ free(merged);
+ return rc;
+ }
+
+ free(ctx->pending);
+ ctx->pending = merged;
+ return 0;
+}
+
+static int ipstats_dump_one(struct nlmsghdr *n, void *arg)
+{
+ struct ipstats_dump_ctx *ctx = arg;
+ int rc;
+
+ if (IFSM_PAYLOAD(n) < 0) {
+ fprintf(stderr, "BUG: wrong nlmsg len %d\n", n->nlmsg_len);
+ return -EINVAL;
+ }
+
+ if (ctx->pending != NULL) {
+ const struct if_stats_msg *pending = NLMSG_DATA(ctx->pending);
+ const struct if_stats_msg *ifsm = NLMSG_DATA(n);
+
+ if (pending->ifindex == ifsm->ifindex)
+ return ipstats_dump_merge(ctx, n);
+
+ rc = ipstats_dump_pending(ctx);
+ if (rc)
+ return rc;
+ }
+
+ ctx->pending = malloc(n->nlmsg_len);
+ if (ctx->pending == NULL) {
+ fprintf(stderr, "Error saving netlink answer: %s\n",
+ strerror(errno));
+ return -ENOMEM;
+ }
+
+ memcpy(ctx->pending, n, n->nlmsg_len);
+ return 0;
+}
+
static int ipstats_dump(struct ipstats_stat_enabled *enabled)
{
- int rc = 0;
+ struct ipstats_dump_ctx ctx = {
+ .enabled = enabled,
+ };
if (rtnl_statsdump_req_filter(&rth, PF_UNSPEC, 0,
ipstats_req_add_filters,
@@ -874,12 +999,13 @@ static int ipstats_dump(struct ipstats_stat_enabled *enabled)
return -2;
}
- if (rtnl_dump_filter(&rth, ipstats_dump_one, enabled) < 0) {
+ if (rtnl_dump_filter(&rth, ipstats_dump_one, &ctx) < 0) {
fprintf(stderr, "Dump terminated\n");
- rc = -2;
+ free(ctx.pending);
+ return -2;
}
- return rc;
+ return ipstats_dump_pending(&ctx);
}
static int
--
2.55.0
^ permalink raw reply related [flat|nested] 3+ messages in thread