netdev - Re: [PATCH iproute2 v3] 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: <2d8adcbe-c379-45c3-9ca9-4f50dbe6a6da@mojatatu.com>
Date: Tue, 4 Mar 2025 14:51:21 -0300
From: Pedro Tammela <pctammela@...atatu.com>
To: Jonathan Lennox <jonathan.lennox@....com>
Cc: Jonathan Lennox <jonathan.lennox42@...il.com>,
 David Ahern <dsahern@...nel.org>, netdev@...r.kernel.org,
 Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [PATCH iproute2 v3] tc: Fix rounding in tc_calc_xmittime and
 tc_calc_xmitsize.

On 03/03/2025 19:13, Jonathan Lennox wrote:
>> On Mar 3, 2025, at 3:35 PM, Pedro Tammela <pctammela@...atatu.com> wrote:
>>
>> On 03/03/2025 16:43, Jonathan Lennox wrote:
>>>> On Mar 3, 2025, at 1:39 PM, Pedro Tammela <pctammela@...atatu.com> wrote:
>>>>
>>>> On 28/02/2025 12:50, patchwork-bot+netdevbpf@...nel.org wrote:
>>>>> Hello:
>>>>> This patch was applied to iproute2/iproute2-next.git (main)
>>>>> by David Ahern <dsahern@...nel.org>:
>>>>> On Wed, 26 Feb 2025 18:53:21 +0000 you wrote:
>>>>>> Currently, 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.
>>>>>>
>>>>>> [...]
>>>>> Here is the summary with links:
>>>>>    - [iproute2,v3] tc: Fix rounding in tc_calc_xmittime and tc_calc_xmitsize.
>>>>>      https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=d947f365602b
>>>>> You are awesome, thank you!
>>>>
>>>> Hi,
>>>>
>>>> This patch broke tdc:
>>>> https://github.com/p4tc-dev/tc-executor/blob/storage/artifacts/17084/1-tdc-sh/stdout#L2323
>>>>
>>> I’m afraid I’m not familiar with this test suite — can you point me at where it lives, what it’s doing,
>>> and what the expected output is?
>>
>> tdc lives here:
>> https://web.git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/tools/testing/selftests/tc-testing
>>
>> The broken tests are here:
>> https://web.git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
>>
>> Unrelated but useful is to use tools like vng to test your changes to tdc very quickly:
>> https://github.com/arighi/virtme-ng
> 
> What’s happening is that when the tests are verifying the test result, with
> "$TC actions get action police index 1”
> 
> and the like after the command
> "$TC actions add action police rate 7mbit burst 1m pipe index 1”
> 
> and the like, the burst size is now being printed as “1Mb” rather than as
> “1024Kb”.  (And the same for the subsequent tests.)
> 
> This is because, due to the rounding errors the patch fixed, the precise
> value being printed for the burst was previously 1048574 (2b less than
> 1Mb) which sprint_size prints as “1024Kb”.  The value being printed is now
> 1048576 (exactly 1Mb).
> 
> I would argue that the new output is more correct, given the input “1m”, and
> the test cases should be updated.

Makes sense

> 
> What is the proper procedure for submitting a patch for the tests?  Does it
> also go to this mailing list?

Yes: https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt
net-next is the tree you should submit to.