[PATCH net-next v2 06/15] mptcp: pm: drop TCP TS with ADD_ADDRv6 + port

Matthieu Baerts (NGI0) posted 15 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH net-next v2 06/15] mptcp: pm: drop TCP TS with ADD_ADDRv6 + port
Posted by Matthieu Baerts (NGI0) 2 months, 1 week ago
With TCP-timestamps (padded) taking 12 bytes and ADD_ADDR IPv6 + port
taking 30 bytes, the 40-byte limit for the TCP options is reached. In
this case, it is then not possible to send the signal.

To be able to send this ADD_ADDR, the TCP timestamps option can now be
dropped. This is done, when needed by setting the *drop_ts parameter
from mptcp_established_options. This feature is controlled by a new
net.mptcp.add_addr_v6_port_drop_ts sysctl knob, enabled by default.

It is important to keep in mind that dropping the TCP timestamps option
for one packet of the connection could eventually disrupt some
middleboxes: even if it should be unlikely, they could drop the packet
or even block the connection. That's why this new feature can be
controlled by a sysctl knob.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/448
Reviewed-by: Mat Martineau <martineau@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/options.c  |  9 +++++++--
 net/mptcp/pm.c       | 13 ++++++++++++-
 net/mptcp/protocol.h |  3 ++-
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 95f16f9f0ce2..8d0680a588dd 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -659,11 +659,13 @@ static u64 add_addr_generate_hmac(u64 key1, u64 key2,
 static bool mptcp_established_options_add_addr(struct sock *sk,
 					       struct sk_buff *skb, int *size,
 					       unsigned int remaining,
+					       bool has_ts,
 					       struct mptcp_out_options *opts)
 {
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
 	struct mptcp_sock *msk = mptcp_sk(subflow->conn);
 	struct mptcp_addr_info addr;
+	bool drop_ts = has_ts;
 	bool echo;
 
 	/* add addr will strip the existing options, be sure to avoid breaking
@@ -672,11 +674,13 @@ static bool mptcp_established_options_add_addr(struct sock *sk,
 	if (!mptcp_pm_should_add_signal(msk) ||
 	    (opts->suboptions & (OPTION_MPTCP_MPJ_ACK | OPTION_MPTCP_MPC_ACK)) ||
 	    !skb || !skb_is_tcp_pure_ack(skb) ||
-	    !mptcp_pm_add_addr_signal(msk, size, remaining, &addr, &echo))
+	    !mptcp_pm_add_addr_signal(msk, size, remaining, &addr, &echo,
+				      &drop_ts))
 		return false;
 
 	pr_debug("drop other suboptions\n");
 	opts->suboptions = OPTION_MPTCP_ADD_ADDR;
+	opts->drop_ts = drop_ts;
 	opts->addr = addr;
 	if (!echo) {
 		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ADDADDRTX);
@@ -859,7 +863,8 @@ int mptcp_established_options(struct sock *sk, struct sk_buff *skb,
 
 	total_size += opt_size;
 	remaining -= opt_size;
-	if (mptcp_established_options_add_addr(sk, skb, &opt_size, remaining, opts)) {
+	if (mptcp_established_options_add_addr(sk, skb, &opt_size, remaining,
+					       has_ts, opts)) {
 		total_size += opt_size;
 		remaining -= opt_size;
 		ret = true;
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 59dc598d343d..ac7de4141738 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -903,7 +903,8 @@ static int mptcp_add_addr_len(int family, bool echo, bool port)
 }
 
 bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, int *size, int remaining,
-			      struct mptcp_addr_info *addr, bool *echo)
+			      struct mptcp_addr_info *addr, bool *echo,
+			      bool *drop_ts)
 {
 	bool skip_add_addr = false;
 	bool ret = false;
@@ -941,6 +942,13 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, int *size, int remaining,
 	if (len > remaining) {
 		struct net *net = sock_net((struct sock *)msk);
 
+		if (*drop_ts && mptcp_add_addr_v6_port_drop_ts(net)) {
+			/* OK without TCP Timestamps? */
+			len -= TCPOLEN_TSTAMP_ALIGNED;
+			if (len <= remaining)
+				goto enough_space;
+		}
+
 		if (*echo) {
 			MPTCP_INC_STATS(net, MPTCP_MIB_ECHOADDTXDROP);
 		} else {
@@ -950,6 +958,9 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, int *size, int remaining,
 		goto drop_signal_mark;
 	}
 
+	*drop_ts = false;
+
+enough_space:
 	ret = true;
 	*size = len;
 
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index b43dae72e7de..e69fcb4d48af 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1208,7 +1208,8 @@ static inline bool mptcp_pm_is_kernel(const struct mptcp_sock *msk)
 }
 
 bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, int *size, int remaining,
