[PATCH net-next 0/4] tcp: fix tcp_base_mss on passive connections and test MTU probing

[PATCH net-next 0/4] tcp: fix tcp_base_mss on passive connections and test MTU probing

[Yongjian Wang]

While adding selftests for MTU probing, I noticed that passive
connections do not start at tcp_base_mss with tcp_mtu_probing=2.
Patch 1 fixes that and patch 2 adds a regression test.

This has been wrong since 2014 and only matters with a non-default
sysctl, so I am sending it to net-next. Patch 1 changes behaviour for
users of tcp_mtu_probing=2. Please let me know if you would prefer it
to go through net or to be backported to stable.

Patches 3 and 4 add the missing coverage for black hole detection
(tcp_mtu_probing=1) and for the MTU search on active connections
(tcp_mtu_probing=2).

---
Yongjian Wang (4):
      tcp: sync MSS after enabling MTU probing on passive connections
      selftests/net: packetdrill: test tcp_base_mss on passive connections
      selftests/net: packetdrill: add TCP black hole detection test
      selftests/net: packetdrill: add TCP MTU probing test

 net/ipv4/tcp_input.c                               |  1 +
 .../net/packetdrill/tcp_mtu_probing_blackhole.pkt  | 37 +++++++++++++++++++
 .../net/packetdrill/tcp_mtu_probing_passive.pkt    | 27 ++++++++++++++
 .../net/packetdrill/tcp_mtu_probing_search.pkt     | 43 ++++++++++++++++++++++
 4 files changed, 108 insertions(+)
---
base-commit: 4dd16e31135ea8437a720222df34e8745f385d6d
change-id: 20261103-mtu-probing-series-a87b39fdbedf

Best regards,
--  
Yongjian Wang <echoowall@gmail.com>

[PATCH net-next 0/4] tcp: fix tcp_base_mss on passive connections and test MTU probing

[Yongjian Wang via B4 Relay]

While adding selftests for MTU probing, I noticed that passive
connections do not start at tcp_base_mss with tcp_mtu_probing=2.
Patch 1 fixes that and patch 2 adds a regression test.

This has been wrong since 2014 and only matters with a non-default
sysctl, so I am sending it to net-next. Patch 1 changes behaviour for
users of tcp_mtu_probing=2. Please let me know if you would prefer it
to go through net or to be backported to stable.

Patches 3 and 4 add the missing coverage for black hole detection
(tcp_mtu_probing=1) and for the MTU search on active connections
(tcp_mtu_probing=2).

---
Yongjian Wang (4):
      tcp: sync MSS after enabling MTU probing on passive connections
      selftests/net: packetdrill: test tcp_base_mss on passive connections
      selftests/net: packetdrill: add TCP black hole detection test
      selftests/net: packetdrill: add TCP MTU probing test

 net/ipv4/tcp_input.c                               |  1 +
 .../net/packetdrill/tcp_mtu_probing_blackhole.pkt  | 37 +++++++++++++++++++
 .../net/packetdrill/tcp_mtu_probing_passive.pkt    | 27 ++++++++++++++
 .../net/packetdrill/tcp_mtu_probing_search.pkt     | 43 ++++++++++++++++++++++
 4 files changed, 108 insertions(+)
---
base-commit: 4dd16e31135ea8437a720222df34e8745f385d6d
change-id: 20261103-mtu-probing-series-a87b39fdbedf

Best regards,
--  
Yongjian Wang <echoowall@gmail.com>

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

[Yongjian Wang]

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);
 

-- 
Git-157)

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

[Yongjian Wang via B4 Relay]

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);
 

-- 
Git-157)

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

[Neal Cardwell]

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?

neal

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

[Kuniyuki Iwashima]

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<---

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

[Neal Cardwell]

On Sat, Nov 7, 2026 at 2:15 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> 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<---

Thanks, Kuniyuki. I agree that seems like a good approach. Though
since 27204aaa9dc6 also moved the tcp_mtup_init() out of
tcp_rcv_synsent_state_process(), I think with your nice approach we'd
also want to place a call to tcp_mtup_init(sk) right before the
tcp_sync_mss() call on the active connect code path as well, something
like:

