mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net] lib/dim: fix 32-bit overflow of epms in dim_calc_stats()
@ 2026-10-05  1:44 Shashank Mohan Jain
  0 siblings, 0 replies; only message in thread
From: Shashank Mohan Jain @ 2026-10-05  1:44 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: netdev, Simon Horman, Tal Gilboa, Saeed Mahameed, Tariq Toukan,
	Andrew Morton, linux-kernel, Leon Romanovsky, Sashiko

Commit 0843b2389064 ("lib/dim: fix 32-bit overflow in dim_calc_stats()
rates") moved the packet, byte and completion rates in dim_calc_stats()
to 64-bit arithmetic, but left the event rate as

	DIV_ROUND_UP(DIM_NEVENTS * USEC_PER_MSEC, delta_us)

DIV_ROUND_UP() computes (64000 + delta_us - 1) / delta_us. On 32-bit
architectures that sum is done in a 32-bit unsigned long and wraps for
delta_us >= 4294903297, the last 64 ms of the u32 microsecond range
that the function is meant to cover ("u32 holds up to 71 minutes").
epms then becomes 0 instead of 1.

net_dim() and rdma_dim() only wait for DIM_NEVENTS events before they
call dim_calc_stats(), with no time limit, so on an almost idle
interface a window can last that long. With epms == 0, cpe_ratio is
set to 0, net_dim_stats_compare() returns DIM_STATS_BETTER instead of
DIM_STATS_SAME when bpms and ppms did not change significantly, and
rdma_dim_stats_compare() compares a cpe_ratio of 0. The algorithm can
then step to another moderation profile based on a wrong rate.

Compute epms with DIV_ROUND_UP_ULL() as well. The result does not
change on 64-bit, or on 32-bit for windows shorter than 4294903297 us.

The issue was found by the Sashiko AI review of the original patch
(see the Closes: link). The fix and the test were written with an LLM
assistant. The dim KUnit suite computed the expected epms with the same
DIV_ROUND_UP() expression as dim_calc_stats(), so it could not catch
this; it now uses explicit expected values and gains two cases with
delta_us of 4294903297 and U32_MAX. Without the fix both new cases fail
on UML i386 and on qemu i386 (epms 0, expected 1) and pass on UML and
qemu x86_64; with the fix all 10 dim cases pass on all four.

Fixes: 0843b2389064 ("lib/dim: fix 32-bit overflow in dim_calc_stats() rates")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927051743.71460-1-jain.sm@gmail.com
Assisted-by: LLM
Signed-off-by: Shashank Mohan Jain <jain.sm@gmail.com>
---
Prepared with Claude Code (Anthropic), model Claude Opus 5.5
(claude-opus-5-5), which checked the Sashiko report against the code and
wrote the fix, the test changes and this changelog.

Tested on net.git main 6dc989ea46b9 with
./tools/testing/kunit/kunit.py run --kunitconfig (CONFIG_NET=y,
CONFIG_DIMLIB_KUNIT_TEST=y) on UML x86_64, UML i386 (SUBARCH=i386),
qemu x86_64 (--arch=x86_64) and qemu i386 (--arch=i386), with and
without the lib/dim/dim.c hunk. W=1 builds of lib/dim/ for i386 and
UML i386 are clean, and the i386 kernels link, so no 64-bit division
helper is pulled in.
Not tested: 32-bit ARM or MIPS builds, and no run on 32-bit hardware
with a NIC; I did not wait 71 minutes on an idle link to see the wrong
profile step happen, the effect on net_dim() follows from the code.

 lib/dim/dim.c       |  9 ++++++---
 lib/dim/dim_kunit.c | 40 +++++++++++++++++++++++++++++-----------
 2 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/lib/dim/dim.c b/lib/dim/dim.c
index 7f3eebae73cb..138589fc9048 100644
--- a/lib/dim/dim.c
+++ b/lib/dim/dim.c
@@ -69,13 +69,16 @@ bool dim_calc_stats(const struct dim_sample *start,
 	if (!delta_us)
 		return false;
 
-	/* u32 * USEC_PER_MSEC overflows a 32-bit long */
+	/*
+	 * u32 * USEC_PER_MSEC overflows a 32-bit long, and so does the
+	 * rounded-up epms dividend 64000 + delta_us - 1 for large delta_us
+	 */
 	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->epms = DIV_ROUND_UP_ULL((u64)DIM_NEVENTS * USEC_PER_MSEC,
+					    delta_us);
 	curr_stats->cpms = DIV_ROUND_UP_ULL((u64)ncomps * USEC_PER_MSEC,
 					    delta_us);
 	if (curr_stats->epms != 0)
diff --git a/lib/dim/dim_kunit.c b/lib/dim/dim_kunit.c
index 2e984f6b9fe4..8aa8df78bd3c 100644
--- a/lib/dim/dim_kunit.c
+++ b/lib/dim/dim_kunit.c
@@ -14,7 +14,7 @@ struct dim_calc_stats_case {
 	u32 start_pkts, end_pkts;
 	u32 start_bytes, end_bytes;
 	u32 start_comps, end_comps;
-	int ppms, bpms, cpms;
+	int ppms, bpms, epms, cpms;
 };
 
 static const struct dim_calc_stats_case dim_calc_stats_cases[] = {
@@ -22,13 +22,13 @@ 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,
+		.ppms = 640, .bpms = 960000, .epms = 64, .cpms = 64,
 	},
 	{
 		.name = "round_up",
 		.delta_us = 3000,
 		.end_pkts = 10, .end_bytes = 10, .end_comps = 1,
-		.ppms = 4, .bpms = 4, .cpms = 1,
+		.ppms = 4, .bpms = 4, .epms = 22, .cpms = 1,
 	},
 	{
 		.name = "counter_wrap",
@@ -36,35 +36,55 @@ static const struct dim_calc_stats_case dim_calc_stats_cases[] = {
 		.start_pkts = 0xffffff00, .end_pkts = 0x100,
 		.start_bytes = 0xfffff000, .end_bytes = 0x1000,
 		.start_comps = 0xfffffff0, .end_comps = 0x10,
-		.ppms = 0x200, .bpms = 0x2000, .cpms = 0x20,
+		.ppms = 0x200, .bpms = 0x2000, .epms = 64, .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,
+		.ppms = 2000, .bpms = 3000000, .epms = 7, .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,
+		.ppms = 182, .bpms = 268750, .epms = 4, .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,
+		.ppms = 83, .bpms = 125000, .epms = 2, .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,
+		.ppms = 2500, .bpms = 2500, .epms = 1, .cpms = 2500,
+	},
+	{
+		/*
+		 * 64 events in 71.6 minutes, the longest window delta_us can
+		 * hold: 64000 + delta_us - 1 exceeds 32 bits
+		 */
+		.name = "long_window",
+		.delta_us = U32_MAX,
+		.end_pkts = 64, .end_bytes = 64 * 1500, .end_comps = 64,
+		.ppms = 1, .bpms = 1, .epms = 1, .cpms = 1,
+	},
+	{
+		/*
+		 * the shortest window for which 64000 + delta_us - 1
+		 * exceeds 32 bits
+		 */
+		.name = "events_32bit_limit",
+		.delta_us = 4294903297U,
+		.end_pkts = 64, .end_bytes = 64 * 1500, .end_comps = 64,
+		.ppms = 1, .bpms = 1, .epms = 1, .cpms = 1,
 	},
 };
 
@@ -95,9 +115,7 @@ static void dim_calc_stats_test(struct kunit *test)
 	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));
+	KUNIT_EXPECT_EQ(test, stats.epms, t->epms);
 }
 
 static void dim_calc_stats_no_time_test(struct kunit *test)
-- 
2.43.0


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2026-10-05  1:44 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-05  1:44 [PATCH net] lib/dim: fix 32-bit overflow of epms in dim_calc_stats() Shashank Mohan Jain

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®