These patches from Geliang add support for the "last time" field in MPTCP Info, and verify that the counters look valid.
Patch 1 adds these counters: last_data_sent, last_data_recv and last_ack_recv. They are available in the MPTCP Info, so exposed via getsockopt(MPTCP_INFO) and the Netlink Diag interface.
Patch 2 adds a test in diag.sh MPTCP selftest, to check that the counters have moved by at least 250ms, after having waited twice that time.
Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- Geliang Tang (2): mptcp: add last time fields in mptcp_info selftests: mptcp: test last time mptcp_info
include/uapi/linux/mptcp.h | 4 +++ net/mptcp/options.c | 1 + net/mptcp/protocol.c | 7 ++++ net/mptcp/protocol.h | 3 ++ net/mptcp/sockopt.c | 4 +++ tools/testing/selftests/net/mptcp/diag.sh | 53 +++++++++++++++++++++++++++++++ 6 files changed, 72 insertions(+) --- base-commit: d76c740b2eaaddc5fc3a8b21eaec5b6b11e8c3f5 change-id: 20240405-upstream-net-next-20240405-mptcp-last-time-info-9b03618e08f1
Best regards,
From: Geliang Tang tanggeliang@kylinos.cn
This patch adds "last time" fields last_data_sent, last_data_recv and last_ack_recv in struct mptcp_sock to record the last time data_sent, data_recv and ack_recv happened. They all are initialized as tcp_jiffies32 in __mptcp_init_sock(), and updated as tcp_jiffies32 too when data is sent in __subflow_push_pending(), data is received in __mptcp_move_skbs_from_subflow(), and ack is received in ack_update_msk().
Similar to tcpi_last_data_sent, tcpi_last_data_recv and tcpi_last_ack_recv exposed with TCP, this patch exposes the last time "an action happened" for MPTCP in mptcp_info, named mptcpi_last_data_sent, mptcpi_last_data_recv and mptcpi_last_ack_recv, calculated in mptcp_diag_fill_info() as the time deltas between now and the newly added last time fields in mptcp_sock.
Also add three reserved bytes in struct mptcp_info not to have holes in this structure exposed to userspace.
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/446 Signed-off-by: Geliang Tang tanggeliang@kylinos.cn Reviewed-by: Mat Martineau martineau@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- include/uapi/linux/mptcp.h | 4 ++++ net/mptcp/options.c | 1 + net/mptcp/protocol.c | 7 +++++++ net/mptcp/protocol.h | 3 +++ net/mptcp/sockopt.c | 4 ++++ 5 files changed, 19 insertions(+)
diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h index 74cfe496891e..67d015df8893 100644 --- a/include/uapi/linux/mptcp.h +++ b/include/uapi/linux/mptcp.h @@ -58,6 +58,10 @@ struct mptcp_info { __u64 mptcpi_bytes_received; __u64 mptcpi_bytes_acked; __u8 mptcpi_subflows_total; + __u8 reserved[3]; + __u32 mptcpi_last_data_sent; + __u32 mptcpi_last_data_recv; + __u32 mptcpi_last_ack_recv; };
/* MPTCP Reset reason codes, rfc8684 */ diff --git a/net/mptcp/options.c b/net/mptcp/options.c index 27ca42c77b02..8e8dcfbc2993 100644 --- a/net/mptcp/options.c +++ b/net/mptcp/options.c @@ -1068,6 +1068,7 @@ static void ack_update_msk(struct mptcp_sock *msk, __mptcp_snd_una_update(msk, new_snd_una); __mptcp_data_acked(sk); } + msk->last_ack_recv = tcp_jiffies32; mptcp_data_unlock(sk);
trace_ack_update_msk(mp_opt->data_ack, diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 7e74b812e366..6c1af0155bb0 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -706,6 +706,8 @@ static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk, } } while (more_data_avail);
+ if (moved > 0) + msk->last_data_recv = tcp_jiffies32; *bytes += moved; return done; } @@ -1556,6 +1558,8 @@ static int __subflow_push_pending(struct sock *sk, struct sock *ssk, err = copied;
out: + if (err > 0) + msk->last_data_sent = tcp_jiffies32; return err; }
@@ -2793,6 +2797,9 @@ static void __mptcp_init_sock(struct sock *sk) WRITE_ONCE(msk->allow_infinite_fallback, true); msk->recovery = false; msk->subflow_id = 1; + msk->last_data_sent = tcp_jiffies32; + msk->last_data_recv = tcp_jiffies32; + msk->last_ack_recv = tcp_jiffies32;
mptcp_pm_data_init(msk);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 46f4655b7123..fdfa843e2d88 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -282,6 +282,9 @@ struct mptcp_sock { u64 bytes_acked; u64 snd_una; u64 wnd_end; + u32 last_data_sent; + u32 last_data_recv; + u32 last_ack_recv; unsigned long timer_ival; u32 token; int rmem_released; diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c index 73fdf423de44..2ec2fdf9f4af 100644 --- a/net/mptcp/sockopt.c +++ b/net/mptcp/sockopt.c @@ -896,6 +896,7 @@ 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 sock *sk = (struct sock *)msk; + u32 now = tcp_jiffies32; u32 flags = 0; bool slow;
@@ -930,6 +931,7 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info) info->mptcpi_snd_una = msk->snd_una; info->mptcpi_rcv_nxt = msk->ack_seq; info->mptcpi_bytes_acked = msk->bytes_acked; + info->mptcpi_last_ack_recv = jiffies_to_msecs(now - msk->last_ack_recv); mptcp_data_unlock(sk);
slow = lock_sock_fast(sk); @@ -942,6 +944,8 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info) info->mptcpi_bytes_retrans = msk->bytes_retrans; info->mptcpi_subflows_total = info->mptcpi_subflows + __mptcp_has_initial_subflow(msk); + info->mptcpi_last_data_sent = jiffies_to_msecs(now - msk->last_data_sent); + info->mptcpi_last_data_recv = jiffies_to_msecs(now - msk->last_data_recv); unlock_sock_fast(sk, slow); } EXPORT_SYMBOL_GPL(mptcp_diag_fill_info);
On Fri, Apr 5, 2024 at 3:06 PM Matthieu Baerts (NGI0) matttbe@kernel.org wrote:
From: Geliang Tang tanggeliang@kylinos.cn
This patch adds "last time" fields last_data_sent, last_data_recv and last_ack_recv in struct mptcp_sock to record the last time data_sent, data_recv and ack_recv happened. They all are initialized as tcp_jiffies32 in __mptcp_init_sock(), and updated as tcp_jiffies32 too when data is sent in __subflow_push_pending(), data is received in __mptcp_move_skbs_from_subflow(), and ack is received in ack_update_msk().
Similar to tcpi_last_data_sent, tcpi_last_data_recv and tcpi_last_ack_recv exposed with TCP, this patch exposes the last time "an action happened" for MPTCP in mptcp_info, named mptcpi_last_data_sent, mptcpi_last_data_recv and mptcpi_last_ack_recv, calculated in mptcp_diag_fill_info() as the time deltas between now and the newly added last time fields in mptcp_sock.
Also add three reserved bytes in struct mptcp_info not to have holes in this structure exposed to userspace.
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/446 Signed-off-by: Geliang Tang tanggeliang@kylinos.cn Reviewed-by: Mat Martineau martineau@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org
include/uapi/linux/mptcp.h | 4 ++++
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c index 73fdf423de44..2ec2fdf9f4af 100644 --- a/net/mptcp/sockopt.c +++ b/net/mptcp/sockopt.c @@ -896,6 +896,7 @@ 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 sock *sk = (struct sock *)msk;
u32 now = tcp_jiffies32; u32 flags = 0; bool slow;
@@ -930,6 +931,7 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info) info->mptcpi_snd_una = msk->snd_una; info->mptcpi_rcv_nxt = msk->ack_seq; info->mptcpi_bytes_acked = msk->bytes_acked;
info->mptcpi_last_ack_recv = jiffies_to_msecs(now - msk->last_ack_recv); mptcp_data_unlock(sk); slow = lock_sock_fast(sk);
lock_sock_fast(sk) can sleep and be quite slow...
I suggest you reload now = jiffies32;
@@ -942,6 +944,8 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info) info->mptcpi_bytes_retrans = msk->bytes_retrans; info->mptcpi_subflows_total = info->mptcpi_subflows + __mptcp_has_initial_subflow(msk);
info->mptcpi_last_data_sent = jiffies_to_msecs(now - msk->last_data_sent);
info->mptcpi_last_data_recv = jiffies_to_msecs(now - msk->last_data_recv); unlock_sock_fast(sk, slow);
} EXPORT_SYMBOL_GPL(mptcp_diag_fill_info);
-- 2.43.0
Hi Eric,
Thank you for the review!
On 05/04/2024 15:29, Eric Dumazet wrote:
On Fri, Apr 5, 2024 at 3:06 PM Matthieu Baerts (NGI0) matttbe@kernel.org wrote:
From: Geliang Tang tanggeliang@kylinos.cn
This patch adds "last time" fields last_data_sent, last_data_recv and last_ack_recv in struct mptcp_sock to record the last time data_sent, data_recv and ack_recv happened. They all are initialized as tcp_jiffies32 in __mptcp_init_sock(), and updated as tcp_jiffies32 too when data is sent in __subflow_push_pending(), data is received in __mptcp_move_skbs_from_subflow(), and ack is received in ack_update_msk().
Similar to tcpi_last_data_sent, tcpi_last_data_recv and tcpi_last_ack_recv exposed with TCP, this patch exposes the last time "an action happened" for MPTCP in mptcp_info, named mptcpi_last_data_sent, mptcpi_last_data_recv and mptcpi_last_ack_recv, calculated in mptcp_diag_fill_info() as the time deltas between now and the newly added last time fields in mptcp_sock.
Also add three reserved bytes in struct mptcp_info not to have holes in this structure exposed to userspace.
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/446 Signed-off-by: Geliang Tang tanggeliang@kylinos.cn Reviewed-by: Mat Martineau martineau@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org
include/uapi/linux/mptcp.h | 4 ++++
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c index 73fdf423de44..2ec2fdf9f4af 100644 --- a/net/mptcp/sockopt.c +++ b/net/mptcp/sockopt.c @@ -896,6 +896,7 @@ 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 sock *sk = (struct sock *)msk;
u32 now = tcp_jiffies32; u32 flags = 0; bool slow;
@@ -930,6 +931,7 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info) info->mptcpi_snd_una = msk->snd_una; info->mptcpi_rcv_nxt = msk->ack_seq; info->mptcpi_bytes_acked = msk->bytes_acked;
info->mptcpi_last_ack_recv = jiffies_to_msecs(now - msk->last_ack_recv); mptcp_data_unlock(sk); slow = lock_sock_fast(sk);
lock_sock_fast(sk) can sleep and be quite slow...
I suggest you reload now = jiffies32;
Good point, it would make more sense to reload it after this lock!
(or defining "now" only here, under this lock_sock_fast(), and move the block here above that is under the data spin lock, after, so all the "time" counter are "in sync"?)
pw-bot: changes-requested
@@ -942,6 +944,8 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info) info->mptcpi_bytes_retrans = msk->bytes_retrans; info->mptcpi_subflows_total = info->mptcpi_subflows + __mptcp_has_initial_subflow(msk);
info->mptcpi_last_data_sent = jiffies_to_msecs(now - msk->last_data_sent);
info->mptcpi_last_data_recv = jiffies_to_msecs(now - msk->last_data_recv); unlock_sock_fast(sk, slow);
} EXPORT_SYMBOL_GPL(mptcp_diag_fill_info);
-- 2.43.0
Cheers, Matt
From: Geliang Tang tanggeliang@kylinos.cn
This patch adds a new helper chk_msk_info() to show the counters in mptcp_info of the given info, and check that the timestamps move forward. Use it to show newly added last_data_sent, last_data_recv and last_ack_recv in mptcp_info in chk_last_time_info().
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn Reviewed-by: Matthieu Baerts (NGI0) matttbe@kernel.org Reviewed-by: Mat Martineau martineau@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- tools/testing/selftests/net/mptcp/diag.sh | 53 +++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)
diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh index bc97ab33a00e..776d43a6922d 100755 --- a/tools/testing/selftests/net/mptcp/diag.sh +++ b/tools/testing/selftests/net/mptcp/diag.sh @@ -200,6 +200,58 @@ chk_msk_cestab() "${expected}" "${msg}" "" }
+msk_info_get_value() +{ + local port="${1}" + local info="${2}" + + ss -N "${ns}" -inHM dport "${port}" | \ + mptcp_lib_get_info_value "${info}" "${info}" +} + +chk_msk_info() +{ + local port="${1}" + local info="${2}" + local cnt="${3}" + local msg="....chk ${info}" + local delta_ms=250 # half what we waited before, just to be sure + local now + + now=$(msk_info_get_value "${port}" "${info}") + + mptcp_lib_print_title "${msg}" + if { [ -z "${cnt}" ] || [ -z "${now}" ]; } && + ! mptcp_lib_expect_all_features; then + mptcp_lib_pr_skip "Feature probably not supported" + mptcp_lib_result_skip "${msg}" + elif [ "$((cnt + delta_ms))" -lt "${now}" ]; then + mptcp_lib_pr_ok + mptcp_lib_result_pass "${msg}" + else + mptcp_lib_pr_fail "value of ${info} changed by $((now - cnt))ms," \ + "expected at least ${delta_ms}ms" + mptcp_lib_result_fail "${msg}" + ret=${KSFT_FAIL} + fi +} + +chk_last_time_info() +{ + local port="${1}" + local data_sent data_recv ack_recv + + data_sent=$(msk_info_get_value "${port}" "last_data_sent") + data_recv=$(msk_info_get_value "${port}" "last_data_recv") + ack_recv=$(msk_info_get_value "${port}" "last_ack_recv") + + sleep 0.5 # wait to check after if the timestamps difference + + chk_msk_info "${port}" "last_data_sent" "${data_sent}" + chk_msk_info "${port}" "last_data_recv" "${data_recv}" + chk_msk_info "${port}" "last_ack_recv" "${ack_recv}" +} + wait_connected() { local listener_ns="${1}" @@ -233,6 +285,7 @@ echo "b" | \ 127.0.0.1 >/dev/null & wait_connected $ns 10000 chk_msk_nr 2 "after MPC handshake " +chk_last_time_info 10000 chk_msk_remote_key_nr 2 "....chk remote_key" chk_msk_fallback_nr 0 "....chk no fallback" chk_msk_inuse 2
linux-kselftest-mirror@lists.linaro.org