David,
I see a recent change in inet_connection_sock.c that uses sysctl_tcp_synack_retries suspiciously.
Previously, sysctl_tcp_synack_retries was a global variable, and now it has been moved into the fragment (really netns_ipv4).
In reqsk_timer_handler (inet_connection_sock.c) near line 563 it is used as the default max_retries value in case icsk->icsk_syn_retries is not set.
Previously, other transports (dccp?) would use the TCP global variable as a default. Now that the defaults come from the fragment, I suspect the fragment variable is not set for other transports. (I can't find where it's set for dccp.)
Of course, this will only show in non-TCP transport protocols when the icsk retries value is not set, so it's a rare case. Perhaps it's an unreachable case, since I don't know all the kernel paths.
Maybe the problem is that the default shouldn't be to a TCP value, but should be a 'transport' value.
This code is somewhat convoluted, so I am not sure of my analysis, but I wanted you to consider it.
John Light
Intel OTC comms
On Wed, 2016-03-30 at 21:05 +0000, Light, John J wrote:
David,
I see a recent change in inet_connection_sock.c that uses
sysctl_tcp_synack_retries suspiciously.
Previously, sysctl_tcp_synack_retries was a global variable, and now
it has been moved into the fragment (really netns_ipv4).
You mean the network namespace ?
In reqsk_timer_handler (inet_connection_sock.c) near line 563 it is
used as the default max_retries value in case icsk->icsk_syn_retries
is not set.
Previously, other transports (dccp?) would use the TCP global variable
as a default. Now that the defaults come from the fragment, I suspect
the fragment variable is not set for other transports. (I can't find
where it's set for dccp.)
Of course, this will only show in non-TCP transport protocols when the
icsk retries value is not set, so it's a rare case. Perhaps it's an
unreachable case, since I don't know all the kernel paths.
Maybe the problem is that the default shouldn't be to a TCP value, but
should be a 'transport' value.
This code is somewhat convoluted, so I am not sure of my analysis, but
I wanted you to consider it.
No idea why you ask David instead of patch author ?
I don't see any problem here.
Do we really want to spend time on this minor issue ?
On Wed, Mar 30, 2016 at 2:05 PM, Light, John J [off-list ref] wrote:
Maybe the problem is that the default shouldn't be to a TCP value, but should be a 'transport' value.
The code is fine because net->ipv4 is always there, it doesn't
depend on any CONFIG, so it is safe for dccp to use too, although
I agree it is a bit ugly.