Re: [PATCH net-next 1/4] tcp: sync MSS after enabling MTU probing on passive connections

[Kuniyuki Iwashima <kuniyu@google.com>]

On Sat, 07 Nov 2026 12:53:36 -0500, Neal Cardwell wrote:
> On Tue, Nov 3, 2026 at 2:09 AM Yongjian Wang via B4 Relay
> <devnull+echoowall.gmail.com@kernel.org> wrote:
> >
> > From: Yongjian Wang <echoowall@gmail.com>
> >
> > tcp_v4_syn_recv_sock() and tcp_v6_syn_recv_sock() call tcp_sync_mss()
> > before MTU probing is enabled, and tcp_init_transfer() enables it later
> > without syncing the MSS again.
> >
> > With net.ipv4.tcp_mtu_probing=2, a passive connection therefore starts
> > at the full MSS instead of tcp_base_mss as documented, while an active
> > connection starts at tcp_base_mss. This has two visible effects, shown
> > here with tcp_base_mss=1024:
> >
> >  - The MSS drops when the first probe is acknowledged, from 1460 to 1242,
> >    although that probe has just shown the path can carry more.
> >
> >  - On a black hole path the MSS never tries tcp_base_mss. It goes from
> >    1460 straight to 512, since MTU probing is already enabled when black
> >    hole detection triggers.
> >
> > Sync the MSS right after tcp_mtup_init(). tcp_sync_mss() only applies
> > search_low when MTU probing is enabled, so the other tcp_mtu_probing
> > values keep their behaviour.
> >
> > The Fixes commit kept tcp_mtup_init() in a single place on purpose, so
> > sync the MSS next to it instead of moving it back into the
> > syn_recv_sock() handlers.
> >
> > Fixes: 4d83e1773031 ("tcp: delete redundant calls of tcp_mtup_init()")
> > Signed-off-by: Yongjian Wang <echoowall@gmail.com>
> > ---
> >  net/ipv4/tcp_input.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > index e1f2d9765..c13f005b2 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -6760,6 +6760,7 @@ void tcp_init_transfer(struct sock *sk, int bpf_op, struct sk_buff *skb)
> >         struct tcp_sock *tp = tcp_sk(sk);
> >
> >         tcp_mtup_init(sk);
> > +       tcp_sync_mss(sk, icsk->icsk_pmtu_cookie);
> >         icsk->icsk_af_ops->rebuild_header(sk);
> >         tcp_init_metrics(sk);
> >
> >
> > --
> 
> Functionally this seems correct. But I'm wondering about the
> performance impact, and whether there is a way to achieve this without
> adding this call to the code path for every connection establishment?
> 
> Eric and Kuniyuki, WDYT?

Looking at history, initially it seems we properly called
tcp_mtup_init() right before tcp_sync_mss() in ->syn_recv_sock(),
but 4d83e1773031 removed them.

The dup tcp_mtup_init() mentioned in the commit is the one in
tcp_init_transfer() today.

Then, 27204aaa9dc6 moved only tcp_mtup_init() to tcp_init_transfer()
without tcp_sync_mss().

The dup exists in tcp_rcv_state_process() from day 0 (5d424d5a674f),
and 168a8f58059a copy-and-pasted the unnecessary tcp_mtup_init().

So, 4d83e1773031 removed the correct one.

Instead of restoring the 2nd set of calls, I think we should
revert 4d83e1773031 and remove tcp_mtup_init() in tcp_init_transfer().

---8<---
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index e1f2d9765f81..8d009c0d9359 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -6759,7 +6759,6 @@ void tcp_init_transfer(struct sock *sk, int bpf_op, struct sk_buff *skb)
 	struct inet_connection_sock *icsk = inet_csk(sk);
 	struct tcp_sock *tp = tcp_sk(sk);
 
-	tcp_mtup_init(sk);
 	icsk->icsk_af_ops->rebuild_header(sk);
 	tcp_init_metrics(sk);
 
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index a7f9242f1e42..579a88472abf 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1740,6 +1740,7 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
 
 	tcp_ca_openreq_child(newsk, dst);
 
+	tcp_mtup_init(newsk);
 	tcp_sync_mss(newsk, dst4_mtu(dst));
 	newtp->advmss = tcp_mss_clamp(tcp_sk(sk), tcp_dst_advmss(dst));
 
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index e006a30360ea..c465fd97baec 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1489,6 +1489,7 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
 
 	tcp_ca_openreq_child(newsk, dst);
 
+	tcp_mtup_init(newsk);
 	tcp_sync_mss(newsk, dst6_mtu(dst));
 	newtp->advmss = tcp_mss_clamp(tcp_sk(sk), tcp_dst_advmss(dst));
 
---8<---