mptcp: sockopt: set timestamp flags on subflow socket, not msk

[PATCH mptcp-net] mptcp: sockopt: set timestamp flags on subflow socket, not msk

Posted by Gang Yan 2 weeks, 1 day ago

From: Gang Yan <yangang@kylinos.cn>

Both mptcp_setsockopt_sol_socket_tstamp() and
mptcp_setsockopt_sol_socket_timestamping() iterate over subflows,
acquire the subflow socket lock, but then erroneously pass the MPTCP
msk socket to sock_set_timestamp() / sock_set_timestamping() instead
of the subflow ssk. As a result, the timestamp flags are set on the
wrong socket and have no effect on the actual subflows.

Pass ssk instead of sk to both helpers.

Fixes: 3f4f958c3a3c ("mptcp: sockopt: propagate timestamp request to subflows")
Fixes: 6c9a0a0f2333 ("mptcp: setsockopt: convert to mptcp_setsockopt_sol_socket_timestamping()")
Assisted-by: GLM-5.1
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
 net/mptcp/sockopt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index de90a2897d2d..79db15903e7a 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -161,7 +161,7 @@ static int mptcp_setsockopt_sol_socket_tstamp(struct mptcp_sock *msk, int optnam
 		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
 		bool slow = lock_sock_fast(ssk);
 
-		sock_set_timestamp(sk, optname, !!val);
+		sock_set_timestamp(ssk, optname, !!val);
 		unlock_sock_fast(ssk, slow);
 	}
 
@@ -237,7 +237,7 @@ static int mptcp_setsockopt_sol_socket_timestamping(struct mptcp_sock *msk,
 		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
 		bool slow = lock_sock_fast(ssk);
 
-		sock_set_timestamping(sk, optname, timestamping);
+		sock_set_timestamping(ssk, optname, timestamping);
 		unlock_sock_fast(ssk, slow);
 	}
 
-- 
2.43.0

Re: [PATCH mptcp-net] mptcp: sockopt: set timestamp flags on subflow socket, not msk

Posted by Matthieu Baerts 1 week, 6 days ago

Hi Gang,

On 20/04/2026 11:33, Gang Yan wrote:
> From: Gang Yan <yangang@kylinos.cn>
> 
> Both mptcp_setsockopt_sol_socket_tstamp() and
> mptcp_setsockopt_sol_socket_timestamping() iterate over subflows,
> acquire the subflow socket lock, but then erroneously pass the MPTCP
> msk socket to sock_set_timestamp() / sock_set_timestamping() instead
> of the subflow ssk. As a result, the timestamp flags are set on the
> wrong socket and have no effect on the actual subflows.
> 
> Pass ssk instead of sk to both helpers.

Thank you for the patch, now in our tree (but don't forget the other
questions in my other emails in this email thread ;-) )

New patches for t/upstream-net and t/upstream:
- 7ed202c8815a: mptcp: sockopt: set timestamp flags on subflow socket,
not msk
- Results: 4ef37f659c52..4f7190e1b98d (export-net)
- Results: 3662f0d0a0b4..823280e7c25b (export)

Tests are now in progress:

- export-net:
https://github.com/multipath-tcp/mptcp_net-next/commit/7f79e7f6f65de8de0d206d00b4e689e52a94b0ce/checks
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/4464afe97dc56e817a23b730979cbc6fc48f1912/checks

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-net] mptcp: sockopt: set timestamp flags on subflow socket, not msk

Posted by MPTCP CI 2 weeks ago

Hi Gang,

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): Unstable: 1 failed test(s): packetdrill_sockopts ⚠️ 
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/24659822856

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


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)

Re: [PATCH mptcp-net] mptcp: sockopt: set timestamp flags on subflow socket, not msk

Posted by Matthieu Baerts 2 weeks, 1 day ago

Hi Gang,

On 20/04/2026 11:33, Gang Yan wrote:
> From: Gang Yan <yangang@kylinos.cn>
> 
> Both mptcp_setsockopt_sol_socket_tstamp() and
> mptcp_setsockopt_sol_socket_timestamping() iterate over subflows,
> acquire the subflow socket lock, but then erroneously pass the MPTCP
> msk socket to sock_set_timestamp() / sock_set_timestamping() instead
> of the subflow ssk. As a result, the timestamp flags are set on the
> wrong socket and have no effect on the actual subflows.
> 
> Pass ssk instead of sk to both helpers.

Good catch! How did you find the issue? It would be good to add a test
for that, but not sure how.

> Fixes: 3f4f958c3a3c ("mptcp: sockopt: propagate timestamp request to subflows")

It looks like this commit doesn't exist in the 'export' or 'export-net'
branches. I guess you meant to use 9061f24bf82e instead?

When you do a git blame, do not use branches from the TopGit tree. Use
'net', 'net-next', or the 'export' ones.

> Fixes: 6c9a0a0f2333 ("mptcp: setsockopt: convert to mptcp_setsockopt_sol_socket_timestamping()")
> Assisted-by: GLM-5.1

Thank you for using this tag!

Apart from the above, it looks good to me:

Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

I will wait for the CI, but I can also fix the SHA (if correct) when
applying the patch.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-net] mptcp: sockopt: set timestamp flags on subflow socket, not msk

Posted by gang.yan@linux.dev 1 week, 6 days ago

April 20, 2026 at 5:46 PM, "Matthieu Baerts" <matttbe@kernel.org mailto:matttbe@kernel.org?to=%22Matthieu%20Baerts%22%20%3Cmatttbe%40kernel.org%3E > wrote:


