Date: Fri, 27 Jun 2025 23:19:14 +0200 From: Michael Tuexen <tuexen@FreeBSD.org> To: Kristof Provost <kp@FreeBSD.org> Cc: "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org> Subject: Re: git: 5fb4b091e835 - main - tcp: allow specifying a MSL for local communications Message-ID: <ECDA9479-DA21-4340-9417-CCEC7F94C8CE@FreeBSD.org> In-Reply-To: <8EF254D7-A8D7-4031-BAC0-FDC36DE22754@FreeBSD.org> References: <202506261703.55QH3fqK060218@gitrepo.freebsd.org> <8EF254D7-A8D7-4031-BAC0-FDC36DE22754@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> On 27. Jun 2025, at 16:17, Kristof Provost <kp@FreeBSD.org> wrote: >=20 > On 26 Jun 2025, at 19:03, Michael Tuexen wrote: >> The branch main has been updated by tuexen: >>=20 >> URL: = https://cgit.FreeBSD.org/src/commit/?id=3D5fb4b091e8352602894fc2b7284c8e1e= 3d8a8729 >>=20 >> 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 >>=20 >> tcp: allow specifying a MSL for local communications >>=20 >> 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. >>=20 >> 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(-) >>=20 >> 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); >>=20 >> V_tcp_msl =3D TCPTV_MSL; >> + V_tcp_msl_local =3D 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"); >>=20 >> +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) >>=20 >> #endif /* _KERNEL */ >>=20 >> 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"); >>=20 >> +static u_int >> +tcp_msl(struct tcpcb *tp) >> +{ >> + struct inpcb *inp =3D tptoinpcb(tp); >> +#ifdef INET6 >> + bool isipv6 =3D 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. >=20 > LINT-NOVIMAGE: >=20 > --- 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. Hi Kristof, thank you very much for reporting the issue. It should be fixed by: = https://cgit.FreeBSD.org/src/commit/?id=3Dea6376adc8b969936259537ed8942131= 62b376e8 = <https://cgit.freebsd.org/src/commit/?id=3Dea6376adc8b969936259537ed894213= 162b376e8> Sorry for the breakage. Best regards Michael >=20 > =E2=80=94 > Kristof
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?ECDA9479-DA21-4340-9417-CCEC7F94C8CE>