On Thu, 23 Jun 2022, Geliang Tang wrote:
> This patch adds the redundant 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 | 98 ++++++++++++++++++++++++++------------------
> 1 file changed, 57 insertions(+), 41 deletions(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 6e44786e01fd..d09b01f1dc29 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1516,63 +1516,79 @@ 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,
> - };
> + struct mptcp_subflow_context *subflow;
> struct mptcp_data_frag *dfrag;
> - int len, copied = 0;
> + int len, max = 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;
> + if (!mptcp_sched_get_send(msk))
> + goto out;
>
> - prev_ssk = ssk;
> - ssk = mptcp_subflow_get_send(msk);
> + dfrag = mptcp_send_head(sk);
I think the looping structure in v7 was closer to what is needed.
Each subflow needs to be able to send from multiple dfrags if they are
able to.
> + if (!dfrag)
> + goto out;
>
> - /* 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;
> + mptcp_for_each_subflow(msk, subflow) {
> + if (READ_ONCE(subflow->scheduled)) {
> + struct sock *prev_ssk = NULL, *ssk = NULL;
> + struct mptcp_sendmsg_info info = {
> + .flags = flags,
> + };
> + int copied = 0;
>
> - /* 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);
There still needs to be a loop here (like in v7) that allows multiple
calls to mptcp_sendmsg_frag(). Each subflow *starts* sending at the
initial dfrag, but can send any amount of data from the available list of
dfrags. If msk->first_pending has not been updated until after all
subflows are done, that can be used as the starting point for each
subflow.
> + 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);
> - goto out;
> - }
> + prev_ssk = ssk;
> + ssk = mptcp_subflow_tcp_sock(subflow);
>
> - info.sent += ret;
> - copied += ret;
> - len -= ret;
> + /* 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;
> +
> + /* 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 = mptcp_sendmsg_frag(sk, ssk, dfrag, &info);
> + if (ret <= 0) {
> + mptcp_push_release(ssk, &info);
> + goto out;
> + }
> +
> + info.sent += ret;
> + copied += ret;
> + len -= ret;
> + }
> + max = max(copied, max);
Would be good to use a variable name other than "max" so it doesn't match
the macro name here.
>
> - mptcp_update_post_push(msk, dfrag, ret);
> + /* at this point we held the socket lock for the last subflow we used */
> + if (ssk) {
> + mptcp_push_release(ssk, &info);
> + msk->last_snd = ssk;
> + mptcp_subflow_set_scheduled(subflow, false);
> + }
> }
> - 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);
> + if (max)
> + mptcp_update_post_push(msk, dfrag, max);
> + WRITE_ONCE(msk->first_pending, mptcp_send_next(sk));
With the notes above on supporting multiple dfrags,
mptcp_update_post_push() also will need to be more complicated. It will
have to update all dfrags that were sent.
>
> out:
> /* ensure the rtx timer is running */
> if (!mptcp_timer_pending(sk))
> mptcp_reset_timer(sk);
> - if (copied)
> + if (max)
> __mptcp_check_send_data_fin(sk);
> }
>
> --
> 2.35.3
>
>
>
--
Mat Martineau
Intel