[PATCH mptcp-next 0/2] mptcp: add TCP_FASTOPEN_NO_COOKIE support

[PATCH mptcp-next 0/2] mptcp: add TCP_FASTOPEN_NO_COOKIE support

[Matthieu Baerts]

This series adds TCP_FASTOPEN_NO_COOKIE socket option support.

The first patch refactors a bit a function introduced by Benjamin to support
TCP_FASTOPEN_CONNECT: the function is now generic.

Adding the support of the new socket option is easy after because the only
action needed is to forward the request to the first subflow.

This has been validated using Packetdrill:

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

Matthieu Baerts (2):
  mptcp: sockopt: make 'tcp_fastopen_connect' generic
  mptcp: add TCP_FASTOPEN_NO_COOKIE support

 net/mptcp/sockopt.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)


base-commit: 095ffd740bdb5564bfe678e5cddf5d1d7c6b7462
-- 
2.37.2

[PATCH mptcp-next 1/2] mptcp: sockopt: make 'tcp_fastopen_connect' generic

[Matthieu Baerts]

There are other socket options that need to act only on the first
subflow, e.g. all TCP_FASTOPEN* socket options.

This is similar to the getsockopt version.

In the next commit, this new mptcp_setsockopt_first_sf_only() helper is
used by other another option.

Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 net/mptcp/sockopt.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index c7cb68c725b2..8d3b09d75c3a 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -769,17 +769,17 @@ static int mptcp_setsockopt_sol_tcp_defer(struct mptcp_sock *msk, sockptr_t optv
 	return tcp_setsockopt(listener->sk, SOL_TCP, TCP_DEFER_ACCEPT, optval, optlen);
 }
 
-static int mptcp_setsockopt_sol_tcp_fastopen_connect(struct mptcp_sock *msk, sockptr_t optval,
-						     unsigned int optlen)
+static int mptcp_setsockopt_first_sf_only(struct mptcp_sock *msk, int level, int optname,
+					  sockptr_t optval, unsigned int optlen)
 {
 	struct socket *sock;
 
-	/* Limit to first subflow */
+	/* Limit to first subflow, before the connection establishment */
 	sock = __mptcp_nmpc_socket(msk);
 	if (!sock)
 		return -EINVAL;
 
-	return tcp_setsockopt(sock->sk, SOL_TCP, TCP_FASTOPEN_CONNECT, optval, optlen);
+	return tcp_setsockopt(sock->sk, level, optname, optval, optlen);
 }
 
 static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
@@ -811,7 +811,8 @@ static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 	case TCP_DEFER_ACCEPT:
 		return mptcp_setsockopt_sol_tcp_defer(msk, optval, optlen);
 	case TCP_FASTOPEN_CONNECT:
-		return mptcp_setsockopt_sol_tcp_fastopen_connect(msk, optval, optlen);
+		return mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname,
+						      optval, optlen);
 	}
 
 	return -EOPNOTSUPP;
-- 
2.37.2

[PATCH mptcp-next 2/2] mptcp: add TCP_FASTOPEN_NO_COOKIE support

[Matthieu Baerts]

The goal of this socket option is to configure MPTCP + TFO without
cookie per socket.

It was already possible to enable TFO without a cookie per netns by
setting net.ipv4.tcp_fastopen sysctl knob to the right value. Per route
was also supported by setting 'fastopen_no_cookie' option. This patch
adds a per socket support like it is possible to do with TCP thanks to
TCP_FASTOPEN_NO_COOKIE socket option.

The only thing to do here is to relay the request to the first subflow
like it is already done for TCP_FASTOPEN_CONNECT.

Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 net/mptcp/sockopt.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 8d3b09d75c3a..1857281a0dd5 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -560,6 +560,7 @@ static bool mptcp_supported_sockopt(int level, int optname)
 		case TCP_TX_DELAY:
 		case TCP_INQ:
 		case TCP_FASTOPEN_CONNECT:
+		case TCP_FASTOPEN_NO_COOKIE:
 			return true;
 		}
 
@@ -568,8 +569,8 @@ static bool mptcp_supported_sockopt(int level, int optname)
 		/* TCP_REPAIR, TCP_REPAIR_QUEUE, TCP_QUEUE_SEQ, TCP_REPAIR_OPTIONS,
 		 * TCP_REPAIR_WINDOW are not supported, better avoid this mess
 		 */
-		/* TCP_FASTOPEN_KEY, TCP_FASTOPEN, TCP_FASTOPEN_NO_COOKIE,
-		 * are not supported fastopen is currently unsupported
+		/* TCP_FASTOPEN_KEY, TCP_FASTOPEN are not supported because
+		 * fastopen for the listener side is currently unsupported
 		 */
 	}
 	return false;
@@ -811,6 +812,7 @@ static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 	case TCP_DEFER_ACCEPT:
 		return mptcp_setsockopt_sol_tcp_defer(msk, optval, optlen);
 	case TCP_FASTOPEN_CONNECT:
