Re: [PATCH mptcp-next v3 2/3] mptcp: add last time fields in mptcp_info

[Mat Martineau <martineau@kernel.org>]

On Sat, 30 Mar 2024, Geliang Tang wrote:

> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> 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.
>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/446
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> include/uapi/linux/mptcp.h | 4 ++++
> net/mptcp/sockopt.c        | 5 +++++
> 2 files changed, 9 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/sockopt.c b/net/mptcp/sockopt.c
> index dcd1c76d2a3b..1e74851614e8 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -898,6 +898,7 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
> 	struct sock *sk = (struct sock *)msk;
> 	u32 flags = 0;
> 	bool slow;
> +	u32 now;
>
> 	memset(info, 0, sizeof(*info));
>
> @@ -942,6 +943,10 @@ 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);
> +	now = tcp_jiffies32;
> +	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);
> +	info->mptcpi_last_ack_recv = jiffies_to_msecs(now - msk->last_ack_recv);

Hi Geliang -

msk->last_ack_recv is protected by mptcp_data_lock, not the msk socket 
lock, so this line should be moved to the data_lock protected section 
earlier in mptcp_diag_fill_info().

Thanks,

Mat

> 	unlock_sock_fast(sk, slow);
> }
> EXPORT_SYMBOL_GPL(mptcp_diag_fill_info);
> -- 
> 2.40.1
>
>
>