From: Shashank Mohan Jain <jain.sm@gmail.com>
To: "David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Cc: netdev@vger.kernel.org, Simon Horman <horms@kernel.org>,
Tal Gilboa <talgi@nvidia.com>, Saeed Mahameed <saeedm@nvidia.com>,
Tariq Toukan <tariqt@nvidia.com>,
Andrew Morton <akpm@linux-foundation.org>,
linux-kernel@vger.kernel.org, Leon Romanovsky <leon@kernel.org>,
Sashiko <sashiko-bot@kernel.org>
Subject: [PATCH net] lib/dim: fix 32-bit overflow of epms in dim_calc_stats()
Date: Mon, 5 Oct 2026 07:14:11 +0530 [thread overview]
Message-ID: <20261005014411.78626-1-jain.sm@gmail.com> (raw)
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
reply other threads:[~2026-10-05 1:44 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261005014411.78626-1-jain.sm@gmail.com \
--to=jain.sm@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=leon@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=saeedm@nvidia.com \
--cc=sashiko-bot@kernel.org \
--cc=talgi@nvidia.com \
--cc=tariqt@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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®