Re: [PATCH mptcp-next v3 7/8] mptcp: attempt to add listening sockets for announced addrs

[Matthieu Baerts <matthieu.baerts@tessares.net>]

Hi Paolo, Kishen,

On 01/02/2022 12:42, Paolo Abeni wrote:
> On Thu, 2022-01-27 at 19:38 -0500, Kishen Maloor wrote:
>> When ADD_ADDR announcements use the port associated with an
>> active subflow, this change ensures that a listening socket is bound
>> to the announced addr+port in the kernel for subsequently receiving
>> MP_JOINs. But if a listening socket for this address is already held
>> by the application then no action is taken.
>>
>> A listening socket is created (when there isn't a listener)
>> just prior to the addr advertisement. If it is desired to not create
>> a listening socket in the kernel for an address, then this can be
>> requested by including the MPTCP_PM_ADDR_FLAG_NO_LISTEN flag
>> with the address.
>>
>> When a listening socket is created, it is stored in
>> struct mptcp_pm_add_entry and released accordingly.
>>
>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/203
>> Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
>> ---
>> v2: fixed formatting
>> v3: added new addr flag MPTCP_PM_ADDR_FLAG_NO_LISTEN to skip creating a
>> listening socket in the kernel during an ADD_ADDR request, use this flag
>> along the in-kernel PM flow for ADD_ADDR requests (Note: listening sockets
>> are always created for port-based endpoints as before), use the
>> lsk_list_find_or_create() helper
> 
> I think it's better introducing the opposite flag (e.g.
> 'MPTCP_PM_ADDR_FLAG_LISTEN') otherwise this will change the default
> behavior

Maybe it is fine to change the behaviour. Without changing the user
exposed API of course.

I mean: it all depends if we consider the fact that when the userspace
closes the listening socket to accept new "MPTCP" connections, it is not
normal (bug) to close the possibility to create new subflows → socket
controlled by the user vs socket controlled by the PM. If we do consider
this as a "bug", then that's OK to change the default behaviour, no?

I don't know if other people are sharing my view here.

For me, if an app closes the listening socket after having accepted a
new connection, it is just not to receive new "main" connections on this
socket but it is OK to accept new subflows as they are part of existing
connections (and managed by the PM).
We would then avoid people hitting issues like #203. If you hit this
issue, it is not easy to find the answer I think.

If people know they don't need/want the creation of a listening socket
only to accept new subflows, they can set the NO_LISTEN flag when adding
an address with "ip mptcp". But if the cost is minimal most of the time
because no additional listening sockets will be actually created, that's
fine to use a "NO_LISTEN" flag I think.

WDYT?

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net