diff mbox series

Re: [PATCH] mptcp: cope with later TCP fallback.

Message ID ff7ef19f-5e1d-20d0-dd63-0a0c0c2ce3d2@tessares.net
State Accepted, archived
Delegated to: Paolo Abeni
Headers show
Series Re: [PATCH] mptcp: cope with later TCP fallback. | expand

Commit Message

Matthieu Baerts Dec. 13, 2019, 6:53 p.m. UTC 
Hi Paolo, Florian, Mat,

On 13/12/2019 17:31, Paolo Abeni wrote:
> With V1 passive connections can fallback TCP after that the
> subflow become established:
> 
> syn+ MP_CAPABLE ->
>                 <- syn, ack + MP_CAPABLE
> 
> ack, seq = 3    ->
>          // OoO packet is accepted because in-sequence
>          // passive socket is created, is in ESTABLISHED
> 	// status and tentatively as MP_CAPABLE
> 
> ack, seq = 2     ->
>          // no MP_CAPABLE opt, subflow should fallback to TCP
> 
> We can't use the 'subflow' socket fallback, as we don't have
> it available for passive connection.
> 
> Instead, when the fallback is detected, replace the mptcp
> socket with the underlining TCP subflow. Beyond covering
> the above scenario, it makes TCP fallback socket as efficient
> as plain TCP ones.
> 
> Co-developed-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> RFC -> v1:
>   - fixed WARN_ON splat on fallback
>   - added a few more comments about locking
>   - tested on both of v1 patches and top of current export
>     branch
> 
> Note: should be applied just after "mptcp: process MP_CAPABLE data option."

I just applied this patch just after "mptcp: process MP_CAPABLE data 
option". Please review at least the resolution of the second conflict:

=====
+++ b/net/mptcp/protocol.c
@@@ -202,11 -189,8 +249,16 @@@ static int mptcp_sendmsg_frag(struct so
                 ret = sk_stream_wait_memory(ssk, timeo);
                 if (ret)
                         return ret;
++<<<<<<< t/mptcp-queue-data-for-mptcp-level-retransmission
  +
  +              /* id sk_stream_wait_memory() sleeps snd_una can change
  +               * significantly, refresh the rtx queue
  +               */
  +              mptcp_clean_una(sk);
++=======
+               if (unlikely(__mptcp_needs_tcp_fallback(msk)))
+                       return 0;
++>>>>>>> top-bases/t/mptcp-queue-data-for-mptcp-level-retransmission
         }

         /* compute copy limit */
=====


- be009e4990f8: conflict in t/mptcp-Add-path-manager-interface
- 6aa8c2826aa9: conflict in 
t/mptcp-queue-data-for-mptcp-level-retransmission
- 1afa4a73e9a9: conflict in t/mptcp-introduce-MPTCP-retransmission-timer
- 61ff4269f0bb: conflict in 
t/mptcp-implement-memory-accounting-for-mptcp-rtx-queue
- d63f402c1c53: conflict in 
t/subflow-place-further-subflows-on-new-join_list
- bc663afcd33e..956fb00125d4: result

Tests + export are in progress!

Cheers,
Matt

Comments

Paolo Abeni Dec. 16, 2019, 6:40 p.m. UTC | #1 
On Fri, 2019-12-13 at 19:53 +0100, Matthieu Baerts wrote:
> Hi Paolo, Florian, Mat,
> 
> On 13/12/2019 17:31, Paolo Abeni wrote:
> > With V1 passive connections can fallback TCP after that the
> > subflow become established:
> > 
> > syn+ MP_CAPABLE ->
> >                 <- syn, ack + MP_CAPABLE
> > 
> > ack, seq = 3    ->
> >          // OoO packet is accepted because in-sequence
> >          // passive socket is created, is in ESTABLISHED
> > 	// status and tentatively as MP_CAPABLE
> > 
> > ack, seq = 2     ->
> >          // no MP_CAPABLE opt, subflow should fallback to TCP
> > 
> > We can't use the 'subflow' socket fallback, as we don't have
> > it available for passive connection.
> > 
> > Instead, when the fallback is detected, replace the mptcp
> > socket with the underlining TCP subflow. Beyond covering
> > the above scenario, it makes TCP fallback socket as efficient
> > as plain TCP ones.
> > 
> > Co-developed-by: Florian Westphal <fw@strlen.de>
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> > RFC -> v1:
> >   - fixed WARN_ON splat on fallback
> >   - added a few more comments about locking
> >   - tested on both of v1 patches and top of current export
> >     branch
> > 
> > Note: should be applied just after "mptcp: process MP_CAPABLE data option."
> 
> I just applied this patch just after "mptcp: process MP_CAPABLE data 
> option". Please review at least the resolution of the second conflict:
> 
> =====
> diff --cc net/mptcp/protocol.c
> index 9970487a855d,08d686c5f614..000000000000
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@@ -202,11 -189,8 +249,16 @@@ static int mptcp_sendmsg_frag(struct so
>                  ret = sk_stream_wait_memory(ssk, timeo);
>                  if (ret)
>                          return ret;
> ++<<<<<<< t/mptcp-queue-data-for-mptcp-level-retransmission
>   +
>   +              /* id sk_stream_wait_memory() sleeps snd_una can change
>   +               * significantly, refresh the rtx queue
>   +               */
>   +              mptcp_clean_una(sk);
> ++=======
> +               if (unlikely(__mptcp_needs_tcp_fallback(msk)))
> +                       return 0;
> ++>>>>>>> top-bases/t/mptcp-queue-data-for-mptcp-level-retransmission
>          }
> 
>          /* compute copy limit */
> =====
__mptcp_tcp_fallback
Conflicts resolution LGTM! 

Additionally I thought/fear we would need some additional cleanup in
__mptcp_tcp_fallback() in part 4, but since we call into
__mptcp_close() we should be good!

Thanks,

Paolo
diff mbox series

Patch

diff --cc net/mptcp/protocol.c
index 9970487a855d,08d686c5f614..000000000000
--- a/net/mptcp/protocol.c