Now that the 'flags' attribute is used, it seems interesting to add one flag for 'server-side', a boolean value.
Here are a few patches related to the 'server-side' attribute:
- Patch 1: only announce this attribute on the server side.
- Patch 2: announce the 'server-side' flag when this is the case.
- Patch 3: deprecate the 'server-side' attribute.
- Patch 4: use the 'server-side' flag in the selftests.
- Patches 5, 6: small cleanups when working on code around.
Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- Matthieu Baerts (NGI0) (6): mptcp: pm: netlink: only add server-side attr when true mptcp: pm: netlink: announce server-side flag mptcp: pm: netlink: deprecate server-side attribute selftests: mptcp: pm: get server-side flag mptcp: use _BITUL() instead of (1 << x) mptcp: remove unused returned value of check_data_fin
Documentation/netlink/specs/mptcp_pm.yaml | 5 +++-- include/uapi/linux/mptcp.h | 11 ++++++----- include/uapi/linux/mptcp_pm.h | 4 ++-- net/mptcp/pm_netlink.c | 9 +++++++-- net/mptcp/protocol.c | 5 +---- tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 9 ++++++++- tools/testing/selftests/net/mptcp/userspace_pm.sh | 2 +- 7 files changed, 28 insertions(+), 17 deletions(-) --- base-commit: 315f423be0d1ebe720d8fd4fa6bed68586b13d34 change-id: 20250916-net-next-mptcp-server-side-flag-0f002418946d
Best regards,
This attribute is a boolean. No need to add it to set it to 'false'.
Indeed, the default value when this attribute is not set is naturally 'false'. A few bytes can then be saved by not adding this attribute if the connection is not on the server side.
This prepares the future deprecation of its attribute, in favour of a new flag.
Reviewed-by: Geliang Tang geliang@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- Documentation/netlink/specs/mptcp_pm.yaml | 4 ++-- include/uapi/linux/mptcp_pm.h | 4 ++-- net/mptcp/pm_netlink.c | 4 +++- tools/testing/selftests/net/mptcp/userspace_pm.sh | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/Documentation/netlink/specs/mptcp_pm.yaml b/Documentation/netlink/specs/mptcp_pm.yaml index d1b4829b580ad09baf4afd73b67abd7b4ef6883a..fc47a2931014c0304efd3215cc24485ea22e1ede 100644 --- a/Documentation/netlink/specs/mptcp_pm.yaml +++ b/Documentation/netlink/specs/mptcp_pm.yaml @@ -28,13 +28,13 @@ definitions: traffic-patterns it can take a long time until the MPTCP_EVENT_ESTABLISHED is sent. Attributes: token, family, saddr4 | saddr6, daddr4 | daddr6, sport, - dport, server-side, [flags]. + dport, [server-side], [flags]. - name: established doc: >- A MPTCP connection is established (can start new subflows). Attributes: token, family, saddr4 | saddr6, daddr4 | daddr6, sport, - dport, server-side, [flags]. + dport, [server-side], [flags]. - name: closed doc: >- diff --git a/include/uapi/linux/mptcp_pm.h b/include/uapi/linux/mptcp_pm.h index 7359d34da446b94be148b363079120db03ba8549..bf44a5cf5b5a1e6d789632682a9bedbf8090feb9 100644 --- a/include/uapi/linux/mptcp_pm.h +++ b/include/uapi/linux/mptcp_pm.h @@ -16,10 +16,10 @@ * good time to allocate memory and send ADD_ADDR if needed. Depending on the * traffic-patterns it can take a long time until the MPTCP_EVENT_ESTABLISHED * is sent. Attributes: token, family, saddr4 | saddr6, daddr4 | daddr6, - * sport, dport, server-side, [flags]. + * sport, dport, [server-side], [flags]. * @MPTCP_EVENT_ESTABLISHED: A MPTCP connection is established (can start new * subflows). Attributes: token, family, saddr4 | saddr6, daddr4 | daddr6, - * sport, dport, server-side, [flags]. + * sport, dport, [server-side], [flags]. * @MPTCP_EVENT_CLOSED: A MPTCP connection has stopped. Attribute: token. * @MPTCP_EVENT_ANNOUNCED: A new address has been announced by the peer. * Attributes: token, rem_id, family, daddr4 | daddr6 [, dport]. diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 483ddbb9ec406a3e965376ee5a833ae295896a02..33a6bf536c020b59717472aca2d38add26255419 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -413,7 +413,9 @@ static int mptcp_event_created(struct sk_buff *skb, if (err) return err;
- if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, READ_ONCE(msk->pm.server_side))) + /* only set when it is the server side */ + if (READ_ONCE(msk->pm.server_side) && + nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1)) return -EMSGSIZE;
if (READ_ONCE(msk->pm.remote_deny_join_id0)) diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh index 3d45991f24ede919264185e3b5c2a3b95c1dcc85..87323942cb8a0779717e6c0cb6be46314d303d26 100755 --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh @@ -241,7 +241,7 @@ make_connection()
print_test "Established IP${is_v6} MPTCP Connection ns2 => ns1" if [ "${client_token}" != "" ] && [ "${server_token}" != "" ] && - [ "${client_serverside}" = 0 ] && [ "${server_serverside}" = 1 ] && + [ "${client_serverside:-0}" = 0 ] && [ "${server_serverside:-0}" = 1 ] && [ "${client_nojoin:-0}" = 0 ] && [ "${server_nojoin:-0}" = 1 ] then test_pass
Now that the 'flags' attribute is used, it seems interesting to add one flag for 'server-side', a boolean value.
This is duplicating the info from the dedicated 'server-side' attribute, but it will be deprecated in the next commit, and removed in a few versions.
Reviewed-by: Geliang Tang geliang@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- include/uapi/linux/mptcp.h | 1 + net/mptcp/pm_netlink.c | 11 +++++++---- 2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h index 5fd5b4cf75ca1e0099e0effa351507d3ab002f1e..95d621f6d59810126cbc37b1d6baf896a40dd9bc 100644 --- a/include/uapi/linux/mptcp.h +++ b/include/uapi/linux/mptcp.h @@ -32,6 +32,7 @@ #define MPTCP_INFO_FLAG_REMOTE_KEY_RECEIVED _BITUL(1)
#define MPTCP_PM_EV_FLAG_DENY_JOIN_ID0 _BITUL(0) +#define MPTCP_PM_EV_FLAG_SERVER_SIDE _BITUL(1)
#define MPTCP_PM_ADDR_FLAG_SIGNAL (1 << 0) #define MPTCP_PM_ADDR_FLAG_SUBFLOW (1 << 1) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 33a6bf536c020b59717472aca2d38add26255419..aa0c73faaa6acad3fd66ea0942726ecd4d0abcc0 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -413,10 +413,13 @@ static int mptcp_event_created(struct sk_buff *skb, if (err) return err;
- /* only set when it is the server side */ - if (READ_ONCE(msk->pm.server_side) && - nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1)) - return -EMSGSIZE; + if (READ_ONCE(msk->pm.server_side)) { + flags |= MPTCP_PM_EV_FLAG_SERVER_SIDE; + + /* only set when it is the server side */ + if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1)) + return -EMSGSIZE; + }
if (READ_ONCE(msk->pm.remote_deny_join_id0)) flags |= MPTCP_PM_EV_FLAG_DENY_JOIN_ID0;
Now that such info is in the 'flags' attribute, it is time to deprecate the dedicated 'server-side' attribute.
It will be removed in a few versions.
Reviewed-by: Geliang Tang geliang@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- Documentation/netlink/specs/mptcp_pm.yaml | 1 + net/mptcp/pm_netlink.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/netlink/specs/mptcp_pm.yaml b/Documentation/netlink/specs/mptcp_pm.yaml index fc47a2931014c0304efd3215cc24485ea22e1ede..ba30a40b9dbf2d2d4f25cc07b309ea560712f65e 100644 --- a/Documentation/netlink/specs/mptcp_pm.yaml +++ b/Documentation/netlink/specs/mptcp_pm.yaml @@ -266,6 +266,7 @@ attribute-sets: - name: server-side type: u8 + doc: "Deprecated: use 'flags'"
operations: list: diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index aa0c73faaa6acad3fd66ea0942726ecd4d0abcc0..d5b383870f79956ce5e10bf384695621604f3ce9 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -416,7 +416,7 @@ static int mptcp_event_created(struct sk_buff *skb, if (READ_ONCE(msk->pm.server_side)) { flags |= MPTCP_PM_EV_FLAG_SERVER_SIDE;
- /* only set when it is the server side */ + /* Deprecated, and only set when it is the server side */ if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1)) return -EMSGSIZE; }
server-side info linked to the MPTCP connect/established events can now come from the flags, in addition to the dedicated attribute.
The attribute is now deprecated -- in favour of the new flag, and will be removed later on.
Print this info only once.
Reviewed-by: Geliang Tang geliang@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c index 93fea3442216c8fef43731a99c1d5710f234b150..d4981b76693bbddca74169437a540ad6294cf1d5 100644 --- a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c +++ b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c @@ -2,6 +2,7 @@
#include <errno.h> #include <error.h> +#include <stdbool.h> #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -113,6 +114,8 @@ static int capture_events(int fd, int event_group) error(1, errno, "could not join the " MPTCP_PM_EV_GRP_NAME " mcast group");
do { + bool server_side = false; + FD_ZERO(&rfds); FD_SET(fd, &rfds); res_len = NLMSG_ALIGN(sizeof(struct nlmsghdr)) + @@ -187,18 +190,22 @@ static int capture_events(int fd, int event_group) else if (attrs->rta_type == MPTCP_ATTR_ERROR) fprintf(stderr, ",error:%u", *(__u8 *)RTA_DATA(attrs)); else if (attrs->rta_type == MPTCP_ATTR_SERVER_SIDE) - fprintf(stderr, ",server_side:%u", *(__u8 *)RTA_DATA(attrs)); + server_side = !!*(__u8 *)RTA_DATA(attrs); else if (attrs->rta_type == MPTCP_ATTR_FLAGS) { __u16 flags = *(__u16 *)RTA_DATA(attrs);
/* only print when present, easier */ if (flags & MPTCP_PM_EV_FLAG_DENY_JOIN_ID0) fprintf(stderr, ",deny_join_id0:1"); + if (flags & MPTCP_PM_EV_FLAG_SERVER_SIDE) + server_side = true; }
attrs = RTA_NEXT(attrs, msg_len); } } + if (server_side) + fprintf(stderr, ",server_side:1"); fprintf(stderr, "\n"); } while (1);
Simply to use the proper way to declare bits, and to align with all other flags declared in this file.
No functional changes intended.
Reviewed-by: Geliang Tang geliang@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- include/uapi/linux/mptcp.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h index 95d621f6d59810126cbc37b1d6baf896a40dd9bc..15eef878690b8556af21be8d959b6a2c9fe617d3 100644 --- a/include/uapi/linux/mptcp.h +++ b/include/uapi/linux/mptcp.h @@ -34,11 +34,11 @@ #define MPTCP_PM_EV_FLAG_DENY_JOIN_ID0 _BITUL(0) #define MPTCP_PM_EV_FLAG_SERVER_SIDE _BITUL(1)
-#define MPTCP_PM_ADDR_FLAG_SIGNAL (1 << 0) -#define MPTCP_PM_ADDR_FLAG_SUBFLOW (1 << 1) -#define MPTCP_PM_ADDR_FLAG_BACKUP (1 << 2) -#define MPTCP_PM_ADDR_FLAG_FULLMESH (1 << 3) -#define MPTCP_PM_ADDR_FLAG_IMPLICIT (1 << 4) +#define MPTCP_PM_ADDR_FLAG_SIGNAL _BITUL(0) +#define MPTCP_PM_ADDR_FLAG_SUBFLOW _BITUL(1) +#define MPTCP_PM_ADDR_FLAG_BACKUP _BITUL(2) +#define MPTCP_PM_ADDR_FLAG_FULLMESH _BITUL(3) +#define MPTCP_PM_ADDR_FLAG_IMPLICIT _BITUL(4)
struct mptcp_info { __u8 mptcpi_subflows;
When working on a fix modifying mptcp_check_data_fin(), I noticed the returned value was no longer used.
It looks like it was used for 3 days, between commit 7ed90803a213 ("mptcp: send explicit ack on delayed ack_seq incr") and commit ea4ca586b16f ("mptcp: refine MPTCP-level ack scheduling").
This returned value can be safely removed.
Reviewed-by: Geliang Tang geliang@kernel.org Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org --- net/mptcp/protocol.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index d9fbddb99ad0fb53f160af7654433a12c609ee25..735a209d40725f077de1056de5e1c64ffec77f55 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -545,11 +545,10 @@ static void mptcp_cleanup_rbuf(struct mptcp_sock *msk, int copied) } }
-static bool mptcp_check_data_fin(struct sock *sk) +static void mptcp_check_data_fin(struct sock *sk) { struct mptcp_sock *msk = mptcp_sk(sk); u64 rcv_data_fin_seq; - bool ret = false;
/* Need to ack a DATA_FIN received from a peer while this side * of the connection is in ESTABLISHED, FIN_WAIT1, or FIN_WAIT2. @@ -588,12 +587,10 @@ static bool mptcp_check_data_fin(struct sock *sk) break; }
- ret = true; if (!__mptcp_check_fallback(msk)) mptcp_send_ack(msk); mptcp_close_wake_up(sk); } - return ret; }
static void mptcp_dss_corruption(struct mptcp_sock *msk, struct sock *ssk)
linux-kselftest-mirror@lists.linaro.org
-
Matthieu Baerts (NGI0)