I ran into a couple of issues while trying to tweak TCP congestion
avoidance to analyze a potential performance regression. It turns out
that overriding the parameters with ip-route(8) does not work as
expected and appears to be buggy.

Changes in v2:
- more background information in the cwnd commit message
- fix handling of initial TCP Slow Start if cwnd is clamped

Petr Tesarik (2):
  tcp_metrics: set congestion window clamp from the dst entry
  tcp_metrics: use ssthresh value from dst if there is no metrics

 net/ipv4/tcp_metrics.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

-- 
2.49.0

On Fri, Jun 20, 2025 at 5:57 AM Petr Tesarik <ptesarik@suse.com> wrote:
>
> I ran into a couple of issues while trying to tweak TCP congestion
> avoidance to analyze a potential performance regression. It turns out
> that overriding the parameters with ip-route(8) does not work as
> expected and appears to be buggy.

Hi Petr

Could you add packetdrill tests as well ?

Given this could accidentally break user setups, maybe net-next would be safer.

Some of us disable tcp_metrics, because metrics used one minute (or
few seconds) in the past are not very helpful, and source of
confusion.

(/proc/sys/net/ipv4/tcp_no_metrics_save set to 1)

On Fri, 20 Jun 2025 06:24:23 -0700
Eric Dumazet <edumazet@google.com> wrote:

> On Fri, Jun 20, 2025 at 5:57 AM Petr Tesarik <ptesarik@suse.com> wrote:
> >
> > I ran into a couple of issues while trying to tweak TCP congestion
> > avoidance to analyze a potential performance regression. It turns out
> > that overriding the parameters with ip-route(8) does not work as
> > expected and appears to be buggy.  
> 
> Hi Petr
> 
> Could you add packetdrill tests as well ?

Glad to do that. But it will be my first time. ;-) Is there a tutorial?
I looked under Documentation/ and didn't see anything.

> Given this could accidentally break user setups, maybe net-next would be safer.

Yeah, you're right. Technically, it is a bugfix, but if it's been
broken for more than a decade without anyone complaining, it can't be
super-urgent.

> Some of us disable tcp_metrics, because metrics used one minute (or
> few seconds) in the past are not very helpful, and source of
> confusion.
> 
> (/proc/sys/net/ipv4/tcp_no_metrics_save set to 1)

Yes, I know about that one. FWIW it didn't help at all in my case,
because then the value from the routing table was ALWAYS ignored...

Petr T

On Mon, Jun 23, 2025 at 12:36 AM Petr Tesarik <ptesarik@suse.com> wrote:
>
> On Fri, 20 Jun 2025 06:24:23 -0700
> Eric Dumazet <edumazet@google.com> wrote:
>
> > On Fri, Jun 20, 2025 at 5:57 AM Petr Tesarik <ptesarik@suse.com> wrote:
> > >
> > > I ran into a couple of issues while trying to tweak TCP congestion
> > > avoidance to analyze a potential performance regression. It turns out
> > > that overriding the parameters with ip-route(8) does not work as
> > > expected and appears to be buggy.
> >
> > Hi Petr
> >
> > Could you add packetdrill tests as well ?
>
> Glad to do that. But it will be my first time. ;-) Is there a tutorial?
> I looked under Documentation/ and didn't see anything.

I came up with the following test (currently working for IPv4 only).
Neal, Willem, any idea on how to have this test done for upstream tree ?

tools/testing/selftests/net/packetdrill/ksft_runner.sh does not have a
way to make a test family dependent.


diff --git a/tools/testing/selftests/net/packetdrill/tcp_cwnd5.pkt
b/tools/testing/selftests/net/packetdrill/tcp_cwnd5.pkt
new file mode 100644
index 0000000000000000000000000000000000000000..e28b63b696d200ca447f613c30003571c1ff1ae8
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_cwnd5.pkt
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+// Test of RTAX_CWND routing attribute
+
+// Set up config.
+`./defaults.sh`
+
++0 `ip ro change 192.0.2.1 via 192.168.0.1 dev tun0 cwnd lock 6`
+
+   +0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+   +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+   +0 bind(3, ..., ...) = 0
+   +0 listen(3, 1) = 0
+
+   +0 < S 0:0(0) win 32792 <mss 1000,sackOK,nop,nop,nop,wscale 7>
+   +0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 8>
+  +.1 < . 1:1(0) ack 1 win 257
+   +0 accept(3, ..., ...) = 4
+
+   +0 %{ assert tcpi_snd_cwnd == 6, tcpi_snd_cwnd }%
+
+   +0 write(4, ..., 20000) = 20000
+   +0 > P. 1:6001(6000) ack 1
+
+   +.1 < . 1:1(0) ack 6001 win 257
+
+   +0 %{ assert tcpi_snd_cwnd == 6, tcpi_snd_cwnd }%



>
> > Given this could accidentally break user setups, maybe net-next would be safer.
>
> Yeah, you're right. Technically, it is a bugfix, but if it's been
> broken for more than a decade without anyone complaining, it can't be
> super-urgent.
>
> > Some of us disable tcp_metrics, because metrics used one minute (or
> > few seconds) in the past are not very helpful, and source of
> > confusion.
> >
> > (/proc/sys/net/ipv4/tcp_no_metrics_save set to 1)
>
> Yes, I know about that one. FWIW it didn't help at all in my case,
> because then the value from the routing table was ALWAYS ignored...
>
> Petr T