TCP_KEEPIDLE, TCP_KEEPINTVL, TCP_KEEPCNT are not implemented in mptcp_setsockopt_sol_tcp #383

zonyitoo · 2023-04-05T15:42:47Z

These options are claimed "supported" in mptcp_supported_sockopt:

Kernel:
https://github.com/torvalds/linux/blob/76f598ba7d8e2bfb4855b5298caedd5af0c374a8/net/mptcp/sockopt.c#L549-L551

Master branch:

mptcp_net-next/net/mptcp/sockopt.c

Lines 549 to 551 in 579883a

    
           case TCP_KEEPIDLE: 
        
           case TCP_KEEPINTVL: 
        
           case TCP_KEEPCNT:

But not implemented in mptcp_setsockopt_sol_tcp:

Kernel:

https://github.com/torvalds/linux/blob/76f598ba7d8e2bfb4855b5298caedd5af0c374a8/net/mptcp/sockopt.c#L780-L819

Master branch:

mptcp_net-next/net/mptcp/sockopt.c

Lines 782 to 821 in 579883a

    
           static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname, 
        
           				    sockptr_t optval, unsigned int optlen) 
        
           { 
        
           	struct sock *sk = (void *)msk; 
        
           	int ret, val; 
        
           	switch (optname) { 
        
           	case TCP_INQ: 
        
           		ret = mptcp_get_int_option(msk, optval, optlen, &val); 
        
           		if (ret) 
        
           			return ret; 
        
           		if (val < 0 || val > 1) 
        
           			return -EINVAL; 
        
           		lock_sock(sk); 
        
           		msk->recvmsg_inq = !!val; 
        
           		release_sock(sk); 
        
           		return 0; 
        
           	case TCP_ULP: 
        
           		return -EOPNOTSUPP; 
        
           	case TCP_CONGESTION: 
        
           		return mptcp_setsockopt_sol_tcp_congestion(msk, optval, optlen); 
        
           	case TCP_CORK: 
        
           		return mptcp_setsockopt_sol_tcp_cork(msk, optval, optlen); 
        
           	case TCP_NODELAY: 
        
           		return mptcp_setsockopt_sol_tcp_nodelay(msk, optval, optlen); 
        
           	case TCP_DEFER_ACCEPT: 
        
           		/* See tcp.c: TCP_DEFER_ACCEPT does not fail */ 
        
           		mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname, optval, optlen); 
        
           		return 0; 
        
           	case TCP_FASTOPEN: 
        
           	case TCP_FASTOPEN_CONNECT: 
        
           	case TCP_FASTOPEN_KEY: 
        
           	case TCP_FASTOPEN_NO_COOKIE: 
        
           		return mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname, 
        
           						      optval, optlen); 
        
           	} 
        
           	return -EOPNOTSUPP; 
        
           }

Which will eventually lead to EOPNOTSUPP when calling setsockopt on these options:

setsockopt(13, SOL_SOCKET, SO_KEEPALIVE, [1], 4) = 0
setsockopt(13, SOL_TCP, TCP_KEEPIDLE, [15], 4) = -1 EOPNOTSUPP
setsockopt(13, SOL_TCP, TCP_KEEPINTVL, [15], 4) = -1 EOPNOTSUPP

Forther more, there are 24 options claimed to be "supported" in mptcp_supported_sockopt, but only 10 are implemented in mptcp_supported_sockopt.

The text was updated successfully, but these errors were encountered:

matttbe · 2023-04-06T13:19:38Z

Hi @zonyitoo

Thank you for this bug report!

TCP_KEEPIDLE, TCP_KEEPINTVL, TCP_KEEPCNT are not implemented

Indeed, it is not implemented.

