netdev - Re: [PATCH iproute2 v2] tc: Fix rounding in tc_calc_xmittime and tc_calc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <789337E1-213F-4FAE-A5D4-4647C0A288E1@8x8.com>
Date: Mon, 24 Feb 2025 11:36:30 -0500
From: Jonathan Lennox <jonathan.lennox@....com>
To: David Ahern <dsahern@...nel.org>
Cc: netdev@...r.kernel.org,
 Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [PATCH iproute2 v2] tc: Fix rounding in tc_calc_xmittime and
 tc_calc_xmitsize.

> On Feb 23, 2025, at 10:06 PM, David Ahern <dsahern@...nel.org> wrote:
> 
> On 2/18/25 1:10 PM, Jonathan Lennox wrote:
>> 
> 
> lacking a commit message. What is the problem with the current code and
> how do this patch fix it. Add an example that led you down this path as
> well.
> 
>> Signed-off-by: Jonathan Lennox <jonathan.lennox@....com>
>> ---
>> tc/tc_core.c | 6 +++---
>> tc/tc_core.h | 2 +-
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>> 
> 

Sorry; this was the v2 patch and I explained it in the v1, but I don’t think the
threading worked.

The problem is that tc_calc_xmittime and tc_calc_xmitsize round from
double to int three times — once when they call tc_core_time2tick / tc_core_tick2time
(whose argument is int), once when those functions return (their return value is int),
and then finally when the tc_calc_* functions return.  This leads to extremely
granular and inaccurate conversions.

As a result, for example, on my test system (where tick_in_usec=15.625,
clock_factor=1, and hz=1000000000) for a bitrate of 1Gbps, all tc htb burst
values between 0 and 999 bytes get encoded as 0 ticks; all values between
1000 and 1999 bytes get encoded as 15 ticks (equivalent to 960 bytes); all
values between 2000 and 2999 bytes as 31 ticks (1984 bytes); etc.

The patch changes the code so these calculations are done internally in
floating-point, and only rounded to integer values when the value is returned.
It also changes tc_calc_xmittime to round its calculated value up, rather than
down, to ensure that the calculated time is actually sufficient for the requested
size.

Can you let me know the desired style for commit messages — how much of this
explanation should be in it?  I can submit a v3 with the desired explanation in
the commit message.

Thanks!

Jonathan Lennox