From: Mat Martineau <mathew.j.martineau@linux.intel.com>
To: Geliang Tang <geliang.tang@suse.com>
Cc: mptcp@lists.linux.dev
Subject: Re: [PATCH mptcp-next v18 10/15] mptcp: use get_send wrapper
Date: Thu, 10 Nov 2022 17:04:35 -0800 (PST) [thread overview]
Message-ID: <d0e481ed-3ce1-7eaf-b6bd-1cc4e00f05eb@linux.intel.com> (raw)
In-Reply-To: <262f6ec0dfa2ffba15899460df4676dc15d1bd62.1667897099.git.geliang.tang@suse.com>
On Tue, 8 Nov 2022, Geliang Tang wrote:
> This patch defines the packet scheduler wrapper mptcp_sched_get_send(),
> invoke data_init() and get_subflow() of msk->sched in it.
>
> Set data->reinject to false in mptcp_sched_get_send(). If msk->sched is
> NULL, use default functions mptcp_subflow_get_send() to send data.
>
> Move sock_owned_by_me() check and fallback check into the wrapper from
> mptcp_subflow_get_send().
>
> Add the multiple subflows support for __mptcp_push_pending() and
> __mptcp_subflow_push_pending(). Use get_send() wrapper instead of
> mptcp_subflow_get_send() in them.
>
> Check the subflow scheduled flags to test which subflow or subflows are
> picked by the scheduler, use them to send data.
>
> This commit allows the scheduler to set the subflow->scheduled bit in
> multiple subflows, but it does not allow for sending redundant data.
> Multiple scheduled subflows will send sequential data on each subflow.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> net/mptcp/protocol.c | 131 ++++++++++++++++++++++++++++---------------
> net/mptcp/protocol.h | 2 +
> net/mptcp/sched.c | 37 ++++++++++++
> 3 files changed, 124 insertions(+), 46 deletions(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index d7aaa49c64f4..5bcadb36b99b 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1406,7 +1406,7 @@ bool mptcp_subflow_active(struct mptcp_subflow_context *subflow)
> * returns the subflow that will transmit the next DSS
> * additionally updates the rtx timeout
> */
> -static struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
> +struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
> {
> struct subflow_send_info send_info[SSK_MODE_MAX];
> struct mptcp_subflow_context *subflow;
> @@ -1417,15 +1417,6 @@ static struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
> u64 linger_time;
> long tout = 0;
>
> - sock_owned_by_me(sk);
> -
> - 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;
> - }
> -
> /* pick the subflow with the lower wmem/wspace ratio */
> for (i = 0; i < SSK_MODE_MAX; ++i) {
> send_info[i].ssk = NULL;
> @@ -1577,42 +1568,58 @@ void __mptcp_push_pending(struct sock *sk, unsigned int flags)
> };
> bool do_check_data_fin = false;
>
> +again:
> while (mptcp_send_head(sk)) {
> + struct mptcp_subflow_context *subflow, *last = NULL;
> 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)
> + if (mptcp_sched_get_send(msk))
> goto out;
>
> - /* 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);
> + mptcp_for_each_subflow(msk, subflow) {
> + if (READ_ONCE(subflow->scheduled))
> + last = subflow;
> + }
Since mptcp_sched_get_send() is always called right before this, the
subflow->scheduled flags will always be set. Does the new code with 'last'
work as expected if the mptcp_sched_get_send() call is skipped when an
existing subflow->scheduled flag is found? That way the old flags will be
used the first time through this loop.
- Mat
>
> - ret = __subflow_push_pending(sk, ssk, &info);
> - if (ret <= 0) {
> - if (ret == -EAGAIN)
> - continue;
> - mptcp_push_release(ssk, &info);
> - goto out;
> + mptcp_for_each_subflow(msk, subflow) {
> + if (READ_ONCE(subflow->scheduled)) {
> + prev_ssk = ssk;
> + ssk = mptcp_subflow_tcp_sock(subflow);
> +
> + /* 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);
> +
> + /* 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);
> +
> + ret = __subflow_push_pending(sk, ssk, &info);
> + if (ret <= 0) {
> + if (ret == -EAGAIN &&
> + inet_sk_state_load(ssk) != TCP_CLOSE)
> + goto again;
> + if (last && subflow != last)
> + continue;
> + goto out;
> + }
> + do_check_data_fin = true;
> + msk->last_snd = ssk;
> + mptcp_subflow_set_scheduled(subflow, false);
> + }
> }
> - do_check_data_fin = true;
> }
>
> +out:
> /* 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 */
> if (!mptcp_timer_pending(sk))
> mptcp_reset_timer(sk);
> @@ -1626,29 +1633,61 @@ static void __mptcp_subflow_push_pending(struct sock *sk, struct sock *ssk, bool
> struct mptcp_sendmsg_info info = {
> .data_lock_held = true,
> };
> - struct sock *xmit_ssk;
> int ret = 0;
>
> info.flags = 0;
> +again:
> while (mptcp_send_head(sk)) {
> + struct mptcp_subflow_context *subflow, *last = NULL;
> +
> /* 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);
> + first = false;
> + if (ret <= 0) {
> + if (ret == -EAGAIN &&
> + inet_sk_state_load(ssk) != TCP_CLOSE)
> + goto again;
> + break;
> + }
> + msk->last_snd = ssk;
> + 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);
> - first = false;
> - if (ret <= 0) {
> - if (ret == -EAGAIN)
> - continue;
> - 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);
> + msk->last_snd = ssk;
> + mptcp_subflow_set_scheduled(subflow, false);
> + goto out;
> + }
> +
> + ret = __subflow_push_pending(sk, ssk, &info);
> + if (ret <= 0) {
> + if (ret == -EAGAIN &&
> + inet_sk_state_load(ssk) != TCP_CLOSE)
> + goto again;
> + if (last && subflow != last)
> + continue;
> + goto out;
> + }
> + msk->last_snd = ssk;
> + mptcp_subflow_set_scheduled(subflow, false);
> + }
> }
> }
>
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index e93d64217896..2bc0acf2d659 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -640,6 +640,8 @@ int mptcp_init_sched(struct mptcp_sock *msk,
> void mptcp_release_sched(struct mptcp_sock *msk);
> void mptcp_subflow_set_scheduled(struct mptcp_subflow_context *subflow,
> bool scheduled);
> +struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk);
> +int mptcp_sched_get_send(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 0d7c73e9562e..bc5d82300863 100644
> --- a/net/mptcp/sched.c
> +++ b/net/mptcp/sched.c
> @@ -112,3 +112,40 @@ void mptcp_sched_data_set_contexts(const struct mptcp_sock *msk,
> for (; i < MPTCP_SUBFLOWS_MAX; i++)
> data->contexts[i] = NULL;
> }
> +
> +int mptcp_sched_get_send(struct mptcp_sock *msk)
> +{
> + struct mptcp_subflow_context *subflow;
> + struct mptcp_sched_data data;
> + struct sock *ssk = NULL;
> +
> + sock_owned_by_me((const struct sock *)msk);
> +
> + mptcp_for_each_subflow(msk, subflow) {
> + if (READ_ONCE(subflow->scheduled))
> + return 0;
> + }
> +
> + /* the following check is moved out of mptcp_subflow_get_send */
> + if (__mptcp_check_fallback(msk)) {
> + 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) {
> + ssk = mptcp_subflow_get_send(msk);
> + if (!ssk)
> + return -EINVAL;
> + mptcp_subflow_set_scheduled(mptcp_subflow_ctx(ssk), true);
> + return 0;
> + }
> +
> + data.reinject = false;
> + msk->sched->data_init(msk, &data);
> + return msk->sched->get_subflow(msk, &data);
> +}
> --
> 2.35.3
>
>
>
--
Mat Martineau
Intel
next prev parent reply other threads:[~2022-11-11 1:04 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-08 9:08 [PATCH mptcp-next v18 00/15] BPF redundant scheduler Geliang Tang
2022-11-08 9:08 ` [PATCH mptcp-next v18 01/15] mptcp: refactor push_pending logic Geliang Tang
2022-11-11 0:11 ` Mat Martineau
2022-11-08 9:08 ` [PATCH mptcp-next v18 02/15] mptcp: drop last_snd and MPTCP_RESET_SCHEDULER Geliang Tang
2022-11-08 9:08 ` [PATCH mptcp-next v18 03/15] mptcp: add sched_data_set_contexts helper Geliang Tang
2022-11-08 9:08 ` [PATCH mptcp-next v18 04/15] Squash to "mptcp: add struct mptcp_sched_ops" Geliang Tang
2022-11-08 9:08 ` [PATCH mptcp-next v18 05/15] Squash to "bpf: Add bpf_mptcp_sched_ops" Geliang Tang
2022-11-08 9:08 ` [PATCH mptcp-next v18 06/15] Squash to "bpf: Add bpf_mptcp_sched_kfunc_set" Geliang Tang
2022-11-08 9:08 ` [PATCH mptcp-next v18 07/15] Squash to "selftests/bpf: Add bpf_first scheduler" Geliang Tang
2022-11-08 9:08 ` [PATCH mptcp-next v18 08/15] Squash to "selftests/bpf: Add bpf_bkup scheduler" Geliang Tang
2022-11-08 9:08 ` [PATCH mptcp-next v18 09/15] Squash to "selftests/bpf: Add bpf_rr scheduler" Geliang Tang
2022-11-11 0:29 ` Mat Martineau
2022-11-08 9:08 ` [PATCH mptcp-next v18 10/15] mptcp: use get_send wrapper Geliang Tang
2022-11-11 1:04 ` Mat Martineau [this message]
2022-11-08 9:08 ` [PATCH mptcp-next v18 11/15] mptcp: use get_retrans wrapper Geliang Tang
2022-11-08 9:08 ` [PATCH mptcp-next v18 12/15] mptcp: delay updating first_pending Geliang Tang
2022-11-08 9:08 ` [PATCH mptcp-next v18 13/15] mptcp: delay updating already_sent Geliang Tang
2022-11-08 9:08 ` [PATCH mptcp-next v18 14/15] selftests/bpf: Add bpf_red scheduler Geliang Tang
2022-11-08 9:08 ` [PATCH mptcp-next v18 15/15] selftests/bpf: Add bpf_red test Geliang Tang
2022-11-08 9:34 ` selftests/bpf: Add bpf_red test: Build Failure MPTCP CI
2022-11-08 10:51 ` selftests/bpf: Add bpf_red test: Tests Results MPTCP CI
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d0e481ed-3ce1-7eaf-b6bd-1cc4e00f05eb@linux.intel.com \
--to=mathew.j.martineau@linux.intel.com \
--cc=geliang.tang@suse.com \
--cc=mptcp@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox