[PATCH net 0/2] lib/dim: fix 32-bit overflow in dim_calc_stats()

[PATCH net 0/2] lib/dim: fix 32-bit overflow in dim_calc_stats()

[Shashank Mohan Jain]

On 32-bit kernels dim_calc_stats() multiplies the u32 byte, packet and
completion counts of a DIM window by USEC_PER_MSEC (1000L) in 32-bit
long arithmetic. Once a window carries more than about 4.3 MB, bpms
wraps, and net_dim steers interrupt moderation on a meaningless
throughput value. At 1 Gbit/s line rate a 64-event window passes that
size when there are fewer than about 1,860 DIM events per second,
which is common while NAPI keeps the interrupt masked under load.
32-bit users of the library include mtk_eth_soc (MT7621, MT7623),
bcmgenet and bcmsysport on 32-bit ARM, xilinx_axienet on Zynq-7000 and
MicroBlaze, and virtio_net in 32-bit guests. The overflow goes back to
the mlx5e code the library was moved from.

Patch 1 does the multiplications in 64 bits and divides with
DIV_ROUND_UP_ULL(); the results on 64-bit are unchanged. Patch 2 adds a
KUnit suite for dim_calc_stats() whose large-window cases fail on
32-bit without patch 1. It is part of this series as described under
"Co-posting selftests" in maintainer-netdev.rst.

The series is based on net (a7bfaba4823e) and has no dependencies; both
patches also apply to mainline (fd179f8a05be) and net-next.

The bug was found and the patches were prepared with Claude Code
(Anthropic), model Claude Opus 5.5 (claude-opus-5-5).

Tested:
- KUnit (CONFIG_DIMLIB_KUNIT_TEST=y) on UML i386 (SUBARCH=i386):
  without patch 1, 4 of the 7 dim_calc_stats cases fail (many_bytes,
  bytes_32bit_limit, gigabit, many_packets); with it all pass. On UML
  x86_64 all cases pass with and without patch 1. Both were run on
  mainline and on net.
- W=1 builds of lib/dim/ for UML x86_64 and i386 without warnings;
  dim.o references no libgcc 64-bit division helpers. A native i386
  defconfig build (vmlinux and modules, DIMLIB=y) succeeds.
- checkpatch --strict. Its "does MAINTAINERS need updating?" warning
  on patch 2 does not apply: lib/dim/ is already covered by the
  DYNAMIC INTERRUPT MODERATION entry.

Not tested: 32-bit ARM or MIPS builds (no cross compiler was
available), and no run on a 32-bit NIC. The traffic levels at which
the drivers hit the overflow are derived from how they count DIM
events, not measured.

Shashank Mohan Jain (2):
  lib/dim: fix 32-bit overflow in dim_calc_stats() rates
  lib/dim: add KUnit test for dim_calc_stats()

 lib/Kconfig.debug   |  15 ++++++
 lib/dim/Makefile    |   2 +
 lib/dim/dim.c       |  10 ++--
 lib/dim/dim_kunit.c | 126 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 150 insertions(+), 3 deletions(-)
 create mode 100644 lib/dim/dim_kunit.c


base-commit: a7bfaba4823e3c165bb2004c74eff7c096672bc7
-- 
2.43.0

[PATCH net 1/2] lib/dim: fix 32-bit overflow in dim_calc_stats() rates

[Shashank Mohan Jain]

dim_calc_stats() computes the per-millisecond rates as

	DIV_ROUND_UP(nbytes * USEC_PER_MSEC, delta_us)

where nbytes is a u32 and USEC_PER_MSEC is 1000L. On 64-bit the product
is done in 64-bit long arithmetic, but on 32-bit architectures long is
32 bits wide and the product wraps as soon as a measurement window
carries more than 4294967 bytes (about 4.3 MB). The same applies to the
packet and completion counts, although those need more than 4.29
million packets or completions per window.

A DIM window spans DIM_NEVENTS (64) events. Drivers count events per
interrupt or per NAPI poll, so under sustained load a window can easily
carry more than 4.3 MB: 64 full NAPI polls of 64 MTU-sized frames are
already 6.2 MB, and drivers such as mtk_eth_soc count one event per
interrupt while NAPI keeps polling with the interrupt masked. On 32-bit
users of the library (for example mtk_eth_soc on MT7621, bcmgenet and
bcmsysport on 32-bit ARM, or virtio_net in a 32-bit guest) bpms then
becomes the product modulo 2^32 divided by the window length, and
net_dim_stats_compare() makes its BETTER/WORSE decisions on a value
that has little to do with the real throughput.

For example, a 1 Gbit/s link at line rate that moves 5 MB in a 40 ms
window gives bpms = 125000 on 64-bit but 17626 on 32-bit, and 5 million
packets in 2 s gives ppms = 353 instead of 2500.

Widen the products to 64 bits and divide with DIV_ROUND_UP_ULL(). The
results are unchanged on 64-bit.

