All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v8] mptcp: normalize seq numbers reported in mptcp_info
@ 2026-10-14 13:08 Kalpan Jani
  2026-10-14 13:28 ` sashiko-bot
  2026-10-14 14:27 ` MPTCP CI
  0 siblings, 2 replies; 3+ messages in thread
From: Kalpan Jani @ 2026-10-14 13:08 UTC (permalink / raw)
  To: mptcp; +Cc: shardul.b, janak, kalpanjani009, Kalpan Jani

The sequence numbers exposed through MPTCP_INFO include random initial
data sequence numbers, making their absolute values difficult for
userspace to interpret.

Report write_seq and snd_una relative to the local IDSN, and rcv_nxt
relative to the peer's initial data sequence number.

Read the sequence baselines from the subflow context under RCU, handle
a missing context, and skip disconnected sockets. Publish
remote_key_valid only after initializing iasn.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/445
Signed-off-by: Kalpan Jani <kalpan.jani@mpiricsoftware.com>

---
Changes since v7:
- Return early for MPTCP sockets in TCP_CLOSE state.
- Access icsk_ulp_data under RCU and handle a NULL context
  when obtaining the local and remote IDSN values.
- Zero-initialize the MPTCP_INFO output structure.

v7: https://lore.kernel.org/all/20260930102804.2796208-1-kalpan.jani@mpiricsoftware.com/
---
 net/mptcp/sockopt.c | 37 ++++++++++++++++++++++++++++++++-----
 net/mptcp/subflow.c |  4 +++-
 2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 922f6ae5c80cb..c441c49e7f5f7 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -1046,7 +1046,10 @@ static int mptcp_getsockopt_first_sf_only(struct mptcp_sock *msk, int level, int
 
 void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 {
+	struct mptcp_subflow_context *subflow;
 	struct sock *sk = (struct sock *)msk;
+	u64 local_idsn = 0, remote_idsn = 0;
+	struct sock *first;
 	u32 flags = 0;
 	bool slow;
 	u32 now;
@@ -1058,7 +1061,8 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 	info->mptcpi_add_addr_accepted = READ_ONCE(msk->pm.add_addr_accepted);
 	info->mptcpi_local_addr_used = READ_ONCE(msk->pm.local_addr_used);
 
-	if (inet_sk_state_load(sk) == TCP_LISTEN)
+	if (inet_sk_state_load(sk) == TCP_LISTEN ||
+	    inet_sk_state_load(sk) == TCP_CLOSE)
 		return;
 
 	/* The following limits only make sense for the in-kernel PM */
@@ -1084,9 +1088,23 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 	info->mptcpi_flags = flags;
 
 	slow = lock_sock_fast(sk);
+	/* Hold a reference to the first subflow while accessing its context. */
+	rcu_read_lock();
+	first = READ_ONCE(msk->first);
+	if (first && !refcount_inc_not_zero(&first->sk_refcnt))
+		first = NULL;
+	rcu_read_unlock();
+
+	rcu_read_lock();
+	subflow = first ?
+		rcu_dereference(inet_csk(first)->icsk_ulp_data) : NULL;
+	if (subflow)
+		local_idsn = subflow->idsn;
+	rcu_read_unlock();
+
 	info->mptcpi_csum_enabled = READ_ONCE(msk->csum_enabled);
 	info->mptcpi_token = msk->token;
-	info->mptcpi_write_seq = msk->write_seq;
+	info->mptcpi_write_seq = msk->write_seq - local_idsn;
 	info->mptcpi_retransmits = inet_csk(sk)->icsk_retransmits;
 	info->mptcpi_bytes_sent = msk->bytes_sent;
 	info->mptcpi_bytes_received = msk->bytes_received;
@@ -1099,17 +1117,26 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 	unlock_sock_fast(sk, slow);
 
 	mptcp_data_lock(sk);
+	rcu_read_lock();
+	subflow = first ?
+		rcu_dereference(inet_csk(first)->icsk_ulp_data) : NULL;
+	if (subflow && subflow->remote_key_valid)
+		remote_idsn = subflow->iasn - 1;
+	rcu_read_unlock();
 	info->mptcpi_last_ack_recv = jiffies_to_msecs(now - msk->last_ack_recv);
-	info->mptcpi_snd_una = msk->snd_una;
-	info->mptcpi_rcv_nxt = msk->ack_seq;
+	info->mptcpi_snd_una = msk->snd_una - local_idsn;
+	info->mptcpi_rcv_nxt = msk->ack_seq - remote_idsn;
 	info->mptcpi_bytes_acked = msk->bytes_acked;
 	mptcp_data_unlock(sk);
+
+	if (first)
+		sock_put(first);
 }
 EXPORT_SYMBOL_GPL(mptcp_diag_fill_info);
 
 static int mptcp_getsockopt_info(struct mptcp_sock *msk, char __user *optval, int __user *optlen)
 {
-	struct mptcp_info m_info;
+	struct mptcp_info m_info = {};
 	int len;
 
 	if (get_user(len, optlen))
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 2d7ccb01d2342..a3313a3db5a76 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -482,7 +482,6 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
 	if (subflow->remote_key_valid)
 		return;
 
-	subflow->remote_key_valid = 1;
 	subflow->remote_key = mp_opt->sndr_key;
 	mptcp_crypto_key_sha(subflow->remote_key, NULL, &subflow->iasn);
 	subflow->iasn++;
@@ -494,6 +493,9 @@ static void subflow_set_remote_key(struct mptcp_sock *msk,
 	WRITE_ONCE(msk->ack_seq, subflow->iasn);
 	WRITE_ONCE(msk->can_ack, true);
 	atomic64_set(&msk->rcv_wnd_sent, subflow->iasn);
+
+	/* publish last, once iasn and the fields above are fully populated */
+	subflow->remote_key_valid = 1;
 }
 
 static void mptcp_propagate_state(struct sock *sk, struct sock *ssk,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next v8] mptcp: normalize seq numbers reported in mptcp_info
  2026-10-14 13:08 [PATCH net-next v8] mptcp: normalize seq numbers reported in mptcp_info Kalpan Jani
@ 2026-10-14 13:28 ` sashiko-bot
  2026-10-14 14:27 ` MPTCP CI
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-10-14 13:28 UTC (permalink / raw)
  To: Kalpan Jani, mptcp

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] mptcp: MPTCP_INFO seq numbers revert to absolute on disconnect
- [Medium] mptcp: MPTCP_INFO drops statistics for TCP_CLOSE sockets
--

commit 226e0982ada8f162a7c854a2bd67d3b843a4af48
Author: Kalpan Jani <kalpan.jani@mpiricsoftware.com>

    mptcp: normalize seq numbers reported in mptcp_info

    The commit changes MPTCP_INFO to report write_seq and snd_una relative to
    the local IDSN, and rcv_nxt relative to the peer's initial data sequence
    number. It reads these baselines from the subflow context under RCU,
    handles missing contexts, and skips disconnected sockets.

    Link: https://github.com/multipath-tcp/mptcp_net-next/issues/445

> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index fcf6feb2a9eb5..76bab204fc33b 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -982,7 +985,8 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
>  	info->mptcpi_add_addr_accepted = READ_ONCE(msk->pm.add_addr_accepted);
>  	info->mptcpi_local_addr_used = READ_ONCE(msk->pm.local_addr_used);
>  
> -	if (inet_sk_state_load(sk) == TCP_LISTEN)
> +	if (inet_sk_state_load(sk) == TCP_LISTEN ||
> +	    inet_sk_state_load(sk) == TCP_CLOSE)
>  		return;

[Severity: Medium]
Will this early return cause MPTCP_INFO to drop final statistics for sockets
in the TCP_CLOSE state?

Since the info structure is zero-initialized at the start of the function,
returning early means userspace tools reading MPTCP_INFO on a fully closed
socket will see zeroes for all historical connection stats like
mptcpi_bytes_sent and mptcpi_retransmits, unlike standard TCP.

[ ... ]

> @@ -1008,9 +1012,23 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
>  	info->mptcpi_flags = flags;
>  
>  	slow = lock_sock_fast(sk);
> +	/* Hold a reference to the first subflow while accessing its context. */
> +	rcu_read_lock();
> +	first = READ_ONCE(msk->first);
> +	if (first && !refcount_inc_not_zero(&first->sk_refcnt))
> +		first = NULL;
> +	rcu_read_unlock();
> +
> +	rcu_read_lock();
> +	subflow = first ?
> +		rcu_dereference(inet_csk(first)->icsk_ulp_data) : NULL;
> +	if (subflow)
> +		local_idsn = subflow->idsn;

[Severity: Medium]
Can local_idsn be read as zero if the first subflow was reset?

If the MPTCP connection survives but the first subflow is closed,
__mptcp_subflow_disconnect() invokes mptcp_subflow_ctx_reset(). This zeroes
the reset struct group, clearing subflow->idsn to 0.

If this occurs, subtracting 0 will suddenly expose raw absolute sequence
numbers to userspace instead of the intended relative values.

> +	rcu_read_unlock();
> +
>  	info->mptcpi_csum_enabled = READ_ONCE(msk->csum_enabled);
>  	info->mptcpi_token = msk->token;

[ ... ]

> @@ -1023,17 +1041,26 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
>  	unlock_sock_fast(sk, slow);
>  
>  	mptcp_data_lock(sk);
> +	rcu_read_lock();
> +	subflow = first ?
> +		rcu_dereference(inet_csk(first)->icsk_ulp_data) : NULL;
> +	if (subflow && subflow->remote_key_valid)
> +		remote_idsn = subflow->iasn - 1;

[Severity: Medium]
Similarly, can subflow->iasn be read as zero or bypassed here?

If mptcp_subflow_ctx_reset() has been called, subflow->remote_key_valid and
subflow->iasn are also zeroed out. This would cause remote_idsn to remain 0,
again exposing absolute sequence numbers for mptcpi_rcv_nxt.

> +	rcu_read_unlock();
>  	info->mptcpi_last_ack_recv = jiffies_to_msecs(now - msk->last_ack_recv);
> -	info->mptcpi_snd_una = msk->snd_una;
> -	info->mptcpi_rcv_nxt = msk->ack_seq;
> +	info->mptcpi_snd_una = msk->snd_una - local_idsn;
> +	info->mptcpi_rcv_nxt = msk->ack_seq - remote_idsn;
>  	info->mptcpi_bytes_acked = msk->bytes_acked;
>  	mptcp_data_unlock(sk);
> +
> +	if (first)
> +		sock_put(first);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014130809.1874184-1-kalpan.jani@mpiricsoftware.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next v8] mptcp: normalize seq numbers reported in mptcp_info
  2026-10-14 13:08 [PATCH net-next v8] mptcp: normalize seq numbers reported in mptcp_info Kalpan Jani
  2026-10-14 13:28 ` sashiko-bot
@ 2026-10-14 14:27 ` MPTCP CI
  1 sibling, 0 replies; 3+ messages in thread
From: MPTCP CI @ 2026-10-14 14:27 UTC (permalink / raw)
  To: Kalpan Jani; +Cc: mptcp

Hi Kalpan,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Perf: Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/38574170831

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/8ad9371d2e02
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1185594


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-14 14:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-14 13:08 [PATCH net-next v8] mptcp: normalize seq numbers reported in mptcp_info Kalpan Jani
2026-10-14 13:28 ` sashiko-bot
2026-10-14 14:27 ` MPTCP CI

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.