netdev - Re: [PATCH net] tcp_cubic: Fix for bug in HyStart implementation in the Linux kernel

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] [day] [month] [year] [list]

Message-ID: <223960459.607292.1737102176209@mail.yahoo.com>
Date: Fri, 17 Jan 2025 08:22:56 +0000 (UTC)
From: Mahdi Arghavani <ma.arghavani@...oo.com>
To: Neal Cardwell <ncardwell@...gle.com>
Cc: Eric Dumazet <edumazet@...gle.com>, 
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, 
	Haibo Zhang <haibo.zhang@...go.ac.nz>, 
	David Eyers <david.eyers@...go.ac.nz>, 
	Abbas Arghavani <abbas.arghavani@....se>
Subject: Re: [PATCH net] tcp_cubic: Fix for bug in HyStart implementation in
 the Linux kernel

Hi Neal,
 Please find attached the updated versions of the packetdrill test files.
Best wishes,
Mahdi





On Friday, January 17, 2025 at 04:13:37 AM GMT+13, Neal Cardwell <ncardwell@...gle.com> wrote: 





On Tue, Jan 14, 2025 at 8:19 PM Mahdi Arghavani <ma.arghavani@...oo.com> wrote:
>
> Hi Neal,
> I appreciate your guidance in submitting the bug fix.
>  Attached are my updated packetdrill tests.
>
> Best wishes,
> Mahdi

Hi Mahdi,

Thanks for sharing the modified/added tests! Can you please make the
following tweaks and retest:

(1) to ease git diffs, please leave the names of the files as-is when
you modify them

(2) in cubic-bulk-166k-idle-restart.pkt, where your version of the test says:

-  +4 write(4, ..., 160000) = 160000
...
+  +0 write(4, ..., 112000) = 112000

...please change the write to actually still insert an idle period of
4 secs, so the test is still testing idle periods:

+  +4 write(4, ..., 112000) = 112000

(3) in each test, please add the following before the first injected
SYN, so we reset nstat counters:
  +0 `nstat -n`

(4) in each test, after the assertion following the "Hystart exits
slow start here" comment, please add an assertion that ssthresh has
been reduced from the TCP_INFINITE_SSTHRESH value:

+0 %{ assert tcpi_snd_ssthresh != TCP_INFINITE_SSTHRESH, tcpi_snd_ssthresh }%

(5) in each test, after the assertion following the "Hystart exits
slow start here" comment, please add a check of the nstat counters to
verify that a Hystart exit of the expected type has happened, like:

...either:
  +0 `nstat | grep TcpExtTCPHystartTrainDetect | grep -q ' 1 '`

...or:
  +0 `nstat | grep TcpExtTCPHystartDelayDetect | grep -q ' 1 '`

Can you please test something like those changes, and re-attach the
pkt tests, so we know what the behavior is after your patch? (e.g.,
which type of Hystart heuristic is triggering in each test after your

patch...)

Thanks!
neal

Download attachment "cubic-bulk-166k-idle-restart.pkt" of type "application/octet-stream" (8521 bytes)

Download attachment "cubic-bulk-166k.pkt" of type "application/octet-stream" (12896 bytes)

Download attachment "cubic-hystart-length-of-ack-train.pkt" of type "application/octet-stream" (4080 bytes)