--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -7015,6 +7015,7 @@ static int tcp_rcv_synsent_state_process(struct
sock *sk, struct sk_buff *skb,
                  tp->tcp_header_len = sizeof(struct tcphdr);
          }

+        tcp_mtup_init(sk);
          tcp_sync_mss(sk, icsk->icsk_pmtu_cookie);
          tcp_initialize_rcv_mss(sk);

WDYT?

neal

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

[Kuniyuki Iwashima]

On Sat, Nov 7, 2026 at 7:17 PM Neal Cardwell <ncardwell@google.com> wrote:
>
> On Sat, Nov 7, 2026 at 2:15 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> >
> > 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<---
>
> Thanks, Kuniyuki. I agree that seems like a good approach. Though
> since 27204aaa9dc6 also moved the tcp_mtup_init() out of
> tcp_rcv_synsent_state_process(), I think with your nice approach we'd
> also want to place a call to tcp_mtup_init(sk) right before the
> tcp_sync_mss() call on the active connect code path as well, something
> like:
>
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -7015,6 +7015,7 @@ static int tcp_rcv_synsent_state_process(struct
> sock *sk, struct sk_buff *skb,
>                   tp->tcp_header_len = sizeof(struct tcphdr);
>           }
>
> +        tcp_mtup_init(sk);
>           tcp_sync_mss(sk, icsk->icsk_pmtu_cookie);
>           tcp_initialize_rcv_mss(sk);
>
> WDYT?

Ah exactly, thanks for catching :)

[PATCH net-next 2/4] selftests/net: packetdrill: test tcp_base_mss on passive connections

[Yongjian Wang]

Check that a passive connection starts at tcp_base_mss with
net.ipv4.tcp_mtu_probing=2, as an active connection does.

Signed-off-by: Yongjian Wang <echoowall@gmail.com>
---
 .../net/packetdrill/tcp_mtu_probing_passive.pkt    | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_passive.pkt b/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_passive.pkt
new file mode 100644
index 000000000..14bae144c
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_passive.pkt
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0
+// Test that a passive connection starts at tcp_base_mss with tcp_mtu_probing=2,
+// the same way as an active connection.
+
+`./defaults.sh
+sysctl -q net.ipv4.tcp_mtu_probing=2
+sysctl -q net.ipv4.tcp_base_mss=1024
+sysctl -q net.ipv4.tcp_timestamps=0`
+
+    0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+   +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+   +0 bind(3, ..., ...) = 0
+   +0 listen(3, 1) = 0
+
+   +0 < S 0:0(0) win 65535 <mss 1460,nop,nop,sackOK>
+   +0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK>
+  +.1 < . 1:1(0) ack 1 win 65535
+   +0 accept(3, ..., ...) = 4
+
+// The initial window is sent with tcp_base_mss, not the peer's MSS.
+   +0 write(4, ..., 10240) = 10240
+   +0 > P. 1:2049(2048) ack 1
+   +0 > P. 2049:4097(2048) ack 1
+   +0 > P. 4097:6145(2048) ack 1
+   +0 > P. 6145:8193(2048) ack 1
+   +0 > P. 8193:10241(2048) ack 1
+  +.1 < . 1:1(0) ack 10241 win 65535

-- 
Git-157)

[PATCH net-next 2/4] selftests/net: packetdrill: test tcp_base_mss on passive connections

[Yongjian Wang via B4 Relay]

From: Yongjian Wang <echoowall@gmail.com>

Check that a passive connection starts at tcp_base_mss with
net.ipv4.tcp_mtu_probing=2, as an active connection does.

Signed-off-by: Yongjian Wang <echoowall@gmail.com>
---
 .../net/packetdrill/tcp_mtu_probing_passive.pkt    | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_passive.pkt b/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_passive.pkt
