* [TCP]: min_t/max_t confusion in tcp_select_initial_window()?
@ 2009-12-23 2:00 Roel Kluin
2009-12-23 3:47 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Roel Kluin @ 2009-12-23 2:00 UTC (permalink / raw)
To: David S. Miller, netdev
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [TCP]: min_t/max_t confusion in tcp_select_initial_window()?
2009-12-23 2:00 [TCP]: min_t/max_t confusion in tcp_select_initial_window()? Roel Kluin
@ 2009-12-23 3:47 ` David Miller
2009-12-23 19:45 ` Ilpo Järvinen
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2009-12-23 3:47 UTC (permalink / raw)
To: roel.kluin; +Cc: netdev
From: Roel Kluin <roel.kluin@gmail.com>
Date: Wed, 23 Dec 2009 03:00:24 +0100
> 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);
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [TCP]: min_t/max_t confusion in tcp_select_initial_window()?
2009-12-23 3:47 ` David Miller
@ 2009-12-23 19:45 ` Ilpo Järvinen
2009-12-23 21:15 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Ilpo Järvinen @ 2009-12-23 19:45 UTC (permalink / raw)
To: David Miller; +Cc: roel.kluin, Netdev
On Tue, 22 Dec 2009, David Miller wrote:
> From: Roel Kluin <roel.kluin@gmail.com>
> Date: Wed, 23 Dec 2009 03:00:24 +0100
>
> > 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);
>
> 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?
--
i.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [TCP]: min_t/max_t confusion in tcp_select_initial_window()?
2009-12-23 19:45 ` Ilpo Järvinen
@ 2009-12-23 21:15 ` David Miller
2009-12-23 21:23 ` Ilpo Järvinen
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2009-12-23 21:15 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: roel.kluin, netdev
From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Wed, 23 Dec 2009 21:45:57 +0200 (EET)
> On Tue, 22 Dec 2009, David Miller wrote:
>
>> From: Roel Kluin <roel.kluin@gmail.com>
>> Date: Wed, 23 Dec 2009 03:00:24 +0100
>>
>> > 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);
>> > ------------------------^^^^^
...
>
> 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?
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]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [TCP]: min_t/max_t confusion in tcp_select_initial_window()?
2009-12-23 21:15 ` David Miller
@ 2009-12-23 21:23 ` Ilpo Järvinen
2010-08-18 14:09 ` Hagen Paul Pfeifer
0 siblings, 1 reply; 6+ messages in thread
From: Ilpo Järvinen @ 2009-12-23 21:23 UTC (permalink / raw)
To: David Miller; +Cc: roel.kluin, Netdev
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1010 bytes --]
On Wed, 23 Dec 2009, David Miller wrote:
> From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
> Date: Wed, 23 Dec 2009 21:45:57 +0200 (EET)
>
> > On Tue, 22 Dec 2009, David Miller wrote:
> >
> >> From: Roel Kluin <roel.kluin@gmail.com>
> >> Date: Wed, 23 Dec 2009 03:00:24 +0100
> >>
> >> > 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);
> >> > ------------------------^^^^^
> ...
> >
> > 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?
>
> 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]
Yeah, I'm wrong, and besides misread your "It looks correct" to mean the
proposed change rather than the current code.
--
i.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [TCP]: min_t/max_t confusion in tcp_select_initial_window()?
2009-12-23 21:23 ` Ilpo Järvinen
@ 2010-08-18 14:09 ` Hagen Paul Pfeifer
0 siblings, 0 replies; 6+ messages in thread
From: Hagen Paul Pfeifer @ 2010-08-18 14:09 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: David Miller, roel.kluin, Netdev
On Wed, 23 Dec 2009 23:23:11 +0200 (EET), "Ilpo Järvinen" wrote:
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-08-18 14:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-23 2:00 [TCP]: min_t/max_t confusion in tcp_select_initial_window()? Roel Kluin
2009-12-23 3:47 ` David Miller
2009-12-23 19:45 ` Ilpo Järvinen
2009-12-23 21:15 ` David Miller
2009-12-23 21:23 ` Ilpo Järvinen
2010-08-18 14:09 ` Hagen Paul Pfeifer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).