-			      struct mptcp_addr_info *addr, bool *echo);
+			      struct mptcp_addr_info *addr, bool *echo,
+			      bool *drop_ts);
 bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
 			     struct mptcp_rm_list *rm_list, int *len);
 int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);

-- 
2.53.0
Re: [PATCH net-next v2 06/15] mptcp: pm: drop TCP TS with ADD_ADDRv6 + port
Posted by Matthieu Baerts 2 months ago
Hello,

On 05/06/2026 19:21, Matthieu Baerts (NGI0) wrote:
> With TCP-timestamps (padded) taking 12 bytes and ADD_ADDR IPv6 + port
> taking 30 bytes, the 40-byte limit for the TCP options is reached. In
> this case, it is then not possible to send the signal.
> 
> To be able to send this ADD_ADDR, the TCP timestamps option can now be
> dropped. This is done, when needed by setting the *drop_ts parameter
> from mptcp_established_options. This feature is controlled by a new
> net.mptcp.add_addr_v6_port_drop_ts sysctl knob, enabled by default.
> 
> It is important to keep in mind that dropping the TCP timestamps option
> for one packet of the connection could eventually disrupt some
> middleboxes: even if it should be unlikely, they could drop the packet
> or even block the connection. That's why this new feature can be
> controlled by a sysctl knob.
FYI, Sashiko's new comments about this patch [1] are suggesting issues
when MPTCP is used in parallel with TCP MD5. But MPTCP cannot be used
with MD5 because there is not enough space in the TCP options. That's
why there is an explicit rejection when initiating the connection.

So I think we can ignore these comments saying that more options could
be dropped, and the size can be negative.

https://sashiko.dev/#/patchset/20260605-net-next-mptcp-add-addr6-port-ts-v2-0-758e7ca73f4d%40kernel.org?part=6

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH net-next v2 06/15] mptcp: pm: drop TCP TS with ADD_ADDRv6 + port
Posted by Jakub Kicinski 2 months ago
On Wed, 10 Jun 2026 17:13:54 +0200 Matthieu Baerts wrote:
> Hello,
> 
> On 05/06/2026 19:21, Matthieu Baerts (NGI0) wrote:
> > With TCP-timestamps (padded) taking 12 bytes and ADD_ADDR IPv6 + port
> > taking 30 bytes, the 40-byte limit for the TCP options is reached. In
> > this case, it is then not possible to send the signal.
> > 
> > To be able to send this ADD_ADDR, the TCP timestamps option can now be
> > dropped. This is done, when needed by setting the *drop_ts parameter
> > from mptcp_established_options. This feature is controlled by a new
> > net.mptcp.add_addr_v6_port_drop_ts sysctl knob, enabled by default.
> > 
> > It is important to keep in mind that dropping the TCP timestamps option
> > for one packet of the connection could eventually disrupt some
> > middleboxes: even if it should be unlikely, they could drop the packet
> > or even block the connection. That's why this new feature can be
> > controlled by a sysctl knob.  
> FYI, Sashiko's new comments about this patch [1] are suggesting issues
> when MPTCP is used in parallel with TCP MD5. But MPTCP cannot be used
> with MD5 because there is not enough space in the TCP options. That's
> why there is an explicit rejection when initiating the connection.
> 
> So I think we can ignore these comments saying that more options could
> be dropped, and the size can be negative.
> 
> https://sashiko.dev/#/patchset/20260605-net-next-mptcp-add-addr6-port-ts-v2-0-758e7ca73f4d%40kernel.org?part=6

I'll probably pull today, but to explain the delay - Sashiko made me
read the code and while I think the reports are false positives - 
the option handling does not read like awesome code. So the series fell
down the priority list.