Re: [PATCH RFC mptcp-net 7/7] mptcp: pm: nl: announce deny-join-id0 attribute

[Matthieu Baerts <matttbe@kernel.org>]

Hi Mat,

Thank you for the review!

On 05/09/2025 02:54, Mat Martineau wrote:
> On Fri, 29 Aug 2025, Matthieu Baerts (NGI0) wrote:
> 
>> During the connection establishment, a peer can tell the other that it
>> cannot establish new subflows to the initial IP address and port by
>> setting the 'C' flag [1]. Doing so makes sense when the sender is behind
>> a strict NAT, operating behind a legacy Layer 4 load balancer, or using
>> anycast IP address for example.
>>
>> When this 'C' flag is set, the path-managers must then not try to
>> establish new subflow to the other peer's initial IP address and port.
>> The in-kernel PM has access to this info, but the userspace PM didn't.
>>
>> When a new connection is created and established, the Netlink events
>> will now contain a new deny-join-id0 attribute. When set to 1, it means
>> no other subflow to the initial IP address and port -- which is part of
>> the event -- can be established.
>>
>> Link: https://datatracker.ietf.org/doc/html/rfc8684#section-3.1-20.6 [1]
>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/532
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>> This patch is marked as an RFC, just so we can agree on the attribute
>> type: should we use a new dedicated attribute, or start using flags? It
>> even looks like we have defined a "flags" attribute, but it is currently
>> not used. Using "flags" would allow us to save some bytes, and in this
>> case, 'server-side' should have used "flags" too. The problem with the
>> flags is that the userspace cannot tell if a new flag is unset because
>> the kernel doesn't support it, or because it doesn't need to be set. But
>> is it really an issue? If the kernel doesn't support it, the userspace
>> will probably not know what to do anyway. So fine to use flags? Or not
>> needed, because it is unlikely to add new attributes later on?
>>
>> If yes, I suggest:
> 
> Hi Matthieu -
> 
> I do prefer 'flags' for this. It is the "C flag" after all :)
> 
>> - switching 'flags' to u32 instead of u16 because it doesn't change
>>  anything, and it is currently not used
> 
> There is already iproute2 code that references the 16-bit flags, even if
> it is just for 'ip mptcp monitor'. Better to not alter the existing
> userspace API here, especially in a -net patch.

Good point!

We can expend it later if needed.

>> - deprecating the 'server-side' attribute, and adding it in the flags
>>  (but keeping it for the moment, maybe we can remove it in a few
>>  versions?)
> 
> Good idea, can duplicate the information between the server-side
> attribute and flag for now. Then update mptcpd to prefer the flag
> version and remove from the kernel sometime after that.
> 
>> - adding deny-join-id0 as a flag: MPTCP_PM_EVENT_FLAG_DENY_JOIN_ID0
> 
> Sounds like a good plan!

OK, I will then do that, but also remove patch 2, and replace it by this
one (as a fix then).

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.