From 00df24f19179c8e8c81fffa95c91d5c2eab8964e Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Thu, 10 Nov 2022 15:23:18 -0800 Subject: [PATCH 1/5] mptcp: use msk instead of mptcp_sk Use msk instead of mptcp_sk(sk) in the functions where the variable "msk = mptcp_sk(sk)" has been defined. Reviewed-by: Mat Martineau Signed-off-by: Geliang Tang Signed-off-by: Mat Martineau Signed-off-by: Jakub Kicinski --- net/mptcp/protocol.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 109eea2c65ff7..64d7070de901b 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -1625,7 +1625,7 @@ static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk) * check for a different subflow usage only after * spooling the first chunk of data */ - xmit_ssk = first ? ssk : mptcp_subflow_get_send(mptcp_sk(sk)); + xmit_ssk = first ? ssk : mptcp_subflow_get_send(msk); if (!xmit_ssk) goto out; if (xmit_ssk != ssk) { @@ -2275,7 +2275,7 @@ bool __mptcp_retransmit_pending_data(struct sock *sk) struct mptcp_data_frag *cur, *rtx_head; struct mptcp_sock *msk = mptcp_sk(sk); - if (__mptcp_check_fallback(mptcp_sk(sk))) + if (__mptcp_check_fallback(msk)) return false; if (tcp_rtx_and_write_queues_empty(sk)) @@ -2949,7 +2949,7 @@ bool __mptcp_close(struct sock *sk, long timeout) sock_hold(sk); pr_debug("msk=%p state=%d", sk, sk->sk_state); - if (mptcp_sk(sk)->token) + if (msk->token) mptcp_event(MPTCP_EVENT_CLOSED, msk, NULL, GFP_KERNEL); if (sk->sk_state == TCP_CLOSE) { @@ -3008,8 +3008,8 @@ static int mptcp_disconnect(struct sock *sk, int flags) mptcp_stop_timer(sk); sk_stop_timer(sk, &sk->sk_timer); - if (mptcp_sk(sk)->token) - mptcp_event(MPTCP_EVENT_CLOSED, mptcp_sk(sk), NULL, GFP_KERNEL); + if (msk->token) + mptcp_event(MPTCP_EVENT_CLOSED, msk, NULL, GFP_KERNEL); /* msk->subflow is still intact, the following will not free the first * subflow From 73a0052a61f98354f39f461e03f1a7e513b84578 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Thu, 10 Nov 2022 15:23:19 -0800 Subject: [PATCH 2/5] mptcp: change 'first' as a parameter The function mptcp_subflow_process_delegated() uses the input ssk first, while __mptcp_check_push() invokes the packet scheduler first. So this patch adds a new parameter named 'first' for the function __mptcp_subflow_push_pending() to deal with these two cases separately. With this change, the code that invokes the packet scheduler in the function __mptcp_check_push() can be removed, and replaced by invoking __mptcp_subflow_push_pending() directly. Reviewed-by: Mat Martineau Signed-off-by: Geliang Tang Signed-off-by: Mat Martineau Signed-off-by: Jakub Kicinski --- net/mptcp/protocol.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 64d7070de901b..5a344788f8439 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -1602,7 +1602,7 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags) __mptcp_check_send_data_fin(sk); } -static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk) +static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk, bool first) { struct mptcp_sock *msk = mptcp_sk(sk); struct mptcp_sendmsg_info info = { @@ -1611,7 +1611,6 @@ static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk) struct mptcp_data_frag *dfrag; struct sock *xmit_ssk; int len, copied = 0; - bool first = true; info.flags = 0; while ((dfrag = mptcp_send_head(sk))) { @@ -1621,8 +1620,7 @@ static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk) while (len > 0) { int ret = 0; - /* the caller already invoked the packet scheduler, - * check for a different subflow usage only after + /* check for a different subflow usage only after * spooling the first chunk of data */ xmit_ssk = first ? ssk : mptcp_subflow_get_send(msk); @@ -3220,16 +3218,10 @@ void __mptcp_check_push(struct sock *sk, struct sock *ssk) if (!mptcp_send_head(sk)) return; - if (!sock_owned_by_user(sk)) { - struct sock *xmit_ssk = mptcp_subflow_get_send(mptcp_sk(sk)); - - if (xmit_ssk == ssk) - __mptcp_subflow_push_pending(sk, ssk); - else if (xmit_ssk) - mptcp_subflow_delegate(mptcp_subflow_ctx(xmit_ssk), MPTCP_DELEGATE_SEND); - } else { + if (!sock_owned_by_user(sk)) + __mptcp_subflow_push_pending(sk, ssk, false); + else __set_bit(MPTCP_PUSH_PENDING, &mptcp_sk(sk)->cb_flags); - } } #define MPTCP_FLAGS_PROCESS_CTX_NEED (BIT(MPTCP_PUSH_PENDING) | \ @@ -3320,7 +3312,7 @@ void mptcp_subflow_process_delegated(struct sock *ssk) if (test_bit(MPTCP_DELEGATE_SEND, &subflow->delegated_status)) { mptcp_data_lock(sk); if (!sock_owned_by_user(sk)) - __mptcp_subflow_push_pending(sk, ssk); + __mptcp_subflow_push_pending(sk, ssk, true); else __set_bit(MPTCP_PUSH_PENDING, &mptcp_sk(sk)->cb_flags); mptcp_data_unlock(sk); From 80638684e840684042325fa5a3fa1eb59fd123cc Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Thu, 10 Nov 2022 15:23:20 -0800 Subject: [PATCH 3/5] mptcp: get sk from msk directly Use '(struct sock *)msk' to get 'sk' from 'msk' in a more direct way instead of using '&msk->sk.icsk_inet.sk'. Reviewed-by: Mat Martineau Signed-off-by: Geliang Tang Signed-off-by: Mat Martineau Signed-off-by: Jakub Kicinski --- net/mptcp/pm_userspace.c | 4 ++-- net/mptcp/protocol.c | 4 ++-- net/mptcp/sockopt.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index 9e82250cbb703..5cb65f0928f4e 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -291,7 +291,7 @@ int mptcp_nl_cmd_sf_create(struct sk_buff *skb, struct genl_info *info) goto create_err; } - sk = &msk->sk.icsk_inet.sk; + sk = (struct sock *)msk; lock_sock(sk); err = __mptcp_subflow_connect(sk, &addr_l, &addr_r); @@ -403,7 +403,7 @@ int mptcp_nl_cmd_sf_destroy(struct sk_buff *skb, struct genl_info *info) goto destroy_err; } - sk = &msk->sk.icsk_inet.sk; + sk = (struct sock *)msk; lock_sock(sk); ssk = mptcp_nl_find_ssk(msk, &addr_l, &addr_r); if (ssk) { diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 5a344788f8439..3796d1bfef6b3 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -2454,7 +2454,7 @@ static bool mptcp_check_close_timeout(const struct sock *sk) static void mptcp_check_fastclose(struct mptcp_sock *msk) { struct mptcp_subflow_context *subflow, *tmp; - struct sock *sk = &msk->sk.icsk_inet.sk; + struct sock *sk = (struct sock *)msk; if (likely(!READ_ONCE(msk->rcv_fastclose))) return; @@ -2616,7 +2616,7 @@ static void mptcp_do_fastclose(struct sock *sk) static void mptcp_worker(struct work_struct *work) { struct mptcp_sock *msk = container_of(work, struct mptcp_sock, work); - struct sock *sk = &msk->sk.icsk_inet.sk; + struct sock *sk = (struct sock *)msk; unsigned long fail_tout; int state; diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c index f85e9bbfe86f2..f62f6483ef775 100644 --- a/net/mptcp/sockopt.c +++ b/net/mptcp/sockopt.c @@ -987,7 +987,7 @@ static int mptcp_getsockopt_tcpinfo(struct mptcp_sock *msk, char __user *optval, int __user *optlen) { struct mptcp_subflow_context *subflow; - struct sock *sk = &msk->sk.icsk_inet.sk; + struct sock *sk = (struct sock *)msk; unsigned int sfcount = 0, copied = 0; struct mptcp_subflow_data sfd; char __user *infoptr; @@ -1078,8 +1078,8 @@ static void mptcp_get_sub_addrs(const struct sock *sk, struct mptcp_subflow_addr static int mptcp_getsockopt_subflow_addrs(struct mptcp_sock *msk, char __user *optval, int __user *optlen) { - struct sock *sk = &msk->sk.icsk_inet.sk; struct mptcp_subflow_context *subflow; + struct sock *sk = (struct sock *)msk; unsigned int sfcount = 0, copied = 0; struct mptcp_subflow_data sfd; char __user *addrptr; From 31b4e63eb24a81770b0e46a768f606a114787262 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Thu, 10 Nov 2022 15:23:21 -0800 Subject: [PATCH 4/5] selftests: mptcp: use max_time instead of time 'time' is the local variable of run_test() function, while 'max_time' is the local variable of do_transfer() function. So in do_transfer(), $max_time should be used, not $time. Please note that here $time == $max_time so the behaviour is not changed but the right variable is used. Reviewed-by: Mat Martineau Signed-off-by: Geliang Tang Signed-off-by: Mat Martineau Signed-off-by: Jakub Kicinski --- tools/testing/selftests/net/mptcp/simult_flows.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh index ffa13a957a363..af70c14e0bf94 100755 --- a/tools/testing/selftests/net/mptcp/simult_flows.sh +++ b/tools/testing/selftests/net/mptcp/simult_flows.sh @@ -173,7 +173,7 @@ do_transfer() timeout ${timeout_test} \ ip netns exec ${ns3} \ - ./mptcp_connect -jt ${timeout_poll} -l -p $port -T $time \ + ./mptcp_connect -jt ${timeout_poll} -l -p $port -T $max_time \ 0.0.0.0 < "$sin" > "$sout" & local spid=$! @@ -181,7 +181,7 @@ do_transfer() timeout ${timeout_test} \ ip netns exec ${ns1} \ - ./mptcp_connect -jt ${timeout_poll} -p $port -T $time \ + ./mptcp_connect -jt ${timeout_poll} -p $port -T $max_time \ 10.0.3.3 < "$cin" > "$cout" & local cpid=$! From 4373bf4b72f93af9d045450a44d294953c590e2d Mon Sep 17 00:00:00 2001 From: Mat Martineau Date: Thu, 10 Nov 2022 15:23:22 -0800 Subject: [PATCH 5/5] mptcp: Fix grammar in a comment We kept getting initial patches from new contributors to remove a duplicate 'the' (since grammar checking scripts flag it), but submitters never followed up after code review. Reviewed-by: Matthieu Baerts Signed-off-by: Mat Martineau Signed-off-by: Jakub Kicinski --- net/mptcp/token.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/mptcp/token.c b/net/mptcp/token.c index f52ee7b26aed1..65430f314a687 100644 --- a/net/mptcp/token.c +++ b/net/mptcp/token.c @@ -287,8 +287,8 @@ EXPORT_SYMBOL_GPL(mptcp_token_get_sock); * This function returns the first mptcp connection structure found inside the * token container starting from the specified position, or NULL. * - * On successful iteration, the iterator is move to the next position and the - * the acquires a reference to the returned socket. + * On successful iteration, the iterator is moved to the next position and + * a reference to the returned socket is acquired. */ struct mptcp_sock *mptcp_token_iter_next(const struct net *net, long *s_slot, long *s_num)