+	case TCP_FASTOPEN_NO_COOKIE:
 		return mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname,
 						      optval, optlen);
 	}
@@ -1175,6 +1177,7 @@ static int mptcp_getsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 	case TCP_CC_INFO:
 	case TCP_DEFER_ACCEPT:
 	case TCP_FASTOPEN_CONNECT:
+	case TCP_FASTOPEN_NO_COOKIE:
 		return mptcp_getsockopt_first_sf_only(msk, SOL_TCP, optname,
 						      optval, optlen);
 	case TCP_INQ:
-- 
2.37.2

Re: mptcp: add TCP_FASTOPEN_NO_COOKIE support: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
  - Task: https://cirrus-ci.com/task/5116179571277824
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5116179571277824/summary/summary.txt

- KVM Validation: debug:
  - Unstable: 1 failed test(s): packetdrill_dss 🔴:
  - Task: https://cirrus-ci.com/task/6242079478120448
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6242079478120448/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/15b58c1eb2c8


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)

Re: [PATCH mptcp-next 1/2] mptcp: sockopt: make 'tcp_fastopen_connect' generic

[Paolo Abeni]

On Tue, 2022-09-27 at 17:03 +0200, Matthieu Baerts wrote:
> There are other socket options that need to act only on the first
> subflow, e.g. all TCP_FASTOPEN* socket options.
> 
> This is similar to the getsockopt version.
> 
> In the next commit, this new mptcp_setsockopt_first_sf_only() helper is
> used by other another option.
> 
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> ---
>  net/mptcp/sockopt.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index c7cb68c725b2..8d3b09d75c3a 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -769,17 +769,17 @@ static int mptcp_setsockopt_sol_tcp_defer(struct mptcp_sock *msk, sockptr_t optv
>  	return tcp_setsockopt(listener->sk, SOL_TCP, TCP_DEFER_ACCEPT, optval, optlen);
>  }
>  
> -static int mptcp_setsockopt_sol_tcp_fastopen_connect(struct mptcp_sock *msk, sockptr_t optval,
> -						     unsigned int optlen)
> +static int mptcp_setsockopt_first_sf_only(struct mptcp_sock *msk, int level, int optname,
> +					  sockptr_t optval, unsigned int optlen)
>  {
>  	struct socket *sock;
>  
> -	/* Limit to first subflow */
> +	/* Limit to first subflow, before the connection establishment */
>  	sock = __mptcp_nmpc_socket(msk);
>  	if (!sock)
>  		return -EINVAL;

Crazy idea: if we add another int argument e.g. 'default_reval', we can
use this helper even for mptcp_setsockopt_sol_tcp_defer()
(default_retval == 0, while here default_reval == -EINVAL) ;)

Not a big deal, I'm fine even this way.

Thanks!

Paolo

Re: [PATCH mptcp-next 1/2] mptcp: sockopt: make 'tcp_fastopen_connect' generic

[Mat Martineau]

On Tue, 27 Sep 2022, Paolo Abeni wrote:

> On Tue, 2022-09-27 at 17:03 +0200, Matthieu Baerts wrote:
>> There are other socket options that need to act only on the first
>> subflow, e.g. all TCP_FASTOPEN* socket options.
>>
>> This is similar to the getsockopt version.
>>
>> In the next commit, this new mptcp_setsockopt_first_sf_only() helper is
>> used by other another option.
>>
>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>> ---
>>  net/mptcp/sockopt.c | 11 ++++++-----
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
>> index c7cb68c725b2..8d3b09d75c3a 100644
>> --- a/net/mptcp/sockopt.c
>> +++ b/net/mptcp/sockopt.c
>> @@ -769,17 +769,17 @@ static int mptcp_setsockopt_sol_tcp_defer(struct mptcp_sock *msk, sockptr_t optv
>>  	return tcp_setsockopt(listener->sk, SOL_TCP, TCP_DEFER_ACCEPT, optval, optlen);
>>  }
>>
>> -static int mptcp_setsockopt_sol_tcp_fastopen_connect(struct mptcp_sock *msk, sockptr_t optval,
>> -						     unsigned int optlen)
>> +static int mptcp_setsockopt_first_sf_only(struct mptcp_sock *msk, int level, int optname,
>> +					  sockptr_t optval, unsigned int optlen)
>>  {
>>  	struct socket *sock;
>>
>> -	/* Limit to first subflow */
>> +	/* Limit to first subflow, before the connection establishment */
>>  	sock = __mptcp_nmpc_socket(msk);
>>  	if (!sock)
>>  		return -EINVAL;
>
> Crazy idea: if we add another int argument e.g. 'default_reval', we can
> use this helper even for mptcp_setsockopt_sol_tcp_defer()
> (default_retval == 0, while here default_reval == -EINVAL) ;)
>
> Not a big deal, I'm fine even this way.

Or do something similar without adding the extra int argument: just have 
the call site for TCP_DEFER_ACCEPT in mptcp_setsockopt_sol_tcp() ignore 
the return value of mptcp_setsockopt_first_sf_only(), and remove 
mptcp_setsockopt_sol_tcp_defer()?

