Re: git: 5fb4b091e835 - main - tcp: allow specifying a MSL for local communications
Date: Fri, 27 Jun 2025 14:17:18 UTC
On 26 Jun 2025, at 19:03, Michael Tuexen wrote:
> The branch main has been updated by tuexen:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=5fb4b091e8352602894fc2b7284c8e1e3d8a8729
>
> commit 5fb4b091e8352602894fc2b7284c8e1e3d8a8729
> Author: Michael Tuexen <tuexen@FreeBSD.org>
> AuthorDate: 2025-06-26 16:59:36 +0000
> Commit: Michael Tuexen <tuexen@FreeBSD.org>
> CommitDate: 2025-06-26 16:59:36 +0000
>
> tcp: allow specifying a MSL for local communications
>
> When setting the sysctl-variable net.inet.tcp.nolocaltimewait to 1,
> which is the default, 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 times
> 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.
>
> Reviewed by: glebius, Peter Lei
> Sponsored by: Netflix, Inc.
> Differential Revision: https://reviews.freebsd.org/D50637
> ---
> share/man/man4/tcp.4 | 7 +++++++
> sys/netinet/tcp_subr.c | 1 +
> sys/netinet/tcp_timer.c | 6 ++++++
> sys/netinet/tcp_timer.h | 3 +++
> sys/netinet/tcp_timewait.c | 26 ++++++++++++++++++++++++--
> 5 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/share/man/man4/tcp.4 b/share/man/man4/tcp.4
> index 536bd904d796..f19b6cb2ae14 100644
> --- a/share/man/man4/tcp.4
> +++ b/share/man/man4/tcp.4
> @@ -780,6 +780,13 @@ Minimum TCP Maximum Segment Size; used to prevent a denial of service attack
> from an unreasonably low MSS.
> .It Va msl
> The Maximum Segment Lifetime, in milliseconds, for a packet.
> +.It Va msl_local
> +The Maximum Segment Lifetime, in milliseconds, for a packet when both endpoints
> +are local.
> +.Va msl_local
> +is only used if
> +.Va nolocaltimewait
> +is zero.
> .It Va mssdflt
> The default value used for the TCP Maximum Segment Size
> .Pq Dq MSS
> diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c
> index 6b1907305fb9..bbcd20b715ba 100644
> --- a/sys/netinet/tcp_subr.c
> +++ b/sys/netinet/tcp_subr.c
> @@ -1455,6 +1455,7 @@ tcp_vnet_init(void *arg __unused)
> VNET_PCPUSTAT_ALLOC(tcpstat, M_WAITOK);
>
> V_tcp_msl = TCPTV_MSL;
> + V_tcp_msl_local = TCPTV_MSL_LOCAL;
> arc4rand(&V_ts_offset_secret, sizeof(V_ts_offset_secret), 0);
> }
> VNET_SYSINIT(tcp_vnet_init, SI_SUB_PROTO_DOMAIN, SI_ORDER_FOURTH,
> diff --git a/sys/netinet/tcp_timer.c b/sys/netinet/tcp_timer.c
> index a9046e5725d5..32ce3001929c 100644
> --- a/sys/netinet/tcp_timer.c
> +++ b/sys/netinet/tcp_timer.c
> @@ -109,6 +109,12 @@ SYSCTL_PROC(_net_inet_tcp, OID_AUTO, msl,
> &VNET_NAME(tcp_msl), 0, sysctl_msec_to_ticks, "I",
> "Maximum segment lifetime");
>
> +VNET_DEFINE(int, tcp_msl_local);
> +SYSCTL_PROC(_net_inet_tcp, OID_AUTO, msl_local,
> + CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_VNET,
> + &VNET_NAME(tcp_msl_local), 0, sysctl_msec_to_ticks, "I",
> + "Maximum segment lifetime for local communication");
> +
> int tcp_rexmit_initial;
> SYSCTL_PROC(_net_inet_tcp, OID_AUTO, rexmit_initial, CTLTYPE_INT | CTLFLAG_RW,
> &tcp_rexmit_initial, 0, sysctl_msec_to_ticks, "I",
> diff --git a/sys/netinet/tcp_timer.h b/sys/netinet/tcp_timer.h
> index 394207bcb89b..34a0f1375463 100644
> --- a/sys/netinet/tcp_timer.h
> +++ b/sys/netinet/tcp_timer.h
> @@ -74,6 +74,7 @@
> * Time constants.
> */
> #define TCPTV_MSL MSEC_2_TICKS(30000) /* max seg lifetime (hah!) */
> +#define TCPTV_MSL_LOCAL MSEC_2_TICKS(10) /* max seg lifetime for local comm */
> #define TCPTV_SRTTBASE 0 /* base roundtrip time;
> if 0, no idea yet */
> #define TCPTV_RTOBASE MSEC_2_TICKS(1000) /* assumed RTO if no info */
> @@ -183,6 +184,8 @@ VNET_DECLARE(int, tcp_v6pmtud_blackhole_mss);
> #define V_tcp_v6pmtud_blackhole_mss VNET(tcp_v6pmtud_blackhole_mss)
> VNET_DECLARE(int, tcp_msl);
> #define V_tcp_msl VNET(tcp_msl)
> +VNET_DECLARE(int, tcp_msl_local);
> +#define V_tcp_msl_local VNET(tcp_msl_local)
>
> #endif /* _KERNEL */
>
> diff --git a/sys/netinet/tcp_timewait.c b/sys/netinet/tcp_timewait.c
> index 2b4ae462af89..9f2943725ef0 100644
> --- a/sys/netinet/tcp_timewait.c
> +++ b/sys/netinet/tcp_timewait.c
> @@ -93,6 +93,28 @@ SYSCTL_BOOL(_net_inet_tcp, OID_AUTO, nolocaltimewait,
> CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(nolocaltimewait), true,
> "Do not create TCP TIME_WAIT state for local connections");
>
> +static u_int
> +tcp_msl(struct tcpcb *tp)
> +{
> + struct inpcb *inp = tptoinpcb(tp);
> +#ifdef INET6
> + bool isipv6 = inp->inp_inc.inc_flags & INC_ISIPV6;
> +#endif
> +
> + if (
> +#ifdef INET6
> + isipv6 ? in6_localip(&inp->in6p_faddr) :
> +#endif
> +#ifdef INET
> + in_localip(inp->inp_faddr))
> +#else
> + false)
> +#endif
> + return (V_tcp_msl_local);
> + else
> + return (V_tcp_msl);
> +}
> +
This seems to make !VIMAGE builds unhappy, probably because V_tcp_msl becomes tcp_msl then, and a function and a variable with the same name confuses the poor compiler.
LINT-NOVIMAGE:
--- tcp_timewait.o ---
/usr/src/sys/netinet/tcp_timewait.c:97:1: error: redefinition of 'tcp_msl' as different kind of symbol
97 | tcp_msl(struct tcpcb *tp)
| ^
/usr/src/sys/netinet/tcp_timer.h:185:19: note: previous definition is here
185 | VNET_DECLARE(int, tcp_msl);
| ^
/usr/src/sys/netinet/tcp_timewait.c:165:45: error: called object type 'int' is not a function or function pointer
165 | tcp_timer_activate(tp, TT_2MSL, 2 * tcp_msl(tp));
| ~~~~~~~^
/usr/src/sys/netinet/tcp_timewait.c:308:47: error: called object type 'int' is not a function or function pointer
308 | tcp_timer_activate(tp, TT_2MSL, 2 * tcp_msl(tp));
| ~~~~~~~^
3 errors generated.
—
Kristof