new file mode 100644
index 000000000..14bae144c
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_passive.pkt
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0
+// Test that a passive connection starts at tcp_base_mss with tcp_mtu_probing=2,
+// the same way as an active connection.
+
+`./defaults.sh
+sysctl -q net.ipv4.tcp_mtu_probing=2
+sysctl -q net.ipv4.tcp_base_mss=1024
+sysctl -q net.ipv4.tcp_timestamps=0`
+
+    0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+   +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+   +0 bind(3, ..., ...) = 0
+   +0 listen(3, 1) = 0
+
+   +0 < S 0:0(0) win 65535 <mss 1460,nop,nop,sackOK>
+   +0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK>
+  +.1 < . 1:1(0) ack 1 win 65535
+   +0 accept(3, ..., ...) = 4
+
+// The initial window is sent with tcp_base_mss, not the peer's MSS.
+   +0 write(4, ..., 10240) = 10240
+   +0 > P. 1:2049(2048) ack 1
+   +0 > P. 2049:4097(2048) ack 1
+   +0 > P. 4097:6145(2048) ack 1
+   +0 > P. 6145:8193(2048) ack 1
+   +0 > P. 8193:10241(2048) ack 1
+  +.1 < . 1:1(0) ack 10241 win 65535

-- 
Git-157)

[PATCH net-next 3/4] selftests/net: packetdrill: add TCP black hole detection test

[Yongjian Wang]

Black hole detection (net.ipv4.tcp_mtu_probing=1) has no selftest
coverage.

Add a test where a full-sized segment keeps timing out without any ICMP
feedback. Check that the MSS drops to tcp_base_mss once tcp_retries1 is
exceeded, and that the smaller MSS is also used for new data after the
retransmission is acknowledged.

Set tcp_retries1 to 1 to keep the test short, and disable TLP so that
every retransmission comes from the RTO timer, which is what
tcp_write_timeout() counts.

Signed-off-by: Yongjian Wang <echoowall@gmail.com>
---
 .../net/packetdrill/tcp_mtu_probing_blackhole.pkt  | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_blackhole.pkt b/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_blackhole.pkt
new file mode 100644
index 000000000..f10dbb36d
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_blackhole.pkt
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0
+// Test black hole detection with tcp_mtu_probing=1: when full-sized segments
+// keep timing out and no ICMP arrives, TCP falls back to tcp_base_mss.
+
+`./defaults.sh
+sysctl -q net.ipv4.tcp_mtu_probing=1
+sysctl -q net.ipv4.tcp_base_mss=1024
+sysctl -q net.ipv4.tcp_retries1=1
+sysctl -q net.ipv4.tcp_early_retrans=0`
+
+    0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+   +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+   +0 bind(3, ..., ...) = 0
+   +0 listen(3, 1) = 0
+
+   +0 < S 0:0(0) win 65535 <mss 1460,nop,nop,sackOK>
+   +0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK>
+  +.1 < . 1:1(0) ack 1 win 65535
+   +0 accept(3, ..., ...) = 4
+
+   +0 write(4, ..., 1460) = 1460
+   +0 > P. 1:1461(1460) ack 1
+
+// The first retransmission keeps the full size.
+  +.3 > P. 1:1461(1460) ack 1
+
+// Once tcp_retries1 is exceeded the MSS drops to tcp_base_mss.
+  +.6 > . 1:1025(1024) ack 1
+  +.1 < . 1:1(0) ack 1025 win 65535
+   +0 > P. 1025:1461(436) ack 1
+  +.1 < . 1:1(0) ack 1461 win 65535
+
+// New data keeps using the smaller MSS.
+   +0 write(4, ..., 1460) = 1460
+   +0 > . 1461:2485(1024) ack 1
+   +0 > P. 2485:2921(436) ack 1
+  +.1 < . 1:1(0) ack 2921 win 65535

-- 
Git-157)

[PATCH net-next 3/4] selftests/net: packetdrill: add TCP black hole detection test

[Yongjian Wang via B4 Relay]

From: Yongjian Wang <echoowall@gmail.com>

Black hole detection (net.ipv4.tcp_mtu_probing=1) has no selftest
coverage.

Add a test where a full-sized segment keeps timing out without any ICMP
feedback. Check that the MSS drops to tcp_base_mss once tcp_retries1 is
exceeded, and that the smaller MSS is also used for new data after the
retransmission is acknowledged.

Set tcp_retries1 to 1 to keep the test short, and disable TLP so that
every retransmission comes from the RTO timer, which is what
tcp_write_timeout() counts.