> 
> Hi Gang,
> 
> On 20/04/2026 11:33, Gang Yan wrote:
> 
> > 
> > From: Gang Yan <yangang@kylinos.cn>
> >  
> >  Both mptcp_setsockopt_sol_socket_tstamp() and
> >  mptcp_setsockopt_sol_socket_timestamping() iterate over subflows,
> >  acquire the subflow socket lock, but then erroneously pass the MPTCP
> >  msk socket to sock_set_timestamp() / sock_set_timestamping() instead
> >  of the subflow ssk. As a result, the timestamp flags are set on the
> >  wrong socket and have no effect on the actual subflows.
> >  
> >  Pass ssk instead of sk to both helpers.
> > 
> Good catch! How did you find the issue? It would be good to add a test
> for that, but not sure how.
> 

Hi Matt,

Sorry for the late feedback.

Thanks! I’ve been using AI assistance to study and understand this part of
the code recently, and that’s how I found the issue.

And I noticed the issue of sashiko mentioned. I’ll take time to look into it
recently, and I will post updates on this issue to the mailing list.

Thanks
Gang
> > 
> > Fixes: 3f4f958c3a3c ("mptcp: sockopt: propagate timestamp request to subflows")
> > 
> It looks like this commit doesn't exist in the 'export' or 'export-net'
> branches. I guess you meant to use 9061f24bf82e instead?
> 
> When you do a git blame, do not use branches from the TopGit tree. Use
> 'net', 'net-next', or the 'export' ones.
> 
> > 
> > Fixes: 6c9a0a0f2333 ("mptcp: setsockopt: convert to mptcp_setsockopt_sol_socket_timestamping()")
> >  Assisted-by: GLM-5.1
> > 
> Thank you for using this tag!
> 
> Apart from the above, it looks good to me:
> 
> Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> 
> I will wait for the CI, but I can also fix the SHA (if correct) when
> applying the patch.
> 
> Cheers,
> Matt
> -- 
> Sponsored by the NGI0 Core fund.
>

Re: [PATCH mptcp-net] mptcp: sockopt: set timestamp flags on subflow socket, not msk

Posted by Matthieu Baerts 1 week, 6 days ago

Hi Gang,

On 20/04/2026 11:46, Matthieu Baerts wrote:
> Hi Gang,
> 
> On 20/04/2026 11:33, Gang Yan wrote:
>> From: Gang Yan <yangang@kylinos.cn>
>>
>> Both mptcp_setsockopt_sol_socket_tstamp() and
>> mptcp_setsockopt_sol_socket_timestamping() iterate over subflows,
>> acquire the subflow socket lock, but then erroneously pass the MPTCP
>> msk socket to sock_set_timestamp() / sock_set_timestamping() instead
>> of the subflow ssk. As a result, the timestamp flags are set on the
>> wrong socket and have no effect on the actual subflows.
>>
>> Pass ssk instead of sk to both helpers.
> 
> Good catch! How did you find the issue? It would be good to add a test
> for that, but not sure how.
> 
>> Fixes: 3f4f958c3a3c ("mptcp: sockopt: propagate timestamp request to subflows")
> 
> It looks like this commit doesn't exist in the 'export' or 'export-net'
> branches. I guess you meant to use 9061f24bf82e instead?
> 
> When you do a git blame, do not use branches from the TopGit tree. Use
> 'net', 'net-next', or the 'export' ones.
> 
>> Fixes: 6c9a0a0f2333 ("mptcp: setsockopt: convert to mptcp_setsockopt_sol_socket_timestamping()")
>> Assisted-by: GLM-5.1
> 
> Thank you for using this tag!
> 
> Apart from the above, it looks good to me:
> 
> Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> 
> I will wait for the CI, but I can also fix the SHA (if correct) when
> applying the patch.
I'm going to apply the patch, but I noticed sashiko mentioned a possible
other existing issue:

  https://sashiko.dev/#/patchset/20260420093343.16443-1-gang.yan@linux.dev

Do you mind checking if what was reported is correct, please?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-net] mptcp: sockopt: set timestamp flags on subflow socket, not msk

Posted by Matthieu Baerts 2 weeks, 1 day ago

On 20/04/2026 11:46, Matthieu Baerts wrote:
> Hi Gang,
> 
> On 20/04/2026 11:33, Gang Yan wrote:
>> From: Gang Yan <yangang@kylinos.cn>
>>
>> Both mptcp_setsockopt_sol_socket_tstamp() and
>> mptcp_setsockopt_sol_socket_timestamping() iterate over subflows,
>> acquire the subflow socket lock, but then erroneously pass the MPTCP
>> msk socket to sock_set_timestamp() / sock_set_timestamping() instead
>> of the subflow ssk. As a result, the timestamp flags are set on the
>> wrong socket and have no effect on the actual subflows.
>>
>> Pass ssk instead of sk to both helpers.
> 
> Good catch! How did you find the issue? It would be good to add a test
> for that, but not sure how.
> 
>> Fixes: 3f4f958c3a3c ("mptcp: sockopt: propagate timestamp request to subflows")
> 
> It looks like this commit doesn't exist in the 'export' or 'export-net'
> branches. I guess you meant to use 9061f24bf82e instead?
> 
> When you do a git blame, do not use branches from the TopGit tree. Use
> 'net', 'net-next', or the 'export' ones.
> 
>> Fixes: 6c9a0a0f2333 ("mptcp: setsockopt: convert to mptcp_setsockopt_sol_socket_timestamping()")

It looks like only the previous one is enough. This second commit moves
code around, but it already contained the issue fixed here.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.