* [PATCH mptcp-next v7 01/12] Squash to "mptcp: add get_subflow wrappers"
2022-10-12 4:39 [PATCH mptcp-next v7 00/12] refactor push pending Geliang Tang
@ 2022-10-12 4:39 ` Geliang Tang
2022-10-12 4:39 ` [PATCH mptcp-next v7 02/12] mptcp: change 'first' as a parameter Geliang Tang
` (10 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2022-10-12 4:39 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
Please update the commit log:
'''
This patch defines two new wrappers mptcp_sched_get_send() and
mptcp_sched_get_retrans(), invoke get_subflow() of msk->sched in them.
Set the subflow pointers array in struct mptcp_sched_data before invoking
get_subflow(), then it can be used in get_subflow() in the BPF contexts.
Check the subflow scheduled flags to test which subflow or subflows are
picked by the scheduler.
Move sock_owned_by_me() and the fallback check code from
mptcp_subflow_get_send/retrans() into the wrappers.
'''
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/mptcp/protocol.c | 8 +++---
net/mptcp/protocol.h | 4 +--
net/mptcp/sched.c | 61 +++++++++++++++++++++-----------------------
3 files changed, 35 insertions(+), 38 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 1aa940928b4f..9a2253436f50 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1547,7 +1547,7 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
int ret = 0;
prev_ssk = ssk;
- ssk = mptcp_sched_get_send(msk);
+ ssk = mptcp_subflow_get_send(msk);
/* First check. If the ssk has changed since
* the last round, release prev_ssk
@@ -1616,7 +1616,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_sched_get_send(mptcp_sk(sk));
+ xmit_ssk = first ? ssk : mptcp_subflow_get_send(mptcp_sk(sk));
if (!xmit_ssk)
goto out;
if (xmit_ssk != ssk) {
@@ -2481,7 +2481,7 @@ static void __mptcp_retrans(struct sock *sk)
mptcp_clean_una_wakeup(sk);
/* first check ssk: need to kick "stale" logic */
- ssk = mptcp_sched_get_retrans(msk);
+ ssk = mptcp_subflow_get_retrans(msk);
dfrag = mptcp_rtx_head(sk);
if (!dfrag) {
if (mptcp_data_fin_enabled(msk)) {
@@ -3199,7 +3199,7 @@ void __mptcp_check_push(struct sock *sk, struct sock *ssk)
return;
if (!sock_owned_by_user(sk)) {
- struct sock *xmit_ssk = mptcp_sched_get_send(mptcp_sk(sk));
+ struct sock *xmit_ssk = mptcp_subflow_get_send(mptcp_sk(sk));
if (xmit_ssk == ssk)
__mptcp_subflow_push_pending(sk, ssk);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 18f866b1afda..d44c97bb41db 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -642,8 +642,8 @@ void mptcp_subflow_set_scheduled(struct mptcp_subflow_context *subflow,
bool scheduled);
struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk);
struct sock *mptcp_subflow_get_retrans(struct mptcp_sock *msk);
-struct sock *mptcp_sched_get_send(struct mptcp_sock *msk);
-struct sock *mptcp_sched_get_retrans(struct mptcp_sock *msk);
+int mptcp_sched_get_send(struct mptcp_sock *msk);
+int mptcp_sched_get_retrans(struct mptcp_sock *msk);
static inline bool __tcp_can_send(const struct sock *ssk)
{
diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
index 044c5ec8bbfb..9b128714055a 100644
--- a/net/mptcp/sched.c
+++ b/net/mptcp/sched.c
@@ -114,67 +114,64 @@ static int mptcp_sched_data_init(struct mptcp_sock *msk, bool reinject,
for (; i < MPTCP_SUBFLOWS_MAX; i++)
data->contexts[i] = NULL;
+ msk->snd_burst = 0;
+
return 0;
}
-struct sock *mptcp_sched_get_send(struct mptcp_sock *msk)
+int mptcp_sched_get_send(struct mptcp_sock *msk)
{
struct mptcp_sched_data data;
struct sock *ssk = NULL;
- int i;
- sock_owned_by_me((struct sock *)msk);
+ sock_owned_by_me((const struct sock *)msk);
/* the following check is moved out of mptcp_subflow_get_send */
if (__mptcp_check_fallback(msk)) {
- if (!msk->first)
- return NULL;
- return __tcp_can_send(msk->first) &&
- sk_stream_memory_free(msk->first) ? msk->first : NULL;
+ if (msk->first &&
+ __tcp_can_send(msk->first) &&
+ sk_stream_memory_free(msk->first)) {
+ mptcp_subflow_set_scheduled(mptcp_subflow_ctx(msk->first), true);
+ return 0;
+ }
+ return -EINVAL;
}
- if (!msk->sched)
- return mptcp_subflow_get_send(msk);
+ if (!msk->sched) {
+ ssk = mptcp_subflow_get_send(msk);
+ if (!ssk)
+ return -EINVAL;
+ mptcp_subflow_set_scheduled(mptcp_subflow_ctx(ssk), true);
+ return 0;
+ }
mptcp_sched_data_init(msk, false, &data);
msk->sched->get_subflow(msk, &data);
- for (i = 0; i < MPTCP_SUBFLOWS_MAX; i++) {
- if (data.contexts[i] && READ_ONCE(data.contexts[i]->scheduled)) {
- ssk = data.contexts[i]->tcp_sock;
- msk->last_snd = ssk;
- break;
- }
- }
-
- return ssk;
+ return 0;
}
-struct sock *mptcp_sched_get_retrans(struct mptcp_sock *msk)
+int mptcp_sched_get_retrans(struct mptcp_sock *msk)
{
struct mptcp_sched_data data;
struct sock *ssk = NULL;
- int i;
sock_owned_by_me((const struct sock *)msk);
/* the following check is moved out of mptcp_subflow_get_retrans */
if (__mptcp_check_fallback(msk))
- return NULL;
+ return -EINVAL;
- if (!msk->sched)
- return mptcp_subflow_get_retrans(msk);
+ if (!msk->sched) {
+ ssk = mptcp_subflow_get_retrans(msk);
+ if (!ssk)
+ return -EINVAL;
+ mptcp_subflow_set_scheduled(mptcp_subflow_ctx(ssk), true);
+ return 0;
+ }
mptcp_sched_data_init(msk, true, &data);
msk->sched->get_subflow(msk, &data);
- for (i = 0; i < MPTCP_SUBFLOWS_MAX; i++) {
- if (data.contexts[i] && READ_ONCE(data.contexts[i]->scheduled)) {
- ssk = data.contexts[i]->tcp_sock;
- msk->last_snd = ssk;
- break;
- }
- }
-
- return ssk;
+ return 0;
}
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH mptcp-next v7 02/12] mptcp: change 'first' as a parameter
2022-10-12 4:39 [PATCH mptcp-next v7 00/12] refactor push pending Geliang Tang
2022-10-12 4:39 ` [PATCH mptcp-next v7 01/12] Squash to "mptcp: add get_subflow wrappers" Geliang Tang
@ 2022-10-12 4:39 ` Geliang Tang
2022-10-12 4:39 ` [PATCH mptcp-next v7 03/12] mptcp: refactor push_pending logic Geliang Tang
` (9 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2022-10-12 4:39 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
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.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/mptcp/protocol.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 9a2253436f50..0285b21ff912 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1593,7 +1593,8 @@ 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 = {
@@ -1602,7 +1603,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))) {
@@ -1612,8 +1612,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(mptcp_sk(sk));
@@ -3198,16 +3197,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) | \
@@ -3298,7 +3291,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);
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH mptcp-next v7 03/12] mptcp: refactor push_pending logic
2022-10-12 4:39 [PATCH mptcp-next v7 00/12] refactor push pending Geliang Tang
2022-10-12 4:39 ` [PATCH mptcp-next v7 01/12] Squash to "mptcp: add get_subflow wrappers" Geliang Tang
2022-10-12 4:39 ` [PATCH mptcp-next v7 02/12] mptcp: change 'first' as a parameter Geliang Tang
@ 2022-10-12 4:39 ` Geliang Tang
2022-10-12 4:39 ` [PATCH mptcp-next v7 04/12] mptcp: drop last_snd of struct mptcp_sock Geliang Tang
` (8 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2022-10-12 4:39 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
To support redundant package schedulers more easily, this patch refactors
__mptcp_push_pending() logic from:
For each dfrag:
While sends succeed:
Call the scheduler (selects subflow and msk->snd_burst)
Update subflow locks (push/release/acquire as needed)
Send the dfrag data with mptcp_sendmsg_frag()
Update already_sent, snd_nxt, snd_burst
Update msk->first_pending
Push/release on final subflow
->
While the scheduler selects one subflow:
Lock the subflow
For each pending dfrag:
While sends succeed:
Send the dfrag data with mptcp_sendmsg_frag()
Update already_sent, snd_nxt, snd_burst
Update msk->first_pending
Break if required by msk->snd_burst / etc
Push and release the subflow
This patch also moves the burst check conditions out of the function
mptcp_subflow_get_send(), check them in __mptcp_push_pending() and
__mptcp_subflow_push_pending() in the inner "for each pending dfrag"
loop.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/mptcp/protocol.c | 86 ++++++++++++++++++++------------------------
1 file changed, 39 insertions(+), 47 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 0285b21ff912..52ac57fd8c27 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1417,14 +1417,6 @@ struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
u64 linger_time;
long tout = 0;
- /* re-use last subflow, if the burst allow that */
- if (msk->last_snd && msk->snd_burst > 0 &&
- sk_stream_memory_free(msk->last_snd) &&
- mptcp_subflow_active(mptcp_subflow_ctx(msk->last_snd))) {
- mptcp_set_timeout(sk);
- return msk->last_snd;
- }
-
/* pick the subflow with the lower wmem/wspace ratio */
for (i = 0; i < SSK_MODE_MAX; ++i) {
send_info[i].ssk = NULL;
@@ -1530,60 +1522,53 @@ void mptcp_check_and_set_pending(struct sock *sk)
void __mptcp_push_pending(struct sock *sk, unsigned int flags)
{
- struct sock *prev_ssk = NULL, *ssk = NULL;
struct mptcp_sock *msk = mptcp_sk(sk);
struct mptcp_sendmsg_info info = {
.flags = flags,
};
bool do_check_data_fin = false;
struct mptcp_data_frag *dfrag;
+ struct sock *ssk;
int len;
- while ((dfrag = mptcp_send_head(sk))) {
- info.sent = dfrag->already_sent;
- info.limit = dfrag->data_len;
- len = dfrag->data_len - dfrag->already_sent;
- while (len > 0) {
- int ret = 0;
-
- prev_ssk = ssk;
- ssk = mptcp_subflow_get_send(msk);
-
- /* First check. If the ssk has changed since
- * the last round, release prev_ssk
- */
- if (ssk != prev_ssk && prev_ssk)
- mptcp_push_release(prev_ssk, &info);
- if (!ssk)
- goto out;
+again:
+ while (mptcp_send_head(sk) && (ssk = mptcp_subflow_get_send(msk))) {
+ lock_sock(ssk);
- /* Need to lock the new subflow only if different
- * from the previous one, otherwise we are still
- * helding the relevant lock
- */
- if (ssk != prev_ssk)
- lock_sock(ssk);
+ while ((dfrag = mptcp_send_head(sk))) {
+ info.sent = dfrag->already_sent;
+ info.limit = dfrag->data_len;
+ len = dfrag->data_len - dfrag->already_sent;
+ while (len > 0) {
+ int ret = 0;
+
+ ret = mptcp_sendmsg_frag(sk, ssk, dfrag, &info);
+ if (ret <= 0) {
+ mptcp_push_release(ssk, &info);
+ if (ret == -EAGAIN)
+ goto again;
+ goto out;
+ }
+
+ do_check_data_fin = true;
+ info.sent += ret;
+ len -= ret;
+
+ mptcp_update_post_push(msk, dfrag, ret);
+ }
+ WRITE_ONCE(msk->first_pending, mptcp_send_next(sk));
- ret = mptcp_sendmsg_frag(sk, ssk, dfrag, &info);
- if (ret <= 0) {
- if (ret == -EAGAIN)
- continue;
+ if (msk->snd_burst <= 0 ||
+ !sk_stream_memory_free(ssk) ||
+ !mptcp_subflow_active(mptcp_subflow_ctx(ssk))) {
mptcp_push_release(ssk, &info);
- goto out;
+ goto again;
}
-
- do_check_data_fin = true;
- info.sent += ret;
- len -= ret;
-
- mptcp_update_post_push(msk, dfrag, ret);
+ mptcp_set_timeout(sk);
}
- WRITE_ONCE(msk->first_pending, mptcp_send_next(sk));
- }
- /* at this point we held the socket lock for the last subflow we used */
- if (ssk)
mptcp_push_release(ssk, &info);
+ }
out:
/* ensure the rtx timer is running */
@@ -1636,6 +1621,13 @@ static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk,
mptcp_update_post_push(msk, dfrag, ret);
}
WRITE_ONCE(msk->first_pending, mptcp_send_next(sk));
+
+ if (msk->snd_burst <= 0 ||
+ !sk_stream_memory_free(ssk) ||
+ !mptcp_subflow_active(mptcp_subflow_ctx(ssk))) {
+ goto out;
+ }
+ mptcp_set_timeout(sk);
}
out:
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH mptcp-next v7 04/12] mptcp: drop last_snd of struct mptcp_sock
2022-10-12 4:39 [PATCH mptcp-next v7 00/12] refactor push pending Geliang Tang
` (2 preceding siblings ...)
2022-10-12 4:39 ` [PATCH mptcp-next v7 03/12] mptcp: refactor push_pending logic Geliang Tang
@ 2022-10-12 4:39 ` Geliang Tang
2022-10-12 4:40 ` [PATCH mptcp-next v7 05/12] mptcp: simplify push_pending Geliang Tang
` (7 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2022-10-12 4:39 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
msk->last_snd is no longer used anymore, drop it as well as the macro
MPTCP_RESET_SCHEDULER.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/mptcp/pm.c | 9 +--------
net/mptcp/pm_netlink.c | 3 ---
net/mptcp/protocol.c | 11 +----------
net/mptcp/protocol.h | 2 --
4 files changed, 2 insertions(+), 23 deletions(-)
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 45e2a48397b9..cdeb7280ac76 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -282,15 +282,8 @@ void mptcp_pm_mp_prio_received(struct sock *ssk, u8 bkup)
pr_debug("subflow->backup=%d, bkup=%d\n", subflow->backup, bkup);
msk = mptcp_sk(sk);
- if (subflow->backup != bkup) {
+ if (subflow->backup != bkup)
subflow->backup = bkup;
- mptcp_data_lock(sk);
- if (!sock_owned_by_user(sk))
- msk->last_snd = NULL;
- else
- __set_bit(MPTCP_RESET_SCHEDULER, &msk->cb_flags);
- mptcp_data_unlock(sk);
- }
mptcp_event(MPTCP_EVENT_SUB_PRIORITY, msk, ssk, GFP_ATOMIC);
}
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 9813ed0fde9b..1f2da4aedcb4 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -475,9 +475,6 @@ static void __mptcp_pm_send_ack(struct mptcp_sock *msk, struct mptcp_subflow_con
slow = lock_sock_fast(ssk);
if (prio) {
- if (subflow->backup != backup)
- msk->last_snd = NULL;
-
subflow->send_mp_prio = 1;
subflow->backup = backup;
subflow->request_bkup = backup;
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 52ac57fd8c27..7d5f89799c9a 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1469,16 +1469,13 @@ struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
burst = min_t(int, MPTCP_SEND_BURST_SIZE, mptcp_wnd_end(msk) - msk->snd_nxt);
wmem = READ_ONCE(ssk->sk_wmem_queued);
- if (!burst) {
- msk->last_snd = NULL;
+ if (!burst)
return ssk;
- }
subflow = mptcp_subflow_ctx(ssk);
subflow->avg_pacing_rate = div_u64((u64)subflow->avg_pacing_rate * wmem +
READ_ONCE(ssk->sk_pacing_rate) * burst,
burst + wmem);
- msk->last_snd = ssk;
msk->snd_burst = burst;
return ssk;
}
@@ -2346,9 +2343,6 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
msk->first = NULL;
out:
- if (ssk == msk->last_snd)
- msk->last_snd = NULL;
-
if (need_push)
__mptcp_push_pending(sk, 0);
}
@@ -2981,7 +2975,6 @@ static int mptcp_disconnect(struct sock *sk, int flags)
* subflow
*/
mptcp_destroy_common(msk, MPTCP_CF_FASTCLOSE);
- msk->last_snd = NULL;
WRITE_ONCE(msk->flags, 0);
msk->cb_flags = 0;
msk->push_pending = 0;
@@ -3242,8 +3235,6 @@ static void mptcp_release_cb(struct sock *sk)
__mptcp_set_connected(sk);
if (__test_and_clear_bit(MPTCP_ERROR_REPORT, &msk->cb_flags))
__mptcp_error_report(sk);
- if (__test_and_clear_bit(MPTCP_RESET_SCHEDULER, &msk->cb_flags))
- msk->last_snd = NULL;
}
__mptcp_update_rmem(sk);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index d44c97bb41db..68a2b22a47eb 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -124,7 +124,6 @@
#define MPTCP_RETRANSMIT 4
#define MPTCP_FLUSH_JOIN_LIST 5
#define MPTCP_CONNECTED 6
-#define MPTCP_RESET_SCHEDULER 7
static inline bool before64(__u64 seq1, __u64 seq2)
{
@@ -258,7 +257,6 @@ struct mptcp_sock {
atomic64_t rcv_wnd_sent;
u64 rcv_data_fin_seq;
int rmem_fwd_alloc;
- struct sock *last_snd;
int snd_burst;
int old_wspace;
u64 recovery_snd_nxt; /* in recovery mode accept up to this seq;
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH mptcp-next v7 05/12] mptcp: simplify push_pending
2022-10-12 4:39 [PATCH mptcp-next v7 00/12] refactor push pending Geliang Tang
` (3 preceding siblings ...)
2022-10-12 4:39 ` [PATCH mptcp-next v7 04/12] mptcp: drop last_snd of struct mptcp_sock Geliang Tang
@ 2022-10-12 4:40 ` Geliang Tang
2022-10-12 4:40 ` [PATCH mptcp-next v7 06/12] mptcp: multi subflows push_pending Geliang Tang
` (6 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2022-10-12 4:40 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
This patch moves the duplicate code from __mptcp_push_pending() and
__mptcp_subflow_push_pending() into a new helper function, named
__subflow_push_pending(). And simplify __mptcp_push_pending() by
invoking this helper.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/mptcp/protocol.c | 95 +++++++++++++++++++++++++-------------------
1 file changed, 54 insertions(+), 41 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 7d5f89799c9a..817e539d1d12 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1480,12 +1480,6 @@ struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
return ssk;
}
-static void mptcp_push_release(struct sock *ssk, struct mptcp_sendmsg_info *info)
-{
- tcp_push(ssk, 0, info->mss_now, tcp_sk(ssk)->nonagle, info->size_goal);
- release_sock(ssk);
-}
-
static void mptcp_update_post_push(struct mptcp_sock *msk,
struct mptcp_data_frag *dfrag,
u32 sent)
@@ -1517,61 +1511,80 @@ void mptcp_check_and_set_pending(struct sock *sk)
mptcp_sk(sk)->push_pending |= BIT(MPTCP_PUSH_PENDING);
}
+static int __subflow_push_pending(struct sock *sk, struct sock *ssk,
+ struct mptcp_sendmsg_info *info)
+{
+ struct mptcp_sock *msk = mptcp_sk(sk);
+ struct mptcp_data_frag *dfrag;
+ int len, copied = 0, err = 0;
+
+ while ((dfrag = mptcp_send_head(sk))) {
+ info->sent = dfrag->already_sent;
+ info->limit = dfrag->data_len;
+ len = dfrag->data_len - dfrag->already_sent;
+ while (len > 0) {
+ int ret = 0;
+
+ ret = mptcp_sendmsg_frag(sk, ssk, dfrag, info);
+ if (ret <= 0) {
+ err = copied ? : ret;
+ goto out;
+ }
+
+ info->sent += ret;
+ copied += ret;
+ len -= ret;
+
+ mptcp_update_post_push(msk, dfrag, ret);
+ }
+ WRITE_ONCE(msk->first_pending, mptcp_send_next(sk));
+
+ if (msk->snd_burst <= 0 ||
+ !sk_stream_memory_free(ssk) ||
+ !mptcp_subflow_active(mptcp_subflow_ctx(ssk))) {
+ err = copied ? : -EAGAIN;
+ goto out;
+ }
+ mptcp_set_timeout(sk);
+ }
+ err = copied;
+
+out:
+ if (copied) {
+ tcp_push(ssk, 0, info->mss_now, tcp_sk(ssk)->nonagle,
+ info->size_goal);
+ }
+
+ return err;
+}
+
void __mptcp_push_pending(struct sock *sk, unsigned int flags)
{
struct mptcp_sock *msk = mptcp_sk(sk);
struct mptcp_sendmsg_info info = {
.flags = flags,
};
- bool do_check_data_fin = false;
- struct mptcp_data_frag *dfrag;
struct sock *ssk;
- int len;
+ int ret = 0;
again:
while (mptcp_send_head(sk) && (ssk = mptcp_subflow_get_send(msk))) {
lock_sock(ssk);
+ ret = __subflow_push_pending(sk, ssk, &info);
+ release_sock(ssk);
- while ((dfrag = mptcp_send_head(sk))) {
- info.sent = dfrag->already_sent;
- info.limit = dfrag->data_len;
- len = dfrag->data_len - dfrag->already_sent;
- while (len > 0) {
- int ret = 0;
-
- ret = mptcp_sendmsg_frag(sk, ssk, dfrag, &info);
- if (ret <= 0) {
- mptcp_push_release(ssk, &info);
- if (ret == -EAGAIN)
- goto again;
- goto out;
- }
-
- do_check_data_fin = true;
- info.sent += ret;
- len -= ret;
-
- mptcp_update_post_push(msk, dfrag, ret);
- }
- WRITE_ONCE(msk->first_pending, mptcp_send_next(sk));
-
- if (msk->snd_burst <= 0 ||
- !sk_stream_memory_free(ssk) ||
- !mptcp_subflow_active(mptcp_subflow_ctx(ssk))) {
- mptcp_push_release(ssk, &info);
+ if (ret <= 0) {
+ if (ret == -EAGAIN)
goto again;
- }
- mptcp_set_timeout(sk);
+ goto out;
}
-
- mptcp_push_release(ssk, &info);
}
out:
/* ensure the rtx timer is running */
if (!mptcp_timer_pending(sk))
mptcp_reset_timer(sk);
- if (do_check_data_fin)
+ if (ret > 0)
__mptcp_check_send_data_fin(sk);
}
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH mptcp-next v7 06/12] mptcp: multi subflows push_pending
2022-10-12 4:39 [PATCH mptcp-next v7 00/12] refactor push pending Geliang Tang
` (4 preceding siblings ...)
2022-10-12 4:40 ` [PATCH mptcp-next v7 05/12] mptcp: simplify push_pending Geliang Tang
@ 2022-10-12 4:40 ` Geliang Tang
2022-10-12 4:40 ` [PATCH mptcp-next v7 07/12] mptcp: use msk instead of mptcp_sk Geliang Tang
` (5 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2022-10-12 4:40 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
This patch adds the multiple subflows support for __mptcp_push_pending().
Use mptcp_sched_get_send() wrapper instead of mptcp_subflow_get_send() in
it.
Check the subflow scheduled flags to test which subflow or subflows are
picked by the scheduler, use them to send data.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/mptcp/protocol.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 817e539d1d12..86ac38d10bc4 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1561,22 +1561,30 @@ static int __subflow_push_pending(struct sock *sk, struct sock *ssk,
void __mptcp_push_pending(struct sock *sk, unsigned int flags)
{
struct mptcp_sock *msk = mptcp_sk(sk);
- struct mptcp_sendmsg_info info = {
- .flags = flags,
- };
- struct sock *ssk;
int ret = 0;
again:
- while (mptcp_send_head(sk) && (ssk = mptcp_subflow_get_send(msk))) {
- lock_sock(ssk);
- ret = __subflow_push_pending(sk, ssk, &info);
- release_sock(ssk);
+ while (mptcp_send_head(sk) && !mptcp_sched_get_send(msk)) {
+ struct mptcp_subflow_context *subflow;
+ struct mptcp_sendmsg_info info = {
+ .flags = flags,
+ };
- if (ret <= 0) {
- if (ret == -EAGAIN)
- goto again;
- goto out;
+ mptcp_for_each_subflow(msk, subflow) {
+ if (READ_ONCE(subflow->scheduled)) {
+ struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+
+ lock_sock(ssk);
+ ret = __subflow_push_pending(sk, ssk, &info);
+ release_sock(ssk);
+
+ if (ret <= 0) {
+ if (ret == -EAGAIN)
+ goto again;
+ goto out;
+ }
+ mptcp_subflow_set_scheduled(subflow, false);
+ }
}
}
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH mptcp-next v7 07/12] mptcp: use msk instead of mptcp_sk
2022-10-12 4:39 [PATCH mptcp-next v7 00/12] refactor push pending Geliang Tang
` (5 preceding siblings ...)
2022-10-12 4:40 ` [PATCH mptcp-next v7 06/12] mptcp: multi subflows push_pending Geliang Tang
@ 2022-10-12 4:40 ` Geliang Tang
2022-10-12 4:40 ` [PATCH mptcp-next v7 08/12] mptcp: refactor subflow_push_pending logic Geliang Tang
` (4 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2022-10-12 4:40 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
Use msk instead of mptcp_sk(sk) in the functions where the variable
"msk = mptcp_sk(sk)" has been defined.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
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 86ac38d10bc4..abdaeb0d867a 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1618,7 +1618,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) {
@@ -2251,7 +2251,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))
@@ -2930,7 +2930,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) {
@@ -2989,8 +2989,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
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH mptcp-next v7 08/12] mptcp: refactor subflow_push_pending logic
2022-10-12 4:39 [PATCH mptcp-next v7 00/12] refactor push pending Geliang Tang
` (6 preceding siblings ...)
2022-10-12 4:40 ` [PATCH mptcp-next v7 07/12] mptcp: use msk instead of mptcp_sk Geliang Tang
@ 2022-10-12 4:40 ` Geliang Tang
2022-10-12 4:40 ` [PATCH mptcp-next v7 09/12] mptcp: simplify subflow_push_pending Geliang Tang
` (3 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2022-10-12 4:40 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
This patch refactors __mptcp_subflow_push_pending logic from:
For each dfrag:
While sends succeed:
Call the scheduler (selects subflow and msk->snd_burst)
Send the dfrag data with mptcp_subflow_delegate(), break
Send the dfrag data with mptcp_sendmsg_frag()
Update dfrag->already_sent, msk->snd_nxt, msk->snd_burst
Update msk->first_pending
->
While first_pending isn't empty:
Call the scheduler (selects subflow and msk->snd_burst)
Send the dfrag data with mptcp_subflow_delegate(), break
Send the dfrag data with mptcp_sendmsg_frag()
For each pending dfrag:
While sends succeed:
Send the dfrag data with mptcp_sendmsg_frag()
Update already_sent, snd_nxt, snd_burst
Update msk->first_pending
Break if required by msk->snd_burst / etc
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/mptcp/protocol.c | 66 +++++++++++++++++++++++---------------------
1 file changed, 34 insertions(+), 32 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index abdaeb0d867a..fcb65c4ec774 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1608,44 +1608,46 @@ static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk,
int len, copied = 0;
info.flags = 0;
- while ((dfrag = mptcp_send_head(sk))) {
- info.sent = dfrag->already_sent;
- info.limit = dfrag->data_len;
- len = dfrag->data_len - dfrag->already_sent;
- while (len > 0) {
- int ret = 0;
+ while (mptcp_send_head(sk)) {
+ /* check for a different subflow usage only after
+ * spooling the first chunk of data
+ */
+ xmit_ssk = first ? ssk : mptcp_subflow_get_send(msk);
+ if (!xmit_ssk)
+ goto out;
+ if (xmit_ssk != ssk) {
+ mptcp_subflow_delegate(mptcp_subflow_ctx(xmit_ssk),
+ MPTCP_DELEGATE_SEND);
+ goto out;
+ }
- /* check for a different subflow usage only after
- * spooling the first chunk of data
- */
- xmit_ssk = first ? ssk : mptcp_subflow_get_send(msk);
- if (!xmit_ssk)
- goto out;
- if (xmit_ssk != ssk) {
- mptcp_subflow_delegate(mptcp_subflow_ctx(xmit_ssk),
- MPTCP_DELEGATE_SEND);
- goto out;
- }
+ while ((dfrag = mptcp_send_head(sk))) {
+ info.sent = dfrag->already_sent;
+ info.limit = dfrag->data_len;
+ len = dfrag->data_len - dfrag->already_sent;
+ while (len > 0) {
+ int ret = 0;
- ret = mptcp_sendmsg_frag(sk, ssk, dfrag, &info);
- if (ret <= 0)
- goto out;
+ ret = mptcp_sendmsg_frag(sk, ssk, dfrag, &info);
+ if (ret <= 0)
+ goto out;
- info.sent += ret;
- copied += ret;
- len -= ret;
- first = false;
+ info.sent += ret;
+ copied += ret;
+ len -= ret;
+ first = false;
- mptcp_update_post_push(msk, dfrag, ret);
- }
- WRITE_ONCE(msk->first_pending, mptcp_send_next(sk));
+ mptcp_update_post_push(msk, dfrag, ret);
+ }
+ WRITE_ONCE(msk->first_pending, mptcp_send_next(sk));
- if (msk->snd_burst <= 0 ||
- !sk_stream_memory_free(ssk) ||
- !mptcp_subflow_active(mptcp_subflow_ctx(ssk))) {
- goto out;
+ if (msk->snd_burst <= 0 ||
+ !sk_stream_memory_free(ssk) ||
+ !mptcp_subflow_active(mptcp_subflow_ctx(ssk))) {
+ goto out;
+ }
+ mptcp_set_timeout(sk);
}
- mptcp_set_timeout(sk);
}
out:
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH mptcp-next v7 09/12] mptcp: simplify subflow_push_pending
2022-10-12 4:39 [PATCH mptcp-next v7 00/12] refactor push pending Geliang Tang
` (7 preceding siblings ...)
2022-10-12 4:40 ` [PATCH mptcp-next v7 08/12] mptcp: refactor subflow_push_pending logic Geliang Tang
@ 2022-10-12 4:40 ` Geliang Tang
2022-10-12 4:40 ` [PATCH mptcp-next v7 10/12] mptcp: multi subflows subflow_push_pending Geliang Tang
` (2 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2022-10-12 4:40 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
This patch simplifies __mptcp_subflow_push_pending() by invoking
__subflow_push_pending() helper.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/mptcp/protocol.c | 39 ++++++++-------------------------------
1 file changed, 8 insertions(+), 31 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index fcb65c4ec774..36525d7ea8e7 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1603,11 +1603,11 @@ static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk,
struct mptcp_sendmsg_info info = {
.data_lock_held = true,
};
- struct mptcp_data_frag *dfrag;
struct sock *xmit_ssk;
- int len, copied = 0;
+ int ret = 0;
info.flags = 0;
+again:
while (mptcp_send_head(sk)) {
/* check for a different subflow usage only after
* spooling the first chunk of data
@@ -1621,32 +1621,11 @@ static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk,
goto out;
}
- while ((dfrag = mptcp_send_head(sk))) {
- info.sent = dfrag->already_sent;
- info.limit = dfrag->data_len;
- len = dfrag->data_len - dfrag->already_sent;
- while (len > 0) {
- int ret = 0;
-
- ret = mptcp_sendmsg_frag(sk, ssk, dfrag, &info);
- if (ret <= 0)
- goto out;
-
- info.sent += ret;
- copied += ret;
- len -= ret;
- first = false;
-
- mptcp_update_post_push(msk, dfrag, ret);
- }
- WRITE_ONCE(msk->first_pending, mptcp_send_next(sk));
-
- if (msk->snd_burst <= 0 ||
- !sk_stream_memory_free(ssk) ||
- !mptcp_subflow_active(mptcp_subflow_ctx(ssk))) {
- goto out;
- }
- mptcp_set_timeout(sk);
+ ret = __subflow_push_pending(sk, ssk, &info);
+ if (ret <= 0) {
+ if (ret == -EAGAIN)
+ goto again;
+ break;
}
}
@@ -1654,9 +1633,7 @@ static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk,
/* __mptcp_alloc_tx_skb could have released some wmem and we are
* not going to flush it via release_sock()
*/
- if (copied) {
- tcp_push(ssk, 0, info.mss_now, tcp_sk(ssk)->nonagle,
- info.size_goal);
+ if (ret > 0) {
if (!mptcp_timer_pending(sk))
mptcp_reset_timer(sk);
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH mptcp-next v7 10/12] mptcp: multi subflows subflow_push_pending
2022-10-12 4:39 [PATCH mptcp-next v7 00/12] refactor push pending Geliang Tang
` (8 preceding siblings ...)
2022-10-12 4:40 ` [PATCH mptcp-next v7 09/12] mptcp: simplify subflow_push_pending Geliang Tang
@ 2022-10-12 4:40 ` Geliang Tang
2022-10-14 1:14 ` Mat Martineau
2022-10-12 4:40 ` [PATCH mptcp-next v7 11/12] mptcp: multi subflows retrans support Geliang Tang
2022-10-12 4:40 ` [PATCH mptcp-next v7 12/12] Squash to "selftests/bpf: Add bpf_rr scheduler" Geliang Tang
11 siblings, 1 reply; 15+ messages in thread
From: Geliang Tang @ 2022-10-12 4:40 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
This patch adds the multiple subflows support for
__mptcp_subflow_push_pending(). Use mptcp_sched_get_send() wrapper
instead of mptcp_subflow_get_send() in it.
Check the subflow scheduled flags to test which subflow or subflows are
picked by the scheduler, use them to send data.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/mptcp/protocol.c | 50 +++++++++++++++++++++++++++++++++-----------
1 file changed, 38 insertions(+), 12 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 36525d7ea8e7..4c9c2c6dfbe5 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1599,11 +1599,11 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk,
bool first)
{
+ struct mptcp_subflow_context *subflow, *last = NULL;
struct mptcp_sock *msk = mptcp_sk(sk);
struct mptcp_sendmsg_info info = {
.data_lock_held = true,
};
- struct sock *xmit_ssk;
int ret = 0;
info.flags = 0;
@@ -1612,20 +1612,46 @@ 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(msk);
- if (!xmit_ssk)
- goto out;
- if (xmit_ssk != ssk) {
- mptcp_subflow_delegate(mptcp_subflow_ctx(xmit_ssk),
- MPTCP_DELEGATE_SEND);
+ if (first) {
+ ret = __subflow_push_pending(sk, ssk, &info);
+ if (ret <= 0) {
+ if (ret == -EAGAIN)
+ goto again;
+ break;
+ }
+ first = false;
+ continue;
+ }
+
+ if (mptcp_sched_get_send(msk))
goto out;
+
+ mptcp_for_each_subflow(msk, subflow) {
+ if (READ_ONCE(subflow->scheduled))
+ last = subflow;
}
- ret = __subflow_push_pending(sk, ssk, &info);
- if (ret <= 0) {
- if (ret == -EAGAIN)
- goto again;
- break;
+ mptcp_for_each_subflow(msk, subflow) {
+ if (READ_ONCE(subflow->scheduled)) {
+ struct sock *xmit_ssk = mptcp_subflow_tcp_sock(subflow);
+
+ if (xmit_ssk != ssk) {
+ mptcp_subflow_delegate(subflow,
+ MPTCP_DELEGATE_SEND);
+ mptcp_subflow_set_scheduled(subflow, false);
+ if (last && subflow != last)
+ continue;
+ goto out;
+ }
+
+ ret = __subflow_push_pending(sk, ssk, &info);
+ if (ret <= 0) {
+ if (ret == -EAGAIN)
+ goto again;
+ goto out;
+ }
+ mptcp_subflow_set_scheduled(subflow, false);
+ }
}
}
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH mptcp-next v7 10/12] mptcp: multi subflows subflow_push_pending
2022-10-12 4:40 ` [PATCH mptcp-next v7 10/12] mptcp: multi subflows subflow_push_pending Geliang Tang
@ 2022-10-14 1:14 ` Mat Martineau
0 siblings, 0 replies; 15+ messages in thread
From: Mat Martineau @ 2022-10-14 1:14 UTC (permalink / raw)
To: Geliang Tang; +Cc: mptcp
On Wed, 12 Oct 2022, Geliang Tang wrote:
> This patch adds the multiple subflows support for
> __mptcp_subflow_push_pending(). Use mptcp_sched_get_send() wrapper
> instead of mptcp_subflow_get_send() in it.
>
> Check the subflow scheduled flags to test which subflow or subflows are
> picked by the scheduler, use them to send data.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
Hi Geliang -
We talked about redundant transmits from __mptcp_subflow_push_pending() in
the meeting today:
https://annuel2.framapad.org/p/mptcp_upstreaming_20221013#L110
It was hard to capture the whole discussion in the notes, but the main
point is that handling the multi-sublow __mptcp_subflow_push_pending() is
a lot more complicated than the __mptcp_push_pending() case.
__mptcp_subflow_push_pending() is running in one of two contexts, where
the ssk lock is already held and sock_owned_by_user(msk) has been checked:
1. From mptcp_incoming_options(). This allows transmission of unsent data
from mptcp_send_head when acks are received on a subflow - if the
scheduler selects the subflow that is already locked, then additional data
can be quickly sent without any delegated actions, release callbacks, or
worker threads. If the scheduler selects a different subflow, a "delegated
action" is used to schedule a callback for that subflow using the NAPI
poll API.
2. From mptcp_napi_poll() (the "delegated action"), as scheduled by the
case above.
There are some constraints to observe here:
In both case 1 and case 2, it should only delegate to one other subflow.
If there are more than two redundant subflows scheduled, there would need
to be a way to chain delegated calls. (I'll explain more about this with
the code below).
It's also important to understand that the subflow and msk data locks are
released between case 1 above and a delegated action that runs later.
__mptcp_push_pending() could run and invoke the scheduler again, which
would overwrite the subflow->scheduled bits that were set by
__mptcp_subflow_push_pending(). This means __mptcp_push_pending() needs to
be aware of multiple-subflow sends that are in progress, and finish those
pushes before calling the scheduler again.
To make all this work, we think more data needs to be stored in each
subflow by the scheduler. Each scheduled subflow needs to know:
* Is it scheduled? (subflow->scheduled)
* Where to start sending
* How much data to try to send on that subflow
As the code is now, __mptcp_subflow_push_pending() will send sequential
data instead of redundant data if multiple subflows are scheduled.
Like I said above, this is getting quite a bit more complicated for the
multiple subflow case. What do you think about posting a v8 patch set that
has only the single subflow refactoring and any necessary squash-to
patches? Then we could add those commits to the export branch, and start
working on a dedicated multi-subflow series?
> ---
> net/mptcp/protocol.c | 50 +++++++++++++++++++++++++++++++++-----------
> 1 file changed, 38 insertions(+), 12 deletions(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 36525d7ea8e7..4c9c2c6dfbe5 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1599,11 +1599,11 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
> static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk,
> bool first)
> {
> + struct mptcp_subflow_context *subflow, *last = NULL;
> struct mptcp_sock *msk = mptcp_sk(sk);
> struct mptcp_sendmsg_info info = {
> .data_lock_held = true,
> };
> - struct sock *xmit_ssk;
> int ret = 0;
>
> info.flags = 0;
> @@ -1612,20 +1612,46 @@ 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(msk);
> - if (!xmit_ssk)
> - goto out;
> - if (xmit_ssk != ssk) {
> - mptcp_subflow_delegate(mptcp_subflow_ctx(xmit_ssk),
> - MPTCP_DELEGATE_SEND);
> + if (first) {
> + ret = __subflow_push_pending(sk, ssk, &info);
> + if (ret <= 0) {
> + if (ret == -EAGAIN)
> + goto again;
> + break;
> + }
> + first = false;
> + continue;
> + }
> +
> + if (mptcp_sched_get_send(msk))
> goto out;
> +
> + mptcp_for_each_subflow(msk, subflow) {
> + if (READ_ONCE(subflow->scheduled))
> + last = subflow;
> }
>
> - ret = __subflow_push_pending(sk, ssk, &info);
> - if (ret <= 0) {
> - if (ret == -EAGAIN)
> - goto again;
> - break;
> + mptcp_for_each_subflow(msk, subflow) {
> + if (READ_ONCE(subflow->scheduled)) {
> + struct sock *xmit_ssk = mptcp_subflow_tcp_sock(subflow);
> +
> + if (xmit_ssk != ssk) {
> + mptcp_subflow_delegate(subflow,
> + MPTCP_DELEGATE_SEND);
> + mptcp_subflow_set_scheduled(subflow, false);
> + if (last && subflow != last)
> + continue;
> + goto out;
mptcp_subflow_delegate() should be called only one time to pick the next
subflow to attempt. mptcp_napi_poll() will try to send on that subflow,
and then call mptcp_subflow_delegate() to schedule the next subflow. This
chain will continue until all the redundant subflows have transmitted
their data.
> + }
> +
> + ret = __subflow_push_pending(sk, ssk, &info);
> + if (ret <= 0) {
> + if (ret == -EAGAIN)
> + goto again;
> + goto out;
> + }
> + mptcp_subflow_set_scheduled(subflow, false);
> + }
> }
> }
>
> --
> 2.35.3
>
>
>
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH mptcp-next v7 11/12] mptcp: multi subflows retrans support
2022-10-12 4:39 [PATCH mptcp-next v7 00/12] refactor push pending Geliang Tang
` (9 preceding siblings ...)
2022-10-12 4:40 ` [PATCH mptcp-next v7 10/12] mptcp: multi subflows subflow_push_pending Geliang Tang
@ 2022-10-12 4:40 ` Geliang Tang
2022-10-12 4:40 ` [PATCH mptcp-next v7 12/12] Squash to "selftests/bpf: Add bpf_rr scheduler" Geliang Tang
11 siblings, 0 replies; 15+ messages in thread
From: Geliang Tang @ 2022-10-12 4:40 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
This patch adds the multiple subflows support for __mptcp_retrans(). In
it, use sched_get_retrans() wrapper instead of mptcp_subflow_get_retrans().
Iterate each subflow of msk, check the scheduled flag to test if it is
picked by the scheduler. If so, use it to retrans data.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/mptcp/protocol.c | 61 +++++++++++++++++++++++++++-----------------
1 file changed, 38 insertions(+), 23 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 4c9c2c6dfbe5..5ddceeb1d334 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2483,16 +2483,17 @@ static void mptcp_check_fastclose(struct mptcp_sock *msk)
static void __mptcp_retrans(struct sock *sk)
{
struct mptcp_sock *msk = mptcp_sk(sk);
+ struct mptcp_subflow_context *subflow;
struct mptcp_sendmsg_info info = {};
struct mptcp_data_frag *dfrag;
- size_t copied = 0;
struct sock *ssk;
- int ret;
+ int ret, err;
+ u16 len = 0;
mptcp_clean_una_wakeup(sk);
/* first check ssk: need to kick "stale" logic */
- ssk = mptcp_subflow_get_retrans(msk);
+ err = mptcp_sched_get_retrans(msk);
dfrag = mptcp_rtx_head(sk);
if (!dfrag) {
if (mptcp_data_fin_enabled(msk)) {
@@ -2511,31 +2512,45 @@ static void __mptcp_retrans(struct sock *sk)
goto reset_timer;
}
- if (!ssk)
+ if (err)
goto reset_timer;
- lock_sock(ssk);
+ mptcp_for_each_subflow(msk, subflow) {
+ if (READ_ONCE(subflow->scheduled)) {
+ u16 copied = 0;
+
+ ssk = mptcp_subflow_tcp_sock(subflow);
+ if (!ssk)
+ goto reset_timer;
+
+ lock_sock(ssk);
+
+ /* limit retransmission to the bytes already sent on some subflows */
+ info.sent = 0;
+ info.limit = READ_ONCE(msk->csum_enabled) ? dfrag->data_len :
+ dfrag->already_sent;
+ while (info.sent < info.limit) {
+ ret = mptcp_sendmsg_frag(sk, ssk, dfrag, &info);
+ if (ret <= 0)
+ break;
+
+ MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RETRANSSEGS);
+ copied += ret;
+ info.sent += ret;
+ }
+ if (copied) {
+ len = max(copied, len);
+ tcp_push(ssk, 0, info.mss_now, tcp_sk(ssk)->nonagle,
+ info.size_goal);
+ WRITE_ONCE(msk->allow_infinite_fallback, false);
+ }
- /* limit retransmission to the bytes already sent on some subflows */
- info.sent = 0;
- info.limit = READ_ONCE(msk->csum_enabled) ? dfrag->data_len : dfrag->already_sent;
- while (info.sent < info.limit) {
- ret = mptcp_sendmsg_frag(sk, ssk, dfrag, &info);
- if (ret <= 0)
- break;
+ release_sock(ssk);
- MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RETRANSSEGS);
- copied += ret;
- info.sent += ret;
- }
- if (copied) {
- dfrag->already_sent = max(dfrag->already_sent, info.sent);
- tcp_push(ssk, 0, info.mss_now, tcp_sk(ssk)->nonagle,
- info.size_goal);
- WRITE_ONCE(msk->allow_infinite_fallback, false);
+ mptcp_subflow_set_scheduled(subflow, false);
+ }
}
-
- release_sock(ssk);
+ dfrag->already_sent = max(dfrag->already_sent, len);
reset_timer:
mptcp_check_and_set_pending(sk);
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH mptcp-next v7 12/12] Squash to "selftests/bpf: Add bpf_rr scheduler"
2022-10-12 4:39 [PATCH mptcp-next v7 00/12] refactor push pending Geliang Tang
` (10 preceding siblings ...)
2022-10-12 4:40 ` [PATCH mptcp-next v7 11/12] mptcp: multi subflows retrans support Geliang Tang
@ 2022-10-12 4:40 ` Geliang Tang
2022-10-12 6:08 ` Squash to "selftests/bpf: Add bpf_rr scheduler": Tests Results MPTCP CI
11 siblings, 1 reply; 15+ messages in thread
From: Geliang Tang @ 2022-10-12 4:40 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
Use last_snd instead of msk->last_snd, then last_snd of struct
mptcp_sock could be removed.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
tools/testing/selftests/bpf/bpf_tcp_helpers.h | 1 -
tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c | 6 ++++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
index c7d4a9a69cfc..c8792e6f125a 100644
--- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
@@ -257,7 +257,6 @@ struct mptcp_sched_ops {
struct mptcp_sock {
struct inet_connection_sock sk;
- struct sock *last_snd;
__u32 token;
struct sock *first;
char ca_name[TCP_CA_NAME_MAX];
diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
index ce4e98f83e43..b7156f6aae8b 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_rr.c
@@ -5,6 +5,7 @@
#include "bpf_tcp_helpers.h"
char _license[] SEC("license") = "GPL";
+struct sock *last_snd = NULL;
SEC("struct_ops/mptcp_sched_rr_init")
void BPF_PROG(mptcp_sched_rr_init, const struct mptcp_sock *msk)
@@ -22,10 +23,10 @@ void BPF_STRUCT_OPS(bpf_rr_get_subflow, const struct mptcp_sock *msk,
int nr = 0;
for (int i = 0; i < MPTCP_SUBFLOWS_MAX; i++) {
- if (!msk->last_snd || !data->contexts[i])
+ if (!last_snd || !data->contexts[i])
break;
- if (data->contexts[i]->tcp_sock == msk->last_snd) {
+ if (data->contexts[i]->tcp_sock == last_snd) {
if (i + 1 == MPTCP_SUBFLOWS_MAX || !data->contexts[i + 1])
break;
@@ -35,6 +36,7 @@ void BPF_STRUCT_OPS(bpf_rr_get_subflow, const struct mptcp_sock *msk,
}
mptcp_subflow_set_scheduled(data->contexts[nr], true);
+ last_snd = data->contexts[nr]->tcp_sock;
}
SEC(".struct_ops")
--
2.35.3
^ permalink raw reply related [flat|nested] 15+ messages in thread