Signed-off-by: Yongjian Wang <echoowall@gmail.com>
---
 .../net/packetdrill/tcp_mtu_probing_blackhole.pkt  | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_blackhole.pkt b/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_blackhole.pkt
new file mode 100644
index 000000000..f10dbb36d
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_blackhole.pkt
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0
+// Test black hole detection with tcp_mtu_probing=1: when full-sized segments
+// keep timing out and no ICMP arrives, TCP falls back to tcp_base_mss.
+
+`./defaults.sh
+sysctl -q net.ipv4.tcp_mtu_probing=1
+sysctl -q net.ipv4.tcp_base_mss=1024
+sysctl -q net.ipv4.tcp_retries1=1
+sysctl -q net.ipv4.tcp_early_retrans=0`
+
+    0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+   +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+   +0 bind(3, ..., ...) = 0
+   +0 listen(3, 1) = 0
+
+   +0 < S 0:0(0) win 65535 <mss 1460,nop,nop,sackOK>
+   +0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK>
+  +.1 < . 1:1(0) ack 1 win 65535
+   +0 accept(3, ..., ...) = 4
+
+   +0 write(4, ..., 1460) = 1460
+   +0 > P. 1:1461(1460) ack 1
+
+// The first retransmission keeps the full size.
+  +.3 > P. 1:1461(1460) ack 1
+
+// Once tcp_retries1 is exceeded the MSS drops to tcp_base_mss.
+  +.6 > . 1:1025(1024) ack 1
+  +.1 < . 1:1(0) ack 1025 win 65535
+   +0 > P. 1025:1461(436) ack 1
+  +.1 < . 1:1(0) ack 1461 win 65535
+
+// New data keeps using the smaller MSS.
+   +0 write(4, ..., 1460) = 1460
+   +0 > . 1461:2485(1024) ack 1
+   +0 > P. 2485:2921(436) ack 1
+  +.1 < . 1:1(0) ack 2921 win 65535

-- 
Git-157)

[PATCH net-next 4/4] selftests/net: packetdrill: add TCP MTU probing test

[Yongjian Wang]

Packetization layer path MTU discovery (net.ipv4.tcp_mtu_probing=2) has
no selftest coverage.

Add a test for an active connection. Check that it starts at
tcp_base_mss, that it sends the first probe half way between
tcp_base_mss and the peer's MSS once cwnd allows it, and that an
acknowledged probe raises the MSS and leads to a larger probe.

Disable TCP timestamps since the MTU search range is computed before the
timestamp option is negotiated, which would otherwise make the MSS
depend on the header length at connect time.

Signed-off-by: Yongjian Wang <echoowall@gmail.com>
---
 .../net/packetdrill/tcp_mtu_probing_search.pkt     | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_search.pkt b/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_search.pkt
new file mode 100644
index 000000000..65e466fbf
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_search.pkt
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+// Test packetization layer path MTU discovery with tcp_mtu_probing=2: an
+// active connection starts at tcp_base_mss and raises its MSS by binary search
+// as probes are acknowledged.
+
+`./defaults.sh
+sysctl -q net.ipv4.tcp_mtu_probing=2
+sysctl -q net.ipv4.tcp_base_mss=1024
+sysctl -q net.ipv4.tcp_timestamps=0`
+
+    0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+   +0 fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK) = 0
+   +0 connect(3, ..., ...) = -1 EINPROGRESS (Operation is now in progress)
+   +0 > S 0:0(0) <...>
+  +.1 < S. 0:0(0) ack 1 win 65535 <mss 1460,nop,nop,sackOK>
+   +0 > . 1:1(0) ack 1
+   +0 fcntl(3, F_SETFL, O_RDWR) = 0
+
+// The initial window is sent with tcp_base_mss. No probe yet, cwnd is below 11.
+   +0 write(3, ..., 40000) = 40000
+   +0 > P. 1:2049(2048) ack 1
+   +0 > P. 2049:4097(2048) ack 1
+   +0 > P. 4097:6145(2048) ack 1
+   +0 > P. 6145:8193(2048) ack 1
+   +0 > P. 8193:10241(2048) ack 1
+
+// The first probe is half way between tcp_base_mss and the peer's MSS.
+  +.1 < . 1:1(0) ack 10241 win 65535
+   +0 > . 10241:11483(1242) ack 1
+   +0 > P. 11483:13531(2048) ack 1
+   +0 > P. 13531:15579(2048) ack 1
+   +0 > P. 15579:17627(2048) ack 1
+   +0 > P. 17627:19675(2048) ack 1
+   +0 > P. 19675:21723(2048) ack 1
+   +0 > P. 21723:23771(2048) ack 1
+   +0 > P. 23771:25819(2048) ack 1
+   +0 > P. 25819:27867(2048) ack 1
+   +0 > P. 27867:29915(2048) ack 1
+
+// Acknowledging the probe raises the MSS, and the next probe is larger.
+  +.1 < . 1:1(0) ack 29915 win 65535
+   +0 > . 29915:31266(1351) ack 1
+   +0 > P. 31266:33750(2484) ack 1

