: min_t/max_t confusion in tcp_select_initial_window()?
Subsystems (2)
I could be confused, but in net/ipv4/tcp_output.c:217:
space = max_t(u32, sysctl_tcp_rmem[2], sysctl_rmem_max);
space = min_t(u32, space, *window_clamp);------------------------^^^^^
shouldn't the min_t and max_t be exchanged? i.e.
space = min_t(u32, sysctl_tcp_rmem[2], sysctl_rmem_max);
space = max_t(u32, space, *window_clamp);Thanks, Roel
David Miller
maintainerFrom: Roel Kluin <[email protected]>
Date: Wed, 23 Dec 2009 03:00:24 +0100
Quoted 10 lines
I could be confused, but in
net/ipv4/tcp_output.c:217:space = max_t(u32, sysctl_tcp_rmem[2], sysctl_rmem_max); space = min_t(u32, space, *window_clamp);------------------------^^^^^
shouldn't the
min_tandmax_tbe exchanged? i.e.space = min_t(u32, sysctl_tcp_rmem[2], sysctl_rmem_max); space = max_t(u32, space, *window_clamp);
It looks correct to me.
Globally, the largest window we could ever end up advertising is the maximum of the sysctl_tcp_rmem[2] threshold and sysctl_rmem_max.
But for this connection, the window clamp is our upper bound.
Quoted 14 lines · David Miller
From: Roel Kluin <[email protected]>
Date: Wed, 23 Dec 2009 03:00:24 +0100Quoted 10 lines
I could be confused, but in
net/ipv4/tcp_output.c:217:space = max_t(u32, sysctl_tcp_rmem[2], sysctl_rmem_max); space = min_t(u32, space, *window_clamp);------------------------^^^^^
shouldn't the
min_tandmax_tbe exchanged? i.e.space = min_t(u32, sysctl_tcp_rmem[2], sysctl_rmem_max); space = max_t(u32, space, *window_clamp);
Quoted 7 lines
It looks correct to me.
Globally, the largest window we could ever end up advertising is the maximum of the sysctl_tcp_rmem[2] threshold and
sysctl_rmem_max.But for this connection, the window clamp is our upper bound.
I don't understand how window_clamp line could be changed to max_t. ...I guess min in both would seem the most reasonable one?
Signature · 1 line
i.
David Miller
maintainerFrom: "Ilpo Järvinen" <[email protected]>
Date: Wed, 23 Dec 2009 21:45:57 +0200 (EET)
Quoted 10 lines · David Miller
Quoted 9 lines
From: Roel Kluin <[email protected]>
Date: Wed, 23 Dec 2009 03:00:24 +0100Quoted 5 lines
I could be confused, but in
net/ipv4/tcp_output.c:217:space = max_t(u32, sysctl_tcp_rmem[2], sysctl_rmem_max); space = min_t(u32, space, *window_clamp);------------------------^^^^^
...
Quoted 3 lines
I don't understand how
window_clampline could be changed tomax_t. ...I guess min in both would seem the most reasonable one?
Nope, the first one must be max. Since I'm pretty sure we let the dynamic RX buffer resizing exceed sysctl_rmem_max if necessary.
And that's what is controlled by sysctl_tcp_rmem[2]
Quoted 15 lines · David Miller
From: "Ilpo Järvinen" <[email protected]>
Date: Wed, 23 Dec 2009 21:45:57 +0200 (EET)Quoted 10 lines · David Miller
Quoted 9 lines
From: Roel Kluin <[email protected]>
Date: Wed, 23 Dec 2009 03:00:24 +0100Quoted 5 lines
I could be confused, but in
net/ipv4/tcp_output.c:217:space = max_t(u32, sysctl_tcp_rmem[2], sysctl_rmem_max); space = min_t(u32, space, *window_clamp);------------------------^^^^^
...
Quoted 1 line
Quoted 7 lines
Quoted 2 lines
I don't understand how
window_clampline could be changed tomax_t. ...I guess min in both would seem the most reasonable one?Nope, the first one must be max. Since I'm pretty sure we let the dynamic RX buffer resizing exceed
sysctl_rmem_maxif necessary.And that's what is controlled by sysctl_tcp_rmem[2]
Yeah, I'm wrong, and besides misread your "It looks correct" to mean the proposed change rather than the current code.
Signature · 1 line
i.
Quoted 3 lines · Ilpo Järvinen
Yeah, I'm wrong, and besides misread your "It looks correct" to mean the
proposed change rather than the current code.
But the code is not correct if we consider reduced buffers via setsockopt(), some days ago I sent a patch to fix the problem.
Ilpo? David?
Hagen