Fixes: cb3c7fd4f839 ("net/mlx5e: Support adaptive RX coalescing")
Fixes: 4c4dbb4a7363 ("net/mlx5e: Move dynamic interrupt coalescing code to include/linux")
Assisted-by: LLM
Signed-off-by: Shashank Mohan Jain <jain.sm@gmail.com>
---
Found by reading lib/dim with an LLM assistant (Claude Code, Claude
Opus 5.5), which also drafted the fix, the KUnit test and the
changelogs.

Tested: the new KUnit suite (patch 2), on mainline and on net, on UML
i386 (fails 4 of 7 dim_calc_stats cases without this patch, passes
with it) and UML x86_64 (passes with and without it); W=1 builds of
lib/dim/ for UML x86_64 and i386, and a native i386 vmlinux+modules
build, with no 64-bit division helper references.
Not tested: 32-bit ARM or MIPS builds (no cross compiler available),
and no run on a 32-bit NIC; the traffic levels at which drivers hit
the overflow are derived from how they count DIM events, not measured.

 lib/dim/dim.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/dim/dim.c b/lib/dim/dim.c
index 97c3d084ebf0..7f3eebae73cb 100644
--- a/lib/dim/dim.c
+++ b/lib/dim/dim.c
@@ -69,11 +69,15 @@ bool dim_calc_stats(const struct dim_sample *start,
 	if (!delta_us)
 		return false;
 
-	curr_stats->ppms = DIV_ROUND_UP(npkts * USEC_PER_MSEC, delta_us);
-	curr_stats->bpms = DIV_ROUND_UP(nbytes * USEC_PER_MSEC, delta_us);
+	/* u32 * USEC_PER_MSEC overflows a 32-bit long */
+	curr_stats->ppms = DIV_ROUND_UP_ULL((u64)npkts * USEC_PER_MSEC,
+					    delta_us);
+	curr_stats->bpms = DIV_ROUND_UP_ULL((u64)nbytes * USEC_PER_MSEC,
+					    delta_us);
 	curr_stats->epms = DIV_ROUND_UP(DIM_NEVENTS * USEC_PER_MSEC,
 					delta_us);
-	curr_stats->cpms = DIV_ROUND_UP(ncomps * USEC_PER_MSEC, delta_us);
+	curr_stats->cpms = DIV_ROUND_UP_ULL((u64)ncomps * USEC_PER_MSEC,
+					    delta_us);
 	if (curr_stats->epms != 0)
 		curr_stats->cpe_ratio = DIV_ROUND_DOWN_ULL(
 			curr_stats->cpms * 100, curr_stats->epms);
-- 
2.43.0

[PATCH net 2/2] lib/dim: add KUnit test for dim_calc_stats()

[Shashank Mohan Jain]

Add a KUnit suite for the DIM library that checks the packet, byte,
event and completion rates computed by dim_calc_stats(), including
counter wraparound and windows whose byte or packet count times
USEC_PER_MSEC does not fit in 32 bits. The latter cases fail on 32-bit
architectures without the previous commit.

Assisted-by: LLM
Signed-off-by: Shashank Mohan Jain <jain.sm@gmail.com>
---
 lib/Kconfig.debug   |  15 ++++++
 lib/dim/Makefile    |   2 +
 lib/dim/dim_kunit.c | 126 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 143 insertions(+)
 create mode 100644 lib/dim/dim_kunit.c

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 134b15a44625..6b24a2c89b84 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2906,6 +2906,21 @@ config LIVEUPDATE_TEST
 
 	  If unsure, say N
 
+config DIMLIB_KUNIT_TEST
+	tristate "KUnit test for the DIM library" if !KUNIT_ALL_TESTS
+	depends on KUNIT && NET
+	select DIMLIB
+	default KUNIT_ALL_TESTS
+	help
+	  This builds the KUnit test for the Dynamic Interrupt Moderation
+	  library (lib/dim/), which checks the rate statistics computed by
+	  dim_calc_stats().
+
+	  For more information on KUnit and unit tests in general please refer
+	  to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+	  If unsure, say N.
+
 config CMDLINE_KUNIT_TEST
 	tristate "KUnit test for cmdline API" if !KUNIT_ALL_TESTS
 	depends on KUNIT
diff --git a/lib/dim/Makefile b/lib/dim/Makefile
index 5b9bfaac7ac1..47e1c36230e4 100644
--- a/lib/dim/Makefile
+++ b/lib/dim/Makefile
@@ -5,3 +5,5 @@
 obj-$(CONFIG_DIMLIB) += dimlib.o
 
 dimlib-y := dim.o net_dim.o rdma_dim.o
+
+obj-$(CONFIG_DIMLIB_KUNIT_TEST) += dim_kunit.o
diff --git a/lib/dim/dim_kunit.c b/lib/dim/dim_kunit.c
new file mode 100644
index 000000000000..2e984f6b9fe4
--- /dev/null
+++ b/lib/dim/dim_kunit.c
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for the Dynamic Interrupt Moderation library
+ */
+
+#include <kunit/test.h>
+#include <linux/dim.h>
+#include <linux/ktime.h>
+#include <linux/module.h>
+
+struct dim_calc_stats_case {
+	const char *name;
+	u32 delta_us;
+	u32 start_pkts, end_pkts;
+	u32 start_bytes, end_bytes;
+	u32 start_comps, end_comps;
+	int ppms, bpms, cpms;
+};
+
+static const struct dim_calc_stats_case dim_calc_stats_cases[] = {
+	{
+		.name = "small",
+		.delta_us = 1000,
+		.end_pkts = 640, .end_bytes = 640 * 1500, .end_comps = 64,
+		.ppms = 640, .bpms = 960000, .cpms = 64,
+	},
+	{
+		.name = "round_up",
+		.delta_us = 3000,
+		.end_pkts = 10, .end_bytes = 10, .end_comps = 1,
+		.ppms = 4, .bpms = 4, .cpms = 1,
+	},
+	{
+		.name = "counter_wrap",
+		.delta_us = 1000,
+		.start_pkts = 0xffffff00, .end_pkts = 0x100,
+		.start_bytes = 0xfffff000, .end_bytes = 0x1000,
+		.start_comps = 0xfffffff0, .end_comps = 0x10,
+		.ppms = 0x200, .bpms = 0x2000, .cpms = 0x20,
+	},
+	{
+		/* 30 MB in 10 ms (24 Gbit/s): nbytes * 1000 exceeds 32 bits */
+		.name = "many_bytes",
+		.delta_us = 10000,
+		.end_pkts = 20000, .end_bytes = 30000000, .end_comps = 64,
+		.ppms = 2000, .bpms = 3000000, .cpms = 7,
+	},
+	{
+		/* 4.3 MB in 16 ms (2.15 Gbit/s), just above the 32-bit limit */
+		.name = "bytes_32bit_limit",
+		.delta_us = 16000,
+		.end_pkts = 2900, .end_bytes = 4300000, .end_comps = 64,
+		.ppms = 182, .bpms = 268750, .cpms = 4,
+	},
+	{
+		/* 5 MB in 40 ms: a 1 Gbit/s link at line rate */
+		.name = "gigabit",
+		.delta_us = 40000,
+		.end_pkts = 3300, .end_bytes = 5000000, .end_comps = 64,
+		.ppms = 83, .bpms = 125000, .cpms = 2,
+	},
+	{
+		/* 5 million packets and completions in 2 s */
+		.name = "many_packets",
+		.delta_us = 2000000,
+		.end_pkts = 5000000, .end_bytes = 5000000, .end_comps = 5000000,
+		.ppms = 2500, .bpms = 2500, .cpms = 2500,
+	},
+};
+
+static void dim_calc_stats_case_desc(const struct dim_calc_stats_case *t,
+				     char *desc)
+{
+	strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(dim_calc_stats, dim_calc_stats_cases,
+		  dim_calc_stats_case_desc);
+
+static void dim_calc_stats_test(struct kunit *test)
+{
+	const struct dim_calc_stats_case *t = test->param_value;
+	struct dim_sample start = {}, end = {};
+	struct dim_stats stats = {};
+
+	dim_update_sample_with_comps(0, t->start_pkts, t->start_bytes,
+				     t->start_comps, &start);
+	dim_update_sample_with_comps(DIM_NEVENTS, t->end_pkts, t->end_bytes,
+				     t->end_comps, &end);
+	/* dim_update_sample() stamps ktime_get(); use fixed times instead */
+	start.time = ktime_set(1000, 0);
+	end.time = ktime_add_us(start.time, t->delta_us);
+
+	KUNIT_ASSERT_TRUE(test, dim_calc_stats(&start, &end, &stats));
+	KUNIT_EXPECT_EQ(test, stats.ppms, t->ppms);
+	KUNIT_EXPECT_EQ(test, stats.bpms, t->bpms);
+	KUNIT_EXPECT_EQ(test, stats.cpms, t->cpms);
+	KUNIT_EXPECT_EQ(test, stats.epms,
+			(int)DIV_ROUND_UP(DIM_NEVENTS * USEC_PER_MSEC,
+					  t->delta_us));
+}
+
+static void dim_calc_stats_no_time_test(struct kunit *test)
+{
+	struct dim_sample sample = {};
+	struct dim_stats stats = {};
+
+	dim_update_sample_with_comps(0, 100, 1000, 10, &sample);
+	KUNIT_EXPECT_FALSE(test, dim_calc_stats(&sample, &sample, &stats));
+}
+
+static struct kunit_case dim_test_cases[] = {
+	KUNIT_CASE_PARAM(dim_calc_stats_test, dim_calc_stats_gen_params),
+	KUNIT_CASE(dim_calc_stats_no_time_test),
+	{}
+};
+
+static struct kunit_suite dim_test_suite = {
+	.name = "dim",
+	.test_cases = dim_test_cases,
+};
+
+kunit_test_suite(dim_test_suite);
+
+MODULE_DESCRIPTION("KUnit tests for the DIM library");
+MODULE_LICENSE("GPL");
-- 
2.43.0