Re: [PATCH mptcp-next v2 00/10] selftests: cover more MPTCP socket options

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

Thank you for looking at that!

On 21/08/2025 10:54, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> v2:
>  - IP_FREEBIND/IPV6_FREEBIND are set in do_getsockopt_transparent, fix
>    it.
>  - Use "for" loops in mptcp_sockopt.sh.
>  - Move "add IP_TOS socket option test" out of this set.
>  - some cleanups.
> 
> This series significantly expands MPTCP socket option test coverage
> by adding validation for 10 more socket options:
> 
> 1. SO_REUSEADDR/SO_REUSEPORT - Test socket reuse capabilities
> 2. SO_BINDTODEVICE/SO_BINDTOIFINDEX - Validate interface binding
> 3. IP_TOS - Verify Type of Service handling
> 4. IP_FREEBIND - Test free address binding
> 5. IP_TRANSPARENT - Validate transparent proxying
> 6. IP_BIND_ADDRESS_NO_PORT - Test deferred port binding
> 7. IP_LOCAL_PORT_RANGE - Check local port range restriction
> 8. IPV6_V6ONLY - Verify IPv6-only sockets
> 
> Each commit systematically adds:
> - Setsockopt/getsockopt operations in mptcp_sockopt.c
> - Dedicated test cases in mptcp_sockopt.sh
> - IPv4 and IPv6 coverage
> - Error handling and edge case validation
> 
> The new tests verify that MPTCP correctly propagates socket options to
> subflows and maintains consistent behavior across address families.

I don't know if the last sentence is correct: if I'm not mistaken, it
looks like the new code "only" set and get some socket options, no?

If yes, that's not enough to me to verify a socket option is doing the
expected job. I think it might even be better not to include such code
giving a false sentiment of "it's fully tested" (+ more code to
maintain). All these options are supposed to affect the behaviour
somehow: a test should then mainly validate that, and check this
behaviour is similar to TCP.

In other words, such tests should be similar to what you did with
TCP_MAXSEG in Packetdrill:

  https://github.com/multipath-tcp/packetdrill/pull/161

I think that when validating a new socket options, the following should
be checked:

 - Comparison with TCP: the behaviour should be as closed as possible
 - set/get before and after the connection
 - behaviour on the client and server side
 - important: checking the new expected behaviour

Checking all this should probably be easier with Packetdrill, especially
to validate a behavioural change. But it might not be always possible,
or best suited, like with IP_BIND_ADDRESS_NO_PORT and
IP_LOCAL_PORT_RANGE which are "properly" tested in:

  tools/testing/selftests/net/mptcp/ip_local_port_range.c

What do you think?

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