netdev - Re: [PATCH net v3 3/3] tcp_cubic: fix to use emulated Reno cwnd one RTT in the future

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: <b91e1160-24c0-42c5-8a71-b10b3421147b@gmail.com>
Date: Fri, 16 Aug 2024 20:21:19 -0500
From: Mingrui Zhang <mrzhang97@...il.com>
To: Neal Cardwell <ncardwell@...gle.com>
Cc: edumazet@...gle.com, davem@...emloft.net, netdev@...r.kernel.org,
 Lisong Xu <xu@....edu>
Subject: Re: [PATCH net v3 3/3] tcp_cubic: fix to use emulated Reno cwnd one
 RTT in the future

On 8/16/24 13:32, Neal Cardwell wrote:
> On Thu, Aug 15, 2024 at 5:41 PM Mingrui Zhang <mrzhang97@...il.com> wrote:
>> The original code estimates RENO snd_cwnd using the estimated
>> RENO snd_cwnd at the current time (i.e., tcp_cwnd).
>>
>> The patched code estimates RENO snd_cwnd using the estimated
>> RENO snd_cwnd after one RTT (i.e., tcp_cwnd_next_rtt),
>> because ca->cnt is used to increase snd_cwnd for the next RTT.
>>
>> Fixes: 89b3d9aaf467 ("[TCP] cubic: precompute constants")
>> Signed-off-by: Mingrui Zhang <mrzhang97@...il.com>
>> Signed-off-by: Lisong Xu <xu@....edu>
>> ---
>> v2->v3: Corrent the "Fixes:" footer content
>> v1->v2: Separate patches
>>
>>  net/ipv4/tcp_cubic.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
>> index 7bc6db82de66..a1467f99a233 100644
>> --- a/net/ipv4/tcp_cubic.c
>> +++ b/net/ipv4/tcp_cubic.c
>> @@ -315,8 +315,11 @@ static inline void bictcp_update(struct bictcp *ca, u32 cwnd, u32 acked)
>>                         ca->tcp_cwnd++;
>>                 }
>>
>> -               if (ca->tcp_cwnd > cwnd) {      /* if bic is slower than tcp */
>> -                       delta = ca->tcp_cwnd - cwnd;
>> +               /* Reno cwnd one RTT in the future */
>> +               u32 tcp_cwnd_next_rtt = ca->tcp_cwnd + (ca->ack_cnt + cwnd) / delta;
>> +
>> +               if (tcp_cwnd_next_rtt > cwnd) {  /* if bic is slower than Reno */
>> +                       delta = tcp_cwnd_next_rtt - cwnd;
>>                         max_cnt = cwnd / delta;
>>                         if (ca->cnt > max_cnt)
>>                                 ca->cnt = max_cnt;
>> --
> I'm getting a compilation error with clang:
>
> net/ipv4/tcp_cubic.c:322:7: error: mixing declarations and code
> is incompatible with standards before C99
> [-Werror,-Wdeclaration-after-statement]
>     u32 tcp_cwnd_next_rtt = ca->tcp_cwnd + (ca->ack_cnt + cwnd) / delta;
>
> Can you please try something like the following:
>
> -               u32 scale = beta_scale;
> +               u32 scale = beta_scale, tcp_cwnd_next_rtt;
> ...
> +               tcp_cwnd_next_rtt = ca->tcp_cwnd + (ca->ack_cnt + cwnd) / delta;
>
> Thanks!
> neal
Thank you, Neal,
We have tried your suggested changes, and they also work for our compile and experiment tests.
We did not find this issue because we have only tried to compile with the default Makefile with GCC.
I agree with your changes, it is conform to the existing codes and compatible with that standards.

Thanks,
Mingrui