netdev - Re: [PATCH net v2 1/3] tcp_cubic: fix to run bictcp

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: <CADVnQykHS1uPT1Wa4WjOdnJ2=bQh1ZkQUBK2GNXGwcg5u5SC3g@mail.gmail.com>
Date: Thu, 15 Aug 2024 10:43:23 -0400
From: Neal Cardwell <ncardwell@...gle.com>
To: Mingrui Zhang <mrzhang97@...il.com>
Cc: edumazet@...gle.com, davem@...emloft.net, netdev@...r.kernel.org, 
	Lisong Xu <xu@....edu>
Subject: Re: [PATCH net v2 1/3] tcp_cubic: fix to run bictcp_update() at least
 once per RTT

On Wed, Aug 14, 2024 at 8:19 PM Mingrui Zhang <mrzhang97@...il.com> wrote:
>
> The original code bypasses bictcp_update() under certain conditions
> to reduce the CPU overhead. Intuitively, when last_cwnd==cwnd,
> bictcp_update() is executed 32 times per second. As a result,
> it is possible that bictcp_update() is not executed for several
> RTTs when RTT is short (specifically < 1/32 second = 31 ms and
> last_cwnd==cwnd which may happen in small-BDP networks),
> thus leading to low throughput in these RTTs.
>
> The patched code executes bictcp_update() 32 times per second
> if RTT > 31 ms or every RTT if RTT < 31 ms, when last_cwnd==cwnd.
>
> Thanks
> Mingrui, and Lisong
>
> Fixes: 91a4599c2ad8 ("tcp_cubic: fix to run bictcp_update() at least once per RTT")

The Fixes: footer is not for restating the commit title in your new
patch. Instead, it should list the SHA1 and first commit message line
from the old commit (potentially years ago) that has buggy behavior
that you are fixing. That way the Linux maintainers know which Linux
releases have the bug, and they know how far back in release history
the fix should be applied, when backporting fixes to stable releases.

More information:
 https://www.kernel.org/doc/html/v6.10/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes

Please update the Fixes footers in the three commits and repost. :-)

Thanks!
neal


> Signed-off-by: Mingrui Zhang <mrzhang97@...il.com>
> Signed-off-by: Lisong Xu <xu@....edu>
>
> ---
>  net/ipv4/tcp_cubic.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
> index 5dbed91c6178..11bad5317a8f 100644
> --- a/net/ipv4/tcp_cubic.c
> +++ b/net/ipv4/tcp_cubic.c
> @@ -218,8 +218,12 @@ static inline void bictcp_update(struct bictcp *ca, u32 cwnd, u32 acked)
>
>         ca->ack_cnt += acked;   /* count the number of ACKed packets */
>
> +       /* Update 32 times per second if RTT > 1/32 second,
> +        *        every RTT if RTT < 1/32 second
> +        *        even when last_cwnd == cwnd
> +        */
>         if (ca->last_cwnd == cwnd &&
> -           (s32)(tcp_jiffies32 - ca->last_time) <= HZ / 32)
> +           (s32)(tcp_jiffies32 - ca->last_time) <= min(HZ / 32, usecs_to_jiffies(ca->delay_min)))
>                 return;
>
>         /* The CUBIC function can update ca->cnt at most once per jiffy.
> --
> 2.34.1
>