-- 
Git-157)

[PATCH net-next 4/4] selftests/net: packetdrill: add TCP MTU probing test

[Yongjian Wang via B4 Relay]

From: Yongjian Wang <echoowall@gmail.com>

Packetization layer path MTU discovery (net.ipv4.tcp_mtu_probing=2) has
no selftest coverage.

Add a test for an active connection. Check that it starts at
tcp_base_mss, that it sends the first probe half way between
tcp_base_mss and the peer's MSS once cwnd allows it, and that an
acknowledged probe raises the MSS and leads to a larger probe.

Disable TCP timestamps since the MTU search range is computed before the
timestamp option is negotiated, which would otherwise make the MSS
depend on the header length at connect time.

Signed-off-by: Yongjian Wang <echoowall@gmail.com>
---
 .../net/packetdrill/tcp_mtu_probing_search.pkt     | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_search.pkt b/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_search.pkt
new file mode 100644
index 000000000..65e466fbf
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_mtu_probing_search.pkt
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+// Test packetization layer path MTU discovery with tcp_mtu_probing=2: an
+// active connection starts at tcp_base_mss and raises its MSS by binary search
+// as probes are acknowledged.
+
+`./defaults.sh
+sysctl -q net.ipv4.tcp_mtu_probing=2
+sysctl -q net.ipv4.tcp_base_mss=1024
+sysctl -q net.ipv4.tcp_timestamps=0`
+
+    0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+   +0 fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK) = 0
+   +0 connect(3, ..., ...) = -1 EINPROGRESS (Operation is now in progress)
+   +0 > S 0:0(0) <...>
+  +.1 < S. 0:0(0) ack 1 win 65535 <mss 1460,nop,nop,sackOK>
+   +0 > . 1:1(0) ack 1
+   +0 fcntl(3, F_SETFL, O_RDWR) = 0
+
+// The initial window is sent with tcp_base_mss. No probe yet, cwnd is below 11.
+   +0 write(3, ..., 40000) = 40000
+   +0 > P. 1:2049(2048) ack 1
+   +0 > P. 2049:4097(2048) ack 1
+   +0 > P. 4097:6145(2048) ack 1
+   +0 > P. 6145:8193(2048) ack 1
+   +0 > P. 8193:10241(2048) ack 1
+
+// The first probe is half way between tcp_base_mss and the peer's MSS.
+  +.1 < . 1:1(0) ack 10241 win 65535
+   +0 > . 10241:11483(1242) ack 1
+   +0 > P. 11483:13531(2048) ack 1
+   +0 > P. 13531:15579(2048) ack 1
+   +0 > P. 15579:17627(2048) ack 1
+   +0 > P. 17627:19675(2048) ack 1
+   +0 > P. 19675:21723(2048) ack 1
+   +0 > P. 21723:23771(2048) ack 1
+   +0 > P. 23771:25819(2048) ack 1
+   +0 > P. 25819:27867(2048) ack 1
+   +0 > P. 27867:29915(2048) ack 1
+
+// Acknowledging the probe raises the MSS, and the next probe is larger.
+  +.1 < . 1:1(0) ack 29915 win 65535
+   +0 > . 29915:31266(1351) ack 1
+   +0 > P. 31266:33750(2484) ack 1

-- 
Git-157)