[MPTCP] Re: [PATCH mptcp-next 1/9] mptcp: add skeleton to sync msk socket options to subflows

[Paolo Abeni <pabeni@redhat.com>]

Hello,

First thing first, thank you for this great effort!

On Wed, 2021-03-17 at 17:38 +0100, Florian Westphal wrote:
> Handle following cases:
> 1. setsockopt is called with multiple subflows.
>    Change might have to be mirrored to all of them.
> 2. Outgoing subflow is created after one or several setsockopt()
>    calls have been made.  Old setsockopt changes should be
>    synced to the new socket.
> 3. Like 2, but for incoming subflow.
>    This needs the work queue because socket lock (and in the future
>    possibly rtnl mutex) might be needed.
> 
> Add sequence numbers to subflow context and mptcp socket so
> synchronization functions know which subflow is already updated
> and which ones are not.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  net/mptcp/protocol.c | 31 ++++++++++++++++++++++++++++---
>  net/mptcp/protocol.h | 12 ++++++++++++
>  net/mptcp/sockopt.c  | 27 +++++++++++++++++++++++++++
>  net/mptcp/subflow.c  |  1 +
>  4 files changed, 68 insertions(+), 3 deletions(-)
> 
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 9d7e7e13fba8..0b9ef8ddff55 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -730,18 +730,42 @@ void mptcp_data_ready(struct sock *sk, struct sock *ssk)
>  		sk->sk_data_ready(sk);
>  }
>  
> -void __mptcp_flush_join_list(struct mptcp_sock *msk)
> +static bool mptcp_do_flush_join_list(struct mptcp_sock *msk)
>  {
>  	struct mptcp_subflow_context *subflow;
>  
>  	if (likely(list_empty(&msk->join_list)))
> -		return;
> +		return false;
>  
>  	spin_lock_bh(&msk->join_list_lock);
>  	list_for_each_entry(subflow, &msk->join_list, node)
>  		mptcp_propagate_sndbuf((struct sock *)msk, mptcp_subflow_tcp_sock(subflow));
>  	list_splice_tail_init(&msk->join_list, &msk->conn_list);
>  	spin_unlock_bh(&msk->join_list_lock);
> +
> +	return true;
> +}
> +
> +static void mptcp_work_flush_join_list(struct mptcp_sock *msk)
> +{
> +	bool sync_needed = test_and_clear_bit(MPTCP_WORK_SYNC_SETSOCKOPT, &msk->flags);
> +
> +	if (!mptcp_do_flush_join_list(msk) && !sync_needed)
> +		return;
> +
> +	mptcp_sockopt_sync_all(msk);
> +}
> +
> +void __mptcp_flush_join_list(struct mptcp_sock *msk)

There are a few __mptcp_flush_join_list() call-sites which are already
in process context, e.g.:

mptcp_stream_accept()
__mptcp_push_pending()
__mptcp_move_skbs()
mptcp_disconnect()

what renaming mptcp_work_flush_join_list() in mptcp_flush_join_list() -
or mptcp_work_flush_join_list_lock() or some better name - and use it
in the above places?

> @@ -2569,6 +2593,7 @@ static void __mptcp_destroy_sock(struct sock *sk)
>  	xfrm_sk_free_policy(sk);
>  	sk_refcnt_debug_release(sk);
>  	mptcp_dispose_initial_subflow(msk);
> +

I guess the added empty line is not intentional? ;)

Thanks!

Paolo
_______________________________________________
mptcp mailing list -- mptcp@lists.01.org
To unsubscribe send an email to mptcp-leave@lists.01.org