netdev - Re: [PATCH iproute2] 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: <20250216221444.6a94a0fe@hermes.local>
Date: Sun, 16 Feb 2025 22:14:44 -0800
From: Stephen Hemminger <stephen@...workplumber.org>
To: Jonathan Lennox <jonathan.lennox@....com>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH iproute2] tc: Fix rounding in tc_calc_xmittime and
 tc_calc_xmitsize.

On Wed, 5 Feb 2025 16:16:21 -0500
Jonathan Lennox <jonathan.lennox@....com> wrote:

> The logic in tc that converts between sizes and times for a given rate (the
> functions tc_calc_xmittime and tc_calc_xmitsize) suffers from double rounding,
> with intermediate values getting cast to unsigned int.
> 
> 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 get encoded as 0; all values between 1000 and 1999
> get encoded as 15 (equivalent to 960 bytes); all values between 2000 and 2999
> as 31 (1984 bytes); etc.
> 
> The attached patch changes this so these calculations are done entirely 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.
> 
> This is a userspace-only fix to tc; no kernel changes are necessary.
> 
> (Please let me know if anything is wrong with this patch, this is my first
> time submitting to any Linux kernel mailing lists.)
> 
> ---

The concept makes sense, but is missing a valid Signed-Off-by: and therefore
needs to be resent.

Also, you don't need to rename the functions, why not always use floating point