Re: [PATCH mptcp-next 3/4] mptcp: handle join requests via pernet listen socket

[Florian Westphal <fw@strlen.de>]

Mat Martineau <mathew.j.martineau@linux.intel.com> wrote:
> On Thu, 10 Feb 2022, Florian Westphal wrote:
> 
> > Currently mptcp adds kernel-based listener socket for all
> > netlink-configured mptcp address endpoints.
> > 
> > This has caveats because kernel may interfere with unrelated programs
> > that use same address/port pairs.
> > 
> 
> It looks like they still interfere with each other, but now in the opposite
> way: TCP listeners can now be created that interfere with MP_JOINs (and the
> MPTCP side loses).

Yep, I'm not sure its a good idea to announce random addresses:ports.

> Since mptcp_handle_join() is only called if the listener lookup fails, if a
> TCP listen socket has been created for an address & port advertised by
> MPTCP, that TCP listener will be looked up, process the SYN, and send a
> regular TCP SYN/ACK. The peer will then reject it due to lack of correct
> MPTCP options.

Correct.  Its easily fixable by doing mptcp_handle_join() before the
lookup, but that means a new conditional in tcp fastpath.

I'm not sure TCP maintainers will eat that, but its certainly doable.
Idea would be to have __mptcp_handle_join() make a listen socket lookup
and then assign 'skb->sk = magic_listen' if there is none, just like TPROXY.

> Seems like a few more TCP changes are needed to handle this listener
> collision well for both TCP and MPTCP, and without too much overhead. Is it
> too expensive to look for MPTCP options in every incoming TCP SYN header?

I'm not worried about that, Its just the extra
if (th->syn && !th->ack && ...
conditional inside mptcp_handle_join().

ATM the extra conditional is hit only in the error path, not for every
packet.