tcp: allow specifying a MSL for local communications

[tuexen]

Summary

When setting the sysctl-variable net.inet.tcp.nolocaltimewait to 1, a TCP endpoint does not enter the TIME-WAIT state, when the communication is local. This can result in sending RST-segments without any error situation. By setting the sysctl-variable net.inet.tcp.nolocaltimewait to 0, this does not occur, and the behavior is compliant with the TCP specification. But there is no reason to stay in the TIME-WAIT state for two time the value of the sysctl-variable net.inet.tcp.msl, if the communication is local. Therefore provide a separate sysctl- variable net.inet.tcp.msl_local, which controls how long an TCP end-point stays in the TIME-WAIT state, if the communication is local. The default value is 10 ms.

[peter.lei_ieee.org]

share/man/man4/tcp.4

785

Should this reference nolocaltimewait to be explicit - i.e. msl_local is only used if nolocaltimewait is zero for local endpoints ?

[glebius]

Thanks Michael! IMHO, modification of the old nolocaltimewait should print an obsolete message and set local_msl to 0. This can be done separately. I can add this.

[tuexen]

In D50637#1159289, @glebius wrote:

Thanks Michael! IMHO, modification of the old nolocaltimewait should print an obsolete message and set local_msl to 0. This can be done separately. I can add this.

I wanted to obsolete the old nolocaltimewait in a follow-up commit after discussing the details of that procedure on the fortnightly transport VC.

[tuexen]

Address the comment provided by Peter.

[tuexen]

share/man/man4/tcp.4

785

Fixed.

[glebius]

In D50637#1159294, @tuexen wrote:

In D50637#1159289, @glebius wrote:

Thanks Michael! IMHO, modification of the old nolocaltimewait should print an obsolete message and set local_msl to 0. This can be done separately. I can add this.

I wanted to obsolete the old nolocaltimewait in a follow-up commit after discussing the details of that procedure on the fortnightly transport VC.

Great! I will not attend the call tomorrow. I think just throwing

gone_in(16, "The net.inet.tcp.nolocaltimewait is obsolete. Use net.inet.tcp.local_msl instead.")

in the sysctl handler should be fine. Note that I got review that will make gone_in() print a message only once.

[glebius]

sys/netinet/tcp_timer.h

77

Would be a bug if hz < 100. Not sure we support that, though.

[cc]

sys/netinet/tcp_timewait.c

100–112

Can we move this piece of code into an individual function, like this?

static bool
is_ip_local(struct inpcb *inp)
{
#ifdef INET6
	bool isipv6 = inp->inp_inc.inc_flags & INC_ISIPV6;
#endif

	return (
#ifdef INET6
	    isipv6 ? in6_localip(&inp->in6p_faddr) :
#endif
#ifdef INET
	    in_localip(inp->inp_faddr));
#else
	    false);
#endif
}

And then use is_ip_local() in this tcp_msl() function.

151–158

First, I think we shall use in6_localip(), instead of in6_localaddr().

Second, we can cleanup and re-use the proposed is_ip_local() from my comment above.

[glebius]

sys/netinet/tcp_timewait.c

100–112

I don't agree with this suggestion. A single time use function decreases readability. Also, suggested function works on a higher level structure inpcb, so logically belongs to in_pcb.c namespace. But if we move it there it can't be inlined neither optimized.

151–158

First, I think we shall use in6_localip(), instead of in6_localaddr().

You raise a good point here. The current use of in6_localaddr() was copied from net.inet.tcp.nolocaltimewait code, which I created without using much analysis of which function should I use to tell a local IPv6 address. Now, comparing these two functions I'm a bit lost. What's the principal difference between them? The difference should be reflected at least in comments.

P.S. The IPv6 local address database definitely needs to get rid of rmlock(9), it shall already be protected by the epoch.

[tuexen]

sys/netinet/tcp_timewait.c

151–158

Good catch! There are a couple of places, in which we use incorrectly in6_localaddr() instead of in6_localip().
The difference is:

• in6_localaddr() returns true, iff the argument is an address in the LAN. I.e. can be reached without routing. This means local as in Local Area Network (LAN).
• in6_localip() returns true, iff the argument is an address configure on the host. This means local as in local host.

in6_localaddr() corresponds to in_localaddr() and in6_localip() corresponds to in_localip().
Let me fix these issues first...

[tuexen]

sys/netinet/tcp_timewait.c

151–158

Here are the reviews:

• D50828
• D50829
• D50830

[tuexen]

Address issue raise by cc.

[tuexen]

sys/netinet/tcp_timewait.c

151–158

The patch is now using in6_localip().

[tuexen]

sys/netinet/tcp_timewait.c

100–112

The plan is to remove the nolocaltimewait functionality in the long term. Then there is no reuse of the function. So I agree with glebius.

[tuexen]

sys/netinet/tcp_timer.h

77

I will change this to

#define TCPTV_MSL_LOCAL MSEC_2_TICKS(10)

which is simpler to read and avoids problems with hz values smaller then 100.
Before doing so, I suggest to change all other timer settings here to the same style for the same reason in D50840.

[tuexen]

Address comment made by glebius.

sys/netinet/tcp_timer.h

77

Now fixed.

[glebius]

sys/netinet/tcp_timer.h

77

Thanks!