net/mptcp/protocol.h | 3 +++ net/mptcp/sockopt.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 2 deletions(-)
This is linked to a discussion we had a few weeks ago: not supporting
TCP_KEEP* socket options is preventing MPTCP to be used in some apps or
libraries like it was the case in GoLang recently.
Supporting them is not difficult, it should have probably done before,
when SO_KEEPALIVE support has been added. Supporting them is easy and
isolated from the rest, it sounds safe enough to add a Fixes tag, and
check with the stable team to backport them. I understand it can be a
bit controversial, but it would unnecessary delay (1 or 2 years?) some
deployments, for something that simple.
While at it, getsockopt(SO_KEEPALIVE) is now returning the expected
value.
The manipulation of these different socket options can be verified with
this simple packetdrill test:
--tolerance_usecs=100000
`../common/defaults.sh`
0.0 socket(..., SOCK_STREAM, IPPROTO_MPTCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1) = 0
+0 < S 0:0(0) win 8000 <mss 1024, sackOK, nop, nop, nop, wscale 0, mpcapable v1 flags[flag_h] nokey>
+0 > S. 0:0(0) ack 1 <mss 1460, nop, nop, sackOK, nop, wscale 8, mpcapable v1 flags[flag_h] key[skey]>
+0.01 < . 1:1(0) ack 1 win 8000 <mpcapable v1 flags[flag_h] key[ckey=2, skey]>
+0 accept(3, ..., ...) = 4
// Set and check the different Keep-Alive options.
+0 getsockopt(4, SOL_SOCKET, SO_KEEPALIVE, [0], [4]) = 0
+0 setsockopt(4, SOL_SOCKET, SO_KEEPALIVE, [1], 4) = 0
+0 getsockopt(4, SOL_SOCKET, SO_KEEPALIVE, [1], [4]) = 0
+0 getsockopt(4, SOL_TCP, TCP_KEEPIDLE, [7200], [4]) = 0
+0 setsockopt(4, SOL_TCP, TCP_KEEPIDLE, [7200], 4) = 0
+0 getsockopt(4, SOL_TCP, TCP_KEEPIDLE, [7200], [4]) = 0
+0 getsockopt(4, SOL_TCP, TCP_KEEPINTVL, [75], [4]) = 0
+0 setsockopt(4, SOL_TCP, TCP_KEEPINTVL, [10], 4) = 0
+0 getsockopt(4, SOL_TCP, TCP_KEEPINTVL, [10], [4]) = 0
+0 getsockopt(4, SOL_TCP, TCP_KEEPCNT, [9], [4]) = 0
+0 setsockopt(4, SOL_TCP, TCP_KEEPCNT, [2], 4) = 0
+0 getsockopt(4, SOL_TCP, TCP_KEEPCNT, [2], [4]) = 0
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Changes in v2:
- Addressed Paolo's comments in patch 2/2, see the individual changelog.
- Link to v1: https://lore.kernel.org/r/20240508-mptcp-tcp-keepalive-sockopts-v1-0-fdf7e03e14c4@kernel.org
---
Matthieu Baerts (NGI0) (2):
mptcp: SO_KEEPALIVE: fix getsockopt support
mptcp: fix full TCP keep-alive support
net/mptcp/protocol.h | 3 +++
net/mptcp/sockopt.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 61 insertions(+), 2 deletions(-)
---
base-commit: a64044939fd33676d7098eb08397e65e83d278ed
change-id: 20240507-mptcp-tcp-keepalive-sockopts-546d3b1f3256
Best regards,
--
Matthieu Baerts (NGI0) <matttbe@kernel.org>
On Thu, 2024-05-09 at 12:48 +0200, Matthieu Baerts (NGI0) wrote: > This is linked to a discussion we had a few weeks ago: not supporting > TCP_KEEP* socket options is preventing MPTCP to be used in some apps or > libraries like it was the case in GoLang recently. > > Supporting them is not difficult, it should have probably done before, > when SO_KEEPALIVE support has been added. Supporting them is easy and > isolated from the rest, it sounds safe enough to add a Fixes tag, and > check with the stable team to backport them. I understand it can be a > bit controversial, but it would unnecessary delay (1 or 2 years?) some > deployments, for something that simple. > > While at it, getsockopt(SO_KEEPALIVE) is now returning the expected > value. > > The manipulation of these different socket options can be verified with > this simple packetdrill test: > > --tolerance_usecs=100000 > `../common/defaults.sh` > > 0.0 socket(..., SOCK_STREAM, IPPROTO_MPTCP) = 3 > +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0 > +0 bind(3, ..., ...) = 0 > +0 listen(3, 1) = 0 > > +0 < S 0:0(0) win 8000 <mss 1024, sackOK, nop, nop, nop, wscale 0, mpcapable v1 flags[flag_h] nokey> > +0 > S. 0:0(0) ack 1 <mss 1460, nop, nop, sackOK, nop, wscale 8, mpcapable v1 flags[flag_h] key[skey]> > +0.01 < . 1:1(0) ack 1 win 8000 <mpcapable v1 flags[flag_h] key[ckey=2, skey]> > +0 accept(3, ..., ...) = 4 > > // Set and check the different Keep-Alive options. > +0 getsockopt(4, SOL_SOCKET, SO_KEEPALIVE, [0], [4]) = 0 > +0 setsockopt(4, SOL_SOCKET, SO_KEEPALIVE, [1], 4) = 0 > +0 getsockopt(4, SOL_SOCKET, SO_KEEPALIVE, [1], [4]) = 0 > > +0 getsockopt(4, SOL_TCP, TCP_KEEPIDLE, [7200], [4]) = 0 > +0 setsockopt(4, SOL_TCP, TCP_KEEPIDLE, [7200], 4) = 0 > +0 getsockopt(4, SOL_TCP, TCP_KEEPIDLE, [7200], [4]) = 0 > > +0 getsockopt(4, SOL_TCP, TCP_KEEPINTVL, [75], [4]) = 0 > +0 setsockopt(4, SOL_TCP, TCP_KEEPINTVL, [10], 4) = 0 > +0 getsockopt(4, SOL_TCP, TCP_KEEPINTVL, [10], [4]) = 0 > > +0 getsockopt(4, SOL_TCP, TCP_KEEPCNT, [9], [4]) = 0 > +0 setsockopt(4, SOL_TCP, TCP_KEEPCNT, [2], 4) = 0 > +0 getsockopt(4, SOL_TCP, TCP_KEEPCNT, [2], [4]) = 0 > > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> LGTM, thanks! Acked-by: Paolo Abeni <pabeni@redhat.com>
Hi Paolo, On 10/05/2024 10:32, Paolo Abeni wrote: > On Thu, 2024-05-09 at 12:48 +0200, Matthieu Baerts (NGI0) wrote: >> This is linked to a discussion we had a few weeks ago: not supporting >> TCP_KEEP* socket options is preventing MPTCP to be used in some apps or >> libraries like it was the case in GoLang recently. >> >> Supporting them is not difficult, it should have probably done before, >> when SO_KEEPALIVE support has been added. Supporting them is easy and >> isolated from the rest, it sounds safe enough to add a Fixes tag, and >> check with the stable team to backport them. I understand it can be a >> bit controversial, but it would unnecessary delay (1 or 2 years?) some >> deployments, for something that simple. >> >> While at it, getsockopt(SO_KEEPALIVE) is now returning the expected >> value. (...) >> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > > LGTM, thanks! > > Acked-by: Paolo Abeni <pabeni@redhat.com> Thank you for the review! Now in our tree (fixes for net-next, in preparation for the next batch): New patches for t/upstream: - 1f3df7457803: mptcp: SO_KEEPALIVE: fix getsockopt support - f246f7a50963: mptcp: fix full TCP keep-alive support - Results: 18d3f4f7094a..ea351593813d (export) Tests are now in progress: - export: https://github.com/multipath-tcp/mptcp_net-next/commit/2f64d8457b9c03ff3d78f3efb8da42f94ce7e277/checks Cheers, Matt -- Sponsored by the NGI0 Core fund.
© 2016 - 2024 Red Hat, Inc.