When multiple subflows are active, we can receive a
window update on subflow with no write space available.
MPTCP will try to push frames on such subflow and will
fail. Pending frames will be pushed only after receiving
a window update on a subflow with some wspace available.
Overall the above could lead to suboptimal aggregate
bandwidth usage.
Instead, we should try to push pending frames as soon as
the subflow reaches both conditions mentioned above.
We can finally enable self-tests with asymmetric links,
as the above makes them finally pass.
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
---
net/mptcp/options.c | 13 ++++++++-----
net/mptcp/protocol.c | 2 +-
net/mptcp/protocol.h | 2 +-
tools/testing/selftests/net/mptcp/simult_flows.sh | 6 +++---
4 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index f5f4bd04196d..f4e6808691ed 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -876,10 +876,13 @@ static void ack_update_msk(struct mptcp_sock *msk,
new_wnd_end = new_snd_una + tcp_sk(ssk)->snd_wnd;
- if (after64(new_wnd_end, msk->wnd_end)) {
+ if (after64(new_wnd_end, msk->wnd_end))
msk->wnd_end = new_wnd_end;
- __mptcp_wnd_updated(sk, ssk);
- }
+
+ /* this assumes mptcp_incoming_options() is invoked after tcp_ack() */
+ if (after64(msk->wnd_end, READ_ONCE(msk->snd_nxt)) &&
+ sk_stream_memory_free(sk))
+ __mptcp_check_push(sk, ssk);
if (after64(new_snd_una, old_snd_una)) {
msk->snd_una = new_snd_una;
@@ -945,8 +948,8 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
* helpers are cheap.
*/
mptcp_data_lock(subflow->conn);
- if (mptcp_send_head(subflow->conn))
- __mptcp_wnd_updated(subflow->conn, sk);
+ if (sk_stream_memory_free(sk))
+ __mptcp_check_push(subflow->conn, sk);
__mptcp_data_acked(subflow->conn);
mptcp_data_unlock(subflow->conn);
return;
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 97210eaaa89e..670d49825499 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2919,7 +2919,7 @@ void __mptcp_data_acked(struct sock *sk)
mptcp_schedule_work(sk);
}
-void __mptcp_wnd_updated(struct sock *sk, struct sock *ssk)
+void __mptcp_check_push(struct sock *sk, struct sock *ssk)
{
if (!mptcp_send_head(sk))
return;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index b6feb5340dd0..21474a80efd6 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -506,7 +506,7 @@ void mptcp_rcv_space_init(struct mptcp_sock *msk, const struct sock
*ssk);
void mptcp_data_ready(struct sock *sk, struct sock *ssk);
bool mptcp_finish_join(struct sock *sk);
bool mptcp_schedule_work(struct sock *sk);
-void __mptcp_wnd_updated(struct sock *sk, struct sock *ssk);
+void __mptcp_check_push(struct sock *sk, struct sock *ssk);
void __mptcp_data_acked(struct sock *sk);
void mptcp_subflow_eof(struct sock *sk);
bool mptcp_update_rcv_data_fin(struct mptcp_sock *msk, u64 data_fin_seq, bool
use_64bit);
diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh
b/tools/testing/selftests/net/mptcp/simult_flows.sh
index 2f649b431456..f039ee57eb3c 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -287,7 +287,7 @@ run_test 10 10 0 0 "balanced bwidth"
run_test 10 10 1 50 "balanced bwidth with unbalanced delay"
# we still need some additional infrastructure to pass the following test-cases
-# run_test 30 10 0 0 "unbalanced bwidth"
-# run_test 30 10 1 50 "unbalanced bwidth with unbalanced delay"
-# run_test 30 10 50 1 "unbalanced bwidth with opposed, unbalanced delay"
+run_test 30 10 0 0 "unbalanced bwidth"
+run_test 30 10 1 50 "unbalanced bwidth with unbalanced delay"
+run_test 30 10 50 1 "unbalanced bwidth with opposed, unbalanced delay"
exit $ret
--
2.26.2
Show replies by date
On Wed, 9 Dec 2020, Paolo Abeni wrote:
When multiple subflows are active, we can receive a
window update on subflow with no write space available.
MPTCP will try to push frames on such subflow and will
fail. Pending frames will be pushed only after receiving
a window update on a subflow with some wspace available.
Overall the above could lead to suboptimal aggregate
bandwidth usage.
Instead, we should try to push pending frames as soon as
the subflow reaches both conditions mentioned above.
We can finally enable self-tests with asymmetric links,
as the above makes them finally pass.
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
---
net/mptcp/options.c | 13 ++++++++-----
net/mptcp/protocol.c | 2 +-
net/mptcp/protocol.h | 2 +-
tools/testing/selftests/net/mptcp/simult_flows.sh | 6 +++---
4 files changed, 13 insertions(+), 10 deletions(-)
Hi Paolo -
Looks good!
Reviewed-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
--
Mat Martineau
Intel
Hi Paolo, Mat,
On 09/12/2020 18:46, Paolo Abeni wrote:
When multiple subflows are active, we can receive a
window update on subflow with no write space available.
MPTCP will try to push frames on such subflow and will
fail. Pending frames will be pushed only after receiving
a window update on a subflow with some wspace available.
Overall the above could lead to suboptimal aggregate
bandwidth usage.
Instead, we should try to push pending frames as soon as
the subflow reaches both conditions mentioned above.
We can finally enable self-tests with asymmetric links,
as the above makes them finally pass.
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
Thank you for the patch and the review!
Just applied in our tree:
- bbaa11040198: mptcp: push pending frames when subflow has free space
- Results: 3f7872297b23..ffc63be6523f
Tests + export are in progress!
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net