Re: [RFC PATCH mptcp-next v12 1/7] mptcp: introduce MSG_FASTOPEN flag.

[Paolo Abeni <pabeni@redhat.com>]

On Mon, 2022-10-03 at 10:02 +0200, Paolo Abeni wrote:
> Hello,
> 
> On Sat, 2022-10-01 at 05:08 +0200, Dmytro Shytyi wrote:
> > I'm getting the next stack trace [1] when following this approach, yet 
> > it needs uarg to be filled, not NULL.
> > 
> > After Matthieu suggestion, I tried to implement the .connect in mptcp, 
> > but I'm getting errors like "ENOBUFF" or "EINPROGRESS".
> > 
> > Finally I decided to continue with "mptcp_stream_connect()" function in v13.
> 
> The reported error is not clear at all to me. It's better to clarify it
> before moving to the next version, or steps could (likelly, will) be in
> the wrong direction.
> 
> > [1] Code starting with the faulting instruction
> > ===========================================
> > [   27.736069] RSP: 0018:ffffc90000adfce0 EFLAGS: 00010246
> > [   27.737115] RAX: 0000000000000000 RBX: ffff888003894440 RCX: 
> > 0000000000000000
> > [   27.738560] RDX: 0000000000000010 RSI: ffffc90000adfe80 RDI: 
> > ffff888027ce8000
> > [   27.739883] RBP: 0000000000000000 R08: 0000000000000001 R09: 
> > ffff888015710cf0
> > [   27.741286] R10: 0000000000000001 R11: ffff888003ca78c8 R12: 
> > 00000000ffffff96
> > [   27.742718] R13: ffffc90000adfdb0 R14: ffffc90000adfe80 R15: 
> > ffff888027ce8000
> > [   27.744135] FS:  00007f7cc983a740(0000) GS:ffff88803da00000(0000) 
> > knlGS:0000000000000000
> > [   27.745612] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [   27.746788] CR2: ffffffffffffffd6 CR3: 000000001d180006 CR4: 
> > 0000000000370ef0
> > [   27.748125] Call Trace:
> > [   27.748655]  <TASK>
> > [   27.749086] __inet_stream_connect (net/ipv4/af_inet.c:663)
> > [   27.750093] ? sk_reset_timer (net/core/sock.c:3341)
> > [   27.750803] ? tcp_connect (net/ipv4/tcp_output.c:3882)
> > [   27.751590] ? kmem_cache_alloc_trace (mm/slub.c:3286)
> > [   27.752492] tcp_sendmsg_fastopen (net/ipv4/tcp.c:1198)
> > [   27.753347] mptcp_sendmsg (net/mptcp/protocol.c:1710)
> > [   27.754047] sock_sendmsg_nosec (net/socket.c:714)
> > [   27.754831] __sys_sendto (net/socket.c:2117)
> > [   27.755539] ? handle_mm_fault (mm/memory.c:5151)
> > [   27.756311] ? do_user_addr_fault (arch/x86/mm/fault.c:1426)
> > [   27.757175] __x64_sys_sendto (net/socket.c:2129 net/socket.c:2125 
> > net/socket.c:2125)
> > [   27.758029] do_syscall_64 (arch/x86/entry/common.c:50 
> > arch/x86/entry/common.c:80)
> > [   27.758739] entry_SYSCALL_64_after_hwframe 
> > (arch/x86/entry/entry_64.S:120)
> > [   27.759719] RIP: 0033:0x7f7cc99484e6
> > [ 27.760458] Code: 69 0e 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 
> > 1f 00 41 89 ca 64 8b 04 25 18 00 00 00 85 c0c
> 
> This backtrace is incomplete/lacks the oopsing address, so is unclear
> what really went wrong. Please report the full backtrace.
> 
> Important: please include the code you used for this test. No need to
> post the full old patches here, you can e.g. share an URL to your git
> tree/branch.
> 
> The main point is that this backtrace does not look compatible with the
> code suggested previously.

I discussed this with Mat(ttbe) on IRC. It looks like the main problem
is that mptcp don't implement the struct proto/sk->sk_prot->connect,
and the defer_connect is not triggering such path as subflow the socket
is not in SS_UNCONNECTED state on defer connect.

The correct solution would be re-factor the mptcp connect code/hooking
to re-use the inet_stream_connect() infrastructure. I'll try to cook
the relevant patch - not strictily fastclose related - asap, so that
you can rebase this series on top of them. 

Cheers,

Paolo