That would be a third patch to use the new helper, so if you (Matthieu) 
want to make that change I think these two patches are good as-is:

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

--
Mat Martineau
Intel

Re: mptcp: add TCP_FASTOPEN_NO_COOKIE support: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
  - Task: https://cirrus-ci.com/task/4851555361882112
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4851555361882112/summary/summary.txt

- KVM Validation: debug:
  - Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
  - Task: https://cirrus-ci.com/task/5977455268724736
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5977455268724736/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/91a8d06ab5a1


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)

Re: [PATCH mptcp-next 1/2] mptcp: sockopt: make 'tcp_fastopen_connect' generic

[Matthieu Baerts]

Hi Mat, Paolo,

Thank you both for the reviews!

On 27/09/2022 21:59, Mat Martineau wrote:
> On Tue, 27 Sep 2022, Paolo Abeni wrote:
> 
>> On Tue, 2022-09-27 at 17:03 +0200, Matthieu Baerts wrote:
>>> There are other socket options that need to act only on the first
>>> subflow, e.g. all TCP_FASTOPEN* socket options.
>>>
>>> This is similar to the getsockopt version.
>>>
>>> In the next commit, this new mptcp_setsockopt_first_sf_only() helper is
>>> used by other another option.
>>>
>>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>>> ---
>>>  net/mptcp/sockopt.c | 11 ++++++-----
>>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
>>> index c7cb68c725b2..8d3b09d75c3a 100644
>>> --- a/net/mptcp/sockopt.c
>>> +++ b/net/mptcp/sockopt.c
>>> @@ -769,17 +769,17 @@ static int
>>> mptcp_setsockopt_sol_tcp_defer(struct mptcp_sock *msk, sockptr_t optv
>>>      return tcp_setsockopt(listener->sk, SOL_TCP, TCP_DEFER_ACCEPT,
>>> optval, optlen);
>>>  }
>>>
>>> -static int mptcp_setsockopt_sol_tcp_fastopen_connect(struct
>>> mptcp_sock *msk, sockptr_t optval,
>>> -                             unsigned int optlen)
>>> +static int mptcp_setsockopt_first_sf_only(struct mptcp_sock *msk,
>>> int level, int optname,
>>> +                      sockptr_t optval, unsigned int optlen)
>>>  {
>>>      struct socket *sock;
>>>
>>> -    /* Limit to first subflow */
>>> +    /* Limit to first subflow, before the connection establishment */
>>>      sock = __mptcp_nmpc_socket(msk);
>>>      if (!sock)
>>>          return -EINVAL;
>>
>> Crazy idea: if we add another int argument e.g. 'default_reval', we can
>> use this helper even for mptcp_setsockopt_sol_tcp_defer()
>> (default_retval == 0, while here default_reval == -EINVAL) ;)
>>
>> Not a big deal, I'm fine even this way.
> 
> Or do something similar without adding the extra int argument: just have
> the call site for TCP_DEFER_ACCEPT in mptcp_setsockopt_sol_tcp() ignore
> the return value of mptcp_setsockopt_first_sf_only(), and remove
> mptcp_setsockopt_sol_tcp_defer()?
> 
> That would be a third patch to use the new helper, so if you (Matthieu)
> want to make that change I think these two patches are good as-is:

I thought about supporting the tcp_defer case but I didn't think about
just ignoring the error, good idea!

I can apply the two patches and send an extra one

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

Re: [PATCH mptcp-next 0/2] mptcp: add TCP_FASTOPEN_NO_COOKIE support

[Matthieu Baerts]

Hi Mat, Paolo,

On 27/09/2022 17:03, Matthieu Baerts wrote:
> This series adds TCP_FASTOPEN_NO_COOKIE socket option support.
> 
> The first patch refactors a bit a function introduced by Benjamin to support
> TCP_FASTOPEN_CONNECT: the function is now generic.
> 
> Adding the support of the new socket option is easy after because the only
> action needed is to forward the request to the first subflow.
> 
> This has been validated using Packetdrill:
> 
>   https://github.com/multipath-tcp/packetdrill/pull/91

Thank you for your reviews!

Now in our tree (feat. for net-next) with Mat's RvB (I didn't add
Paolo's ACK yet but please tell me if I can/should):

New patches for t/upstream:
- ce72cdadf047: mptcp: sockopt: make 'tcp_fastopen_connect' generic
- 5d745bd13d21: mptcp: add TCP_FASTOPEN_NO_COOKIE support
- Results: 5391f0d311f6..6711fbb5e6c8 (export)


Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220928T081959


Cheers,
Matt

> 
> Matthieu Baerts (2):
>   mptcp: sockopt: make 'tcp_fastopen_connect' generic
>   mptcp: add TCP_FASTOPEN_NO_COOKIE support
> 
>  net/mptcp/sockopt.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> 
> base-commit: 095ffd740bdb5564bfe678e5cddf5d1d7c6b7462

-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net