(It might be easier to do that after #353)

Forther more, there are 24 options claimed to be "supported" in mptcp_supported_sockopt, but only 10 are implemented in mptcp_supported_sockopt.

mptcp_supported_sockopt is indeed a bit confusing. I personally read it as "what might be supported (at some points)".

- TCP_KEEPIDLE, TCP_KEEPINTV may be supported in the future kernel - multipath-tcp/mptcp_net-next#383 - ref #1156

matttbe · 2024-02-15T17:06:47Z

We forgot a bit about this (maybe because of #353).

It looks like supporting these socket options is needed for Go as well, see golang/go#56539 + https://go-review.googlesource.com/c/go/+/563575
As discussed at the last meeting, supporting them should be easy and the patches could be seen as a "bug fix" somehow.

SO_KEEPALIVE support has been added a while ago, as part of a series "adding SOL_SOCKET" support. To have a full control of this keep-alive feature, it is important to also support TCP_KEEP* socket options at the SOL_TCP level. Supporting them on the setsockopt() part is easy, it is just a matter of remembering each value in the MPTCP sock structure, and calling tcp_sock_set_keep*() helpers on each subflow. If the value is not modified (0), calling these helpers will not do anything. For the getsockopt() part, the corresponding value from the MPTCP sock structure or the default one is simply returned. It looks important for kernels supporting SO_KEEPALIVE, to also support TCP_KEEP* options as well: some apps seem to (wrongly) consider that if the former is supported, the latter ones will be supported as well. But also, not having this simple and isolated change is preventing MPTCP support in some apps, and libraries like GoLang [1]. This is why this patch is seen as a fix. Closes: multipath-tcp/mptcp_net-next#383 Fixes: 1b3e7ed ("mptcp: setsockopt: handle SO_KEEPALIVE and SO_PRIORITY") Link: golang/go#56539 [1] Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>

SO_KEEPALIVE support has been added a while ago, as part of a series "adding SOL_SOCKET" support. To have a full control of this keep-alive feature, it is important to also support TCP_KEEP* socket options at the SOL_TCP level. Supporting them on the setsockopt() part is easy, it is just a matter of remembering each value in the MPTCP sock structure, and calling tcp_sock_set_keep*() helpers on each subflow. If the value is not modified (0), calling these helpers will not do anything. For the getsockopt() part, the corresponding value from the MPTCP sock structure or the default one is simply returned. It looks important for kernels supporting SO_KEEPALIVE, to also support TCP_KEEP* options as well: some apps seem to (wrongly) consider that if the former is supported, the latter ones will be supported as well. But also, not having this simple and isolated change is preventing MPTCP support in some apps, and libraries like GoLang [1]. This is why this patch is seen as a fix. Closes: #383 Fixes: 1b3e7ed ("mptcp: setsockopt: handle SO_KEEPALIVE and SO_PRIORITY") Link: golang/go#56539 [1] Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Message-Id: <20240508-mptcp-tcp-keepalive-sockopts-v1-2-fdf7e03e14c4@kernel.org>

SO_KEEPALIVE support has been added a while ago, as part of a series "adding SOL_SOCKET" support. To have a full control of this keep-alive feature, it is important to also support TCP_KEEP* socket options at the SOL_TCP level. Supporting them on the setsockopt() part is easy, it is just a matter of remembering each value in the MPTCP sock structure, and calling tcp_sock_set_keep*() helpers on each subflow. If the value is not modified (0), calling these helpers will not do anything. For the getsockopt() part, the corresponding value from the MPTCP sock structure or the default one is simply returned. All of this is very similar to other TCP_* socket options supported by MPTCP. It looks important for kernels supporting SO_KEEPALIVE, to also support TCP_KEEP* options as well: some apps seem to (wrongly) consider that if the former is supported, the latter ones will be supported as well. But also, not having this simple and isolated change is preventing MPTCP support in some apps, and libraries like GoLang [1]. This is why this patch is seen as a fix. Closes: #383 Fixes: 1b3e7ed ("mptcp: setsockopt: handle SO_KEEPALIVE and SO_PRIORITY") Link: golang/go#56539 [1] Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Message-Id: <20240509-mptcp-tcp-keepalive-sockopts-v2-2-5f97067847a2@kernel.org>

SO_KEEPALIVE support has been added a while ago, as part of a series "adding SOL_SOCKET" support. To have a full control of this keep-alive feature, it is important to also support TCP_KEEP* socket options at the SOL_TCP level. Supporting them on the setsockopt() part is easy, it is just a matter of remembering each value in the MPTCP sock structure, and calling tcp_sock_set_keep*() helpers on each subflow. If the value is not modified (0), calling these helpers will not do anything. For the getsockopt() part, the corresponding value from the MPTCP sock structure or the default one is simply returned. All of this is very similar to other TCP_* socket options supported by MPTCP. It looks important for kernels supporting SO_KEEPALIVE, to also support TCP_KEEP* options as well: some apps seem to (wrongly) consider that if the former is supported, the latter ones will be supported as well. But also, not having this simple and isolated change is preventing MPTCP support in some apps, and libraries like GoLang [1]. This is why this patch is seen as a fix. Closes: #383 Fixes: 1b3e7ed ("mptcp: setsockopt: handle SO_KEEPALIVE and SO_PRIORITY") Link: golang/go#56539 [1] Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Acked-by: Paolo Abeni <[email protected]>

SO_KEEPALIVE support has been added a while ago, as part of a series "adding SOL_SOCKET" support. To have a full control of this keep-alive feature, it is important to also support TCP_KEEP* socket options at the SOL_TCP level. Supporting them on the setsockopt() part is easy, it is just a matter of remembering each value in the MPTCP sock structure, and calling tcp_sock_set_keep*() helpers on each subflow. If the value is not modified (0), calling these helpers will not do anything. For the getsockopt() part, the corresponding value from the MPTCP sock structure or the default one is simply returned. All of this is very similar to other TCP_* socket options supported by MPTCP. It looks important for kernels supporting SO_KEEPALIVE, to also support TCP_KEEP* options as well: some apps seem to (wrongly) consider that if the former is supported, the latter ones will be supported as well. But also, not having this simple and isolated change is preventing MPTCP support in some apps, and libraries like GoLang [1]. This is why this patch is seen as a fix. Closes: multipath-tcp/mptcp_net-next#383 Fixes: 1b3e7ed ("mptcp: setsockopt: handle SO_KEEPALIVE and SO_PRIORITY") Link: golang/go#56539 [1] Acked-by: Paolo Abeni <[email protected]> Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>

SO_KEEPALIVE support has been added a while ago, as part of a series "adding SOL_SOCKET" support. To have a full control of this keep-alive feature, it is important to also support TCP_KEEP* socket options at the SOL_TCP level. Supporting them on the setsockopt() part is easy, it is just a matter of remembering each value in the MPTCP sock structure, and calling tcp_sock_set_keep*() helpers on each subflow. If the value is not modified (0), calling these helpers will not do anything. For the getsockopt() part, the corresponding value from the MPTCP sock structure or the default one is simply returned. All of this is very similar to other TCP_* socket options supported by MPTCP. It looks important for kernels supporting SO_KEEPALIVE, to also support TCP_KEEP* options as well: some apps seem to (wrongly) consider that if the former is supported, the latter ones will be supported as well. But also, not having this simple and isolated change is preventing MPTCP support in some apps, and libraries like GoLang [1]. This is why this patch is seen as a fix. Closes: #383 Fixes: 1b3e7ed ("mptcp: setsockopt: handle SO_KEEPALIVE and SO_PRIORITY") Link: golang/go#56539 [1] Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Acked-by: Paolo Abeni <[email protected]>

SO_KEEPALIVE support has been added a while ago, as part of a series "adding SOL_SOCKET" support. To have a full control of this keep-alive feature, it is important to also support TCP_KEEP* socket options at the SOL_TCP level. Supporting them on the setsockopt() part is easy, it is just a matter of remembering each value in the MPTCP sock structure, and calling tcp_sock_set_keep*() helpers on each subflow. If the value is not modified (0), calling these helpers will not do anything. For the getsockopt() part, the corresponding value from the MPTCP sock structure or the default one is simply returned. All of this is very similar to other TCP_* socket options supported by MPTCP. It looks important for kernels supporting SO_KEEPALIVE, to also support TCP_KEEP* options as well: some apps seem to (wrongly) consider that if the former is supported, the latter ones will be supported as well. But also, not having this simple and isolated change is preventing MPTCP support in some apps, and libraries like GoLang [1]. This is why this patch is seen as a fix. Closes: multipath-tcp/mptcp_net-next#383 Fixes: 1b3e7ed ("mptcp: setsockopt: handle SO_KEEPALIVE and SO_PRIORITY") Link: golang/go#56539 [1] Acked-by: Paolo Abeni <[email protected]> Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: Mat Martineau <[email protected]>

[ Upstream commit bd11dc4 ] SO_KEEPALIVE support has been added a while ago, as part of a series "adding SOL_SOCKET" support. To have a full control of this keep-alive feature, it is important to also support TCP_KEEP* socket options at the SOL_TCP level. Supporting them on the setsockopt() part is easy, it is just a matter of remembering each value in the MPTCP sock structure, and calling tcp_sock_set_keep*() helpers on each subflow. If the value is not modified (0), calling these helpers will not do anything. For the getsockopt() part, the corresponding value from the MPTCP sock structure or the default one is simply returned. All of this is very similar to other TCP_* socket options supported by MPTCP. It looks important for kernels supporting SO_KEEPALIVE, to also support TCP_KEEP* options as well: some apps seem to (wrongly) consider that if the former is supported, the latter ones will be supported as well. But also, not having this simple and isolated change is preventing MPTCP support in some apps, and libraries like GoLang [1]. This is why this patch is seen as a fix. Closes: multipath-tcp/mptcp_net-next#383 Fixes: 1b3e7ed ("mptcp: setsockopt: handle SO_KEEPALIVE and SO_PRIORITY") Link: golang/go#56539 [1] Acked-by: Paolo Abeni <[email protected]> Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: Mat Martineau <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]> Signed-off-by: Sasha Levin <[email protected]>

commit bd11dc4 upstream. SO_KEEPALIVE support has been added a while ago, as part of a series "adding SOL_SOCKET" support. To have a full control of this keep-alive feature, it is important to also support TCP_KEEP* socket options at the SOL_TCP level. Supporting them on the setsockopt() part is easy, it is just a matter of remembering each value in the MPTCP sock structure, and calling tcp_sock_set_keep*() helpers on each subflow. If the value is not modified (0), calling these helpers will not do anything. For the getsockopt() part, the corresponding value from the MPTCP sock structure or the default one is simply returned. All of this is very similar to other TCP_* socket options supported by MPTCP. It looks important for kernels supporting SO_KEEPALIVE, to also support TCP_KEEP* options as well: some apps seem to (wrongly) consider that if the former is supported, the latter ones will be supported as well. But also, not having this simple and isolated change is preventing MPTCP support in some apps, and libraries like GoLang [1]. This is why this patch is seen as a fix. Closes: multipath-tcp/mptcp_net-next#383 Fixes: 1b3e7ed ("mptcp: setsockopt: handle SO_KEEPALIVE and SO_PRIORITY") Link: golang/go#56539 [1] Acked-by: Paolo Abeni <[email protected]> Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: Mat Martineau <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]> [ Conflicts in the same context, because commit 29b5e5e ("mptcp: implement TCP_NOTSENT_LOWAT support") (new feature) and commit 013e317 ("mptcp: fix rcv space initialization") (not backported because of the various conflicts, and because the race fixed by this commit "does not produce ill effects in practice") are not in this version. Also, TCP_KEEPINTVL and TCP_KEEPCNT value had to be set without lock, the same way it was done on TCP side prior commit 6fd70a6 ("tcp: set TCP_KEEPINTVL locklessly") and commit 8448508 ("tcp: set TCP_KEEPCNT locklessly"). ] Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>

commit bd11dc4 upstream. SO_KEEPALIVE support has been added a while ago, as part of a series "adding SOL_SOCKET" support. To have a full control of this keep-alive feature, it is important to also support TCP_KEEP* socket options at the SOL_TCP level. Supporting them on the setsockopt() part is easy, it is just a matter of remembering each value in the MPTCP sock structure, and calling tcp_sock_set_keep*() helpers on each subflow. If the value is not modified (0), calling these helpers will not do anything. For the getsockopt() part, the corresponding value from the MPTCP sock structure or the default one is simply returned. All of this is very similar to other TCP_* socket options supported by MPTCP. It looks important for kernels supporting SO_KEEPALIVE, to also support TCP_KEEP* options as well: some apps seem to (wrongly) consider that if the former is supported, the latter ones will be supported as well. But also, not having this simple and isolated change is preventing MPTCP support in some apps, and libraries like GoLang [1]. This is why this patch is seen as a fix. Closes: multipath-tcp/mptcp_net-next#383 Fixes: 1b3e7ed ("mptcp: setsockopt: handle SO_KEEPALIVE and SO_PRIORITY") Link: golang/go#56539 [1] Acked-by: Paolo Abeni <[email protected]> Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: Mat Martineau <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]> [ conflicts in the same context, because commit 29b5e5e ("mptcp: implement TCP_NOTSENT_LOWAT support") is not in this version ] Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>

commit bd11dc4 upstream. SO_KEEPALIVE support has been added a while ago, as part of a series "adding SOL_SOCKET" support. To have a full control of this keep-alive feature, it is important to also support TCP_KEEP* socket options at the SOL_TCP level. Supporting them on the setsockopt() part is easy, it is just a matter of remembering each value in the MPTCP sock structure, and calling tcp_sock_set_keep*() helpers on each subflow. If the value is not modified (0), calling these helpers will not do anything. For the getsockopt() part, the corresponding value from the MPTCP sock structure or the default one is simply returned. All of this is very similar to other TCP_* socket options supported by MPTCP. It looks important for kernels supporting SO_KEEPALIVE, to also support TCP_KEEP* options as well: some apps seem to (wrongly) consider that if the former is supported, the latter ones will be supported as well. But also, not having this simple and isolated change is preventing MPTCP support in some apps, and libraries like GoLang [1]. This is why this patch is seen as a fix. Closes: multipath-tcp/mptcp_net-next#383 Fixes: 1b3e7ed ("mptcp: setsockopt: handle SO_KEEPALIVE and SO_PRIORITY") Link: golang/go#56539 [1] Acked-by: Paolo Abeni <[email protected]> Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: Mat Martineau <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]> [ Conflicts in the same context, because commit 29b5e5e ("mptcp: implement TCP_NOTSENT_LOWAT support") (new feature) and commit 013e317 ("mptcp: fix rcv space initialization") (not backported because of the various conflicts, and because the race fixed by this commit "does not produce ill effects in practice") are not in this version. Also, TCP_KEEPINTVL and TCP_KEEPCNT value had to be set without lock, the same way it was done on TCP side prior commit 6fd70a6 ("tcp: set TCP_KEEPINTVL locklessly") and commit 8448508 ("tcp: set TCP_KEEPCNT locklessly"). ] Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>

commit bd11dc4 upstream. SO_KEEPALIVE support has been added a while ago, as part of a series "adding SOL_SOCKET" support. To have a full control of this keep-alive feature, it is important to also support TCP_KEEP* socket options at the SOL_TCP level. Supporting them on the setsockopt() part is easy, it is just a matter of remembering each value in the MPTCP sock structure, and calling tcp_sock_set_keep*() helpers on each subflow. If the value is not modified (0), calling these helpers will not do anything. For the getsockopt() part, the corresponding value from the MPTCP sock structure or the default one is simply returned. All of this is very similar to other TCP_* socket options supported by MPTCP. It looks important for kernels supporting SO_KEEPALIVE, to also support TCP_KEEP* options as well: some apps seem to (wrongly) consider that if the former is supported, the latter ones will be supported as well. But also, not having this simple and isolated change is preventing MPTCP support in some apps, and libraries like GoLang [1]. This is why this patch is seen as a fix. Closes: multipath-tcp/mptcp_net-next#383 Fixes: 1b3e7ed ("mptcp: setsockopt: handle SO_KEEPALIVE and SO_PRIORITY") Link: golang/go#56539 [1] Acked-by: Paolo Abeni <[email protected]> Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: Mat Martineau <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]> [ conflicts in the same context, because commit 29b5e5e ("mptcp: implement TCP_NOTSENT_LOWAT support") is not in this version ] Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>

commit bd11dc4 upstream. SO_KEEPALIVE support has been added a while ago, as part of a series "adding SOL_SOCKET" support. To have a full control of this keep-alive feature, it is important to also support TCP_KEEP* socket options at the SOL_TCP level. Supporting them on the setsockopt() part is easy, it is just a matter of remembering each value in the MPTCP sock structure, and calling tcp_sock_set_keep*() helpers on each subflow. If the value is not modified (0), calling these helpers will not do anything. For the getsockopt() part, the corresponding value from the MPTCP sock structure or the default one is simply returned. All of this is very similar to other TCP_* socket options supported by MPTCP. It looks important for kernels supporting SO_KEEPALIVE, to also support TCP_KEEP* options as well: some apps seem to (wrongly) consider that if the former is supported, the latter ones will be supported as well. But also, not having this simple and isolated change is preventing MPTCP support in some apps, and libraries like GoLang [1]. This is why this patch is seen as a fix. Closes: multipath-tcp/mptcp_net-next#383 Fixes: 1b3e7ed ("mptcp: setsockopt: handle SO_KEEPALIVE and SO_PRIORITY") Link: golang/go#56539 [1] Acked-by: Paolo Abeni <[email protected]> Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: Mat Martineau <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]> [ Conflicts in the same context, because commit 29b5e5e ("mptcp: implement TCP_NOTSENT_LOWAT support") (new feature), commit 013e317 ("mptcp: fix rcv space initialization") (not backported because of the various conflicts, and because the race fixed by this commit "does not produce ill effects in practice"), and commit 4f6e14b ("mptcp: support TCP_CORK and TCP_NODELAY") are not in this version. The adaptations done by 7f71a33 ("mptcp: cleanup SOL_TCP handling") have been adapted to this case here. Also, TCP_KEEPINTVL and TCP_KEEPCNT value had to be set without lock, the same way it was done on TCP side prior commit 6fd70a6 ("tcp: set TCP_KEEPINTVL locklessly") and commit 8448508 ("tcp: set TCP_KEEPCNT locklessly"). ] Signed-off-by: Matthieu Baerts (NGI0) <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>