[PATCH mptcp-net 0/7] mptcp: pm: nl: announce deny-join-id0 attribute

[PATCH mptcp-net 0/7] mptcp: pm: nl: announce deny-join-id0 attribute

[Matthieu Baerts (NGI0)]

During the connection establishment, a peer can tell the other that it
cannot establish new subflows to the initial IP address and port by
setting the 'C' flag [1]. Doing so makes sense when the sender is behind
a strict NAT, operating behind a legacy Layer 4 load balancer, or using
anycast IP address for example.

When this 'C' flag is set, the path-managers must then not try to
establish new subflow to the other peer's initial IP address and port.
The in-kernel PM has access to this info, but the userspace PM didn't.

The last patch is adding the new attribute in the Netlink events. Please
see the note with a question about how to add this new attribute.

But before that, a few fixes have been added:

- Patch 1: add remote_deny_join_id0 info on passive connections. A fix
  for v5.14.

- Patch 2: respect the deny_join_id0 attribute by blocking the creation
  of new subflows to the initial IP address and port if set. A fix for
  v5.19.

- Patch 3: record the deny_join_id0 info when TFO is used. A fix for
  v6.2.

- Patch 4: fix a wrong attribute type in the Netlink MPTCP specs. A fix
  for v6.7.

- Patch 5: stop mentioning net.mptcp.pm_type in the doc as it is
  deprecated. A fix for v6.15.

- Patch 6: support Shellcheck v0.11.0 in the MPTCP selftests.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Matthieu Baerts (NGI0) (7):
      mptcp: set remote_deny_join_id0 on SYN recv
      mptcp: pm: userspace: respect deny_join_id0 attr
      mptcp: tfo: record 'deny join id0' info
      netlink: specs: mptcp: fix if-idx attribute type
      doc: mptcp: net.mptcp.pm_type is deprecated
      selftests: mptcp: shellcheck: support v0.11.0
      [RFC] mptcp: pm: nl: announce deny-join-id0 attribute

 Documentation/netlink/specs/mptcp_pm.yaml          | 9 ++++++---
 Documentation/networking/mptcp.rst                 | 8 ++++----
 include/uapi/linux/mptcp_pm.h                      | 5 +++--
 net/mptcp/options.c                                | 6 +++---
 net/mptcp/pm_netlink.c                             | 4 ++++
 net/mptcp/pm_userspace.c                           | 6 ++++++
 net/mptcp/subflow.c                                | 4 ++++
 tools/testing/selftests/net/mptcp/diag.sh          | 2 +-
 tools/testing/selftests/net/mptcp/mptcp_connect.sh | 2 +-
 tools/testing/selftests/net/mptcp/mptcp_join.sh    | 2 +-
 tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 2 +-
 tools/testing/selftests/net/mptcp/pm_netlink.sh    | 5 +++--
 tools/testing/selftests/net/mptcp/simult_flows.sh  | 2 +-
 tools/testing/selftests/net/mptcp/userspace_pm.sh  | 2 +-
 14 files changed, 39 insertions(+), 20 deletions(-)
---
base-commit: 569dfa7d27f329168b34177931080efc317511b1
change-id: 20250720-mptcp-pm-user-c-flag-a7d5d7a00c6f

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>

[PATCH mptcp-net 1/7] mptcp: set remote_deny_join_id0 on SYN recv

[Matthieu Baerts (NGI0)]

When a SYN containing the 'C' flag (deny join id0) was received, this
piece of information was not propagated to the path-manager.

Even if this flag is mainly set on the server side, a client can also
tell the server it cannot try to establish new subflows to the client's
initial IP address and port. The server's PM should then record such
info when received, and before sending events about the new connection.

Fixes: df377be38725 ("mptcp: add deny_join_id0 in mptcp_options_received")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/subflow.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index c8a7e4b59db1150610f99d6e599a9d4cf7e137bc..e8325890a32238bd1fd558b955137a2fa32f66c2 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -883,6 +883,10 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
 
 			ctx->subflow_id = 1;
 			owner = mptcp_sk(ctx->conn);
+
+			if (mp_opt.deny_join_id0)
+				WRITE_ONCE(owner->pm.remote_deny_join_id0, true);
+
 			mptcp_pm_new_connection(owner, child, 1);
 
 			/* with OoO packets we can reach here without ingress

-- 
2.50.1

[PATCH mptcp-net 2/7] mptcp: pm: userspace: respect deny_join_id0 attr

[Matthieu Baerts (NGI0)]

During the connection establishment, a peer can tell the other that it
cannot establish new subflows to the initial IP address and port by
setting the 'C' flag [1]. The RFC8684 is strict about that:

  (...) therefore the receiver MUST NOT try to open any additional
  subflows toward this address and port.

It is then important not to let the userspace PM establishing such
subflows, and return an error (ECONNREFUSED) if it tries to do so.

Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow establishment")
Link: https://datatracker.ietf.org/doc/html/rfc8684\#section-3.1-20.6 [1]
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_userspace.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 1911fe1799fa38a53381247a830a9a0daf1c4492..9968dc9a8b45112114953f66848ea22c971136d6 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -391,6 +391,12 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 	if (err < 0)
 		goto create_err;
 
+	if (READ_ONCE(msk->pm.remote_deny_join_id0) && addr_r.id == 0) {
+		GENL_SET_ERR_MSG(info, "deny join id0");
+		err = -ECONNREFUSED;
+		goto create_err;
+	}
+
 	if (!mptcp_pm_addr_families_match(sk, &entry.addr, &addr_r)) {
 		GENL_SET_ERR_MSG(info, "families mismatch");
 		err = -EINVAL;

-- 
2.50.1

[PATCH mptcp-net 3/7] mptcp: tfo: record 'deny join id0' info

[Matthieu Baerts (NGI0)]

When TFO is used, the check to see if the 'C' flag (deny join id0) was
set was bypassed.

This flag can be set when TFO is used, so the check should also be done
when TFO is used.

Fixes: dfc8d0603033 ("mptcp: implement delayed seq generation for passive fastopen")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/options.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index d47b8a9bc2df2f14645b1b3d3e10fea1b38567b1..cf531f2d815cdfbc772b837def6e7d558e64d558 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -985,14 +985,14 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
 		return false;
 	}
 
-	if (mp_opt->deny_join_id0)
-		WRITE_ONCE(msk->pm.remote_deny_join_id0, true);
-
 	if (unlikely(!READ_ONCE(msk->pm.server_side)))
 		/* DO-NOT-MERGE: use WARN i/o pr_warn: only for MPTCP export */
 		WARN_ONCE(1, "bogus mpc option on established client sk");
 
 set_fully_established:
+	if (mp_opt->deny_join_id0)
+		WRITE_ONCE(msk->pm.remote_deny_join_id0, true);
+
 	mptcp_data_lock((struct sock *)msk);
 	__mptcp_subflow_fully_established(msk, subflow, mp_opt);
 	mptcp_data_unlock((struct sock *)msk);

-- 
2.50.1

[PATCH mptcp-net 4/7] netlink: specs: mptcp: fix if-idx attribute type

[Matthieu Baerts (NGI0)]

This attribute is used as a signed number in the code in pm_netlink.c:

  nla_put_s32(skb, MPTCP_ATTR_IF_IDX, ssk->sk_bound_dev_if))

The specs should then reflect that. Note that other 'if-idx' attributes
from the same .yaml file use a signed number as well.

Fixes: bc8aeb2045e2 ("Documentation: netlink: add a YAML spec for mptcp")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 Documentation/netlink/specs/mptcp_pm.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/netlink/specs/mptcp_pm.yaml b/Documentation/netlink/specs/mptcp_pm.yaml
index 02f1ddcfbf1cfd81a398dd03c52bb9f281c1aa08..d15335684ec3d6256505f2b3887ce5818eb57462 100644
--- a/Documentation/netlink/specs/mptcp_pm.yaml
+++ b/Documentation/netlink/specs/mptcp_pm.yaml
@@ -256,7 +256,7 @@ attribute-sets:
         type: u32
       -
         name: if-idx
-        type: u32
+        type: s32
       -
         name: reset-reason
         type: u32

-- 
2.50.1

[PATCH mptcp-net 5/7] doc: mptcp: net.mptcp.pm_type is deprecated

[Matthieu Baerts (NGI0)]

The net.mptcp.pm_type sysctl knob has been deprecated in v6.15,
net.mptcp.path_manager should be used instead.

Adapt the section about path managers to suggest using the new sysctl
knob instead of the deprecated one.

Fixes: 595c26d122d1 ("mptcp: sysctl: set path manager by name")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 Documentation/networking/mptcp.rst | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/networking/mptcp.rst b/Documentation/networking/mptcp.rst
index 17f2bab611644727e19c3969fa08fa974c702d92..2e31038d6462051387be9bd8808a23230db08315 100644
--- a/Documentation/networking/mptcp.rst
+++ b/Documentation/networking/mptcp.rst
@@ -60,10 +60,10 @@ address announcements. Typically, it is the client side that initiates subflows,
 and the server side that announces additional addresses via the ``ADD_ADDR`` and
 ``REMOVE_ADDR`` options.
 
-Path managers are controlled by the ``net.mptcp.pm_type`` sysctl knob -- see
-mptcp-sysctl.rst. There are two types: the in-kernel one (type ``0``) where the
-same rules are applied for all the connections (see: ``ip mptcp``) ; and the
-userspace one (type ``1``), controlled by a userspace daemon (i.e. `mptcpd
+Path managers are controlled by the ``net.mptcp.path_manager`` sysctl knob --
+see mptcp-sysctl.rst. There are two types: the in-kernel one (``kernel``) where
+the same rules are applied for all the connections (see: ``ip mptcp``) ; and the
+userspace one (``userspace``), controlled by a userspace daemon (i.e. `mptcpd
 <https://mptcpd.mptcp.dev/>`_) where different rules can be applied for each
 connection. The path managers can be controlled via a Netlink API; see
 netlink_spec/mptcp_pm.rst.

-- 
2.50.1

[PATCH mptcp-net 6/7] selftests: mptcp: shellcheck: support v0.11.0

[Matthieu Baerts (NGI0)]

This v0.11.0 version introduces SC2329:

  Warn when (non-escaping) functions are never invoked.

Except that, similar to SC2317, ShellCheck is currently unable to figure
out functions that are invoked via trap, or indirectly, when calling
functions via variables. It is then needed to disable this new SC2329.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Note: I guess this patch is a good candidate for 'net'.
---
 tools/testing/selftests/net/mptcp/diag.sh          | 2 +-
 tools/testing/selftests/net/mptcp/mptcp_connect.sh | 2 +-
 tools/testing/selftests/net/mptcp/mptcp_join.sh    | 2 +-
 tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 2 +-
 tools/testing/selftests/net/mptcp/pm_netlink.sh    | 5 +++--
 tools/testing/selftests/net/mptcp/simult_flows.sh  | 2 +-
 tools/testing/selftests/net/mptcp/userspace_pm.sh  | 2 +-
 7 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index 7a3cb4c09e450f0ae570015c4724ec268c6dc19f..d847ff1737c30c0eae1cefeb5a83bd3223897707 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -28,7 +28,7 @@ flush_pids()
 }
 
 # This function is used in the cleanup trap
-#shellcheck disable=SC2317
+#shellcheck disable=SC2317,SC2329
 cleanup()
 {
 	ip netns pids "${ns}" | xargs --no-run-if-empty kill -SIGKILL &>/dev/null
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 5e3c56253274a1f938d2ed9986c4290fcea8b96b..c2ab9f7f0d2133559bb18ce884b613d21d1ec5f0 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -134,7 +134,7 @@ ns4=""
 TEST_GROUP=""
 
 # This function is used in the cleanup trap
-#shellcheck disable=SC2317
+#shellcheck disable=SC2317,SC2329
 cleanup()
 {
 	rm -f "$cin_disconnect"
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index a97b568104bc284f050b2f0e09fe3fdd3341c5cb..f2133e247d8cbc17abd3c3563261068a65b56ad8 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -8,7 +8,7 @@
 
 # ShellCheck incorrectly believes that most of the code here is unreachable
 # because it's invoked by variable name, see how the "tests" array is used
-#shellcheck disable=SC2317
+#shellcheck disable=SC2317,SC2329
 
 . "$(dirname "${0}")/mptcp_lib.sh"
 
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index 418a903c3a4d396bd733bf8b6f68b1447d4d1de3..f01989be6e9b3daeecc5a8f41b37c9a284efef61 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -95,7 +95,7 @@ init()
 }
 
 # This function is used in the cleanup trap
-#shellcheck disable=SC2317
+#shellcheck disable=SC2317,SC2329
 cleanup()
 {
 	mptcp_lib_ns_exit "${ns1}" "${ns2}" "${ns_sbox}"
diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
index ac7ec6f9402376a34602ef1ca6c4822e8dde0ded..ec6a8758819194f2c53791d76ae68e088f188813 100755
--- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
+++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
@@ -32,7 +32,7 @@ ns1=""
 err=$(mktemp)
 
 # This function is used in the cleanup trap
-#shellcheck disable=SC2317
+#shellcheck disable=SC2317,SC2329
 cleanup()
 {
 	rm -f "${err}"
@@ -70,8 +70,9 @@ format_endpoints() {
 	mptcp_lib_pm_nl_format_endpoints "${@}"
 }
 
+# This function is invoked indirectly
+#shellcheck disable=SC2317,SC2329
 get_endpoint() {
-	# shellcheck disable=SC2317 # invoked indirectly
 	mptcp_lib_pm_nl_get_endpoint "${ns1}" "${@}"
 }
 
diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index 2329c2f8519b7c336e9f90a705dfa7588207a543..1903e8e84a315175e2ffd620dd7b4e94dbf25dfb 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -35,7 +35,7 @@ usage() {
 }
 
 # This function is used in the cleanup trap
-#shellcheck disable=SC2317
+#shellcheck disable=SC2317,SC2329
 cleanup()
 {
 	rm -f "$cout" "$sout"
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 333064b0b5ac03ae003417d2070f3c08f94743ed..970c329735ff14f87f0048ba0030dc7edaaa86bc 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -94,7 +94,7 @@ test_fail()
 }
 
 # This function is used in the cleanup trap
-#shellcheck disable=SC2317
+#shellcheck disable=SC2317,SC2329
 cleanup()
 {
 	print_title "Cleanup"

-- 
2.50.1

[PATCH RFC mptcp-net 7/7] mptcp: pm: nl: announce deny-join-id0 attribute

[Matthieu Baerts (NGI0)]

During the connection establishment, a peer can tell the other that it
cannot establish new subflows to the initial IP address and port by
setting the 'C' flag [1]. Doing so makes sense when the sender is behind
a strict NAT, operating behind a legacy Layer 4 load balancer, or using
anycast IP address for example.

When this 'C' flag is set, the path-managers must then not try to
establish new subflow to the other peer's initial IP address and port.
The in-kernel PM has access to this info, but the userspace PM didn't.

When a new connection is created and established, the Netlink events
will now contain a new deny-join-id0 attribute. When set to 1, it means
no other subflow to the initial IP address and port -- which is part of
the event -- can be established.

Link: https://datatracker.ietf.org/doc/html/rfc8684#section-3.1-20.6 [1]
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/532
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
This patch is marked as an RFC, just so we can agree on the attribute
type: should we use a new dedicated attribute, or start using flags? It
even looks like we have defined a "flags" attribute, but it is currently
not used. Using "flags" would allow us to save some bytes, and in this
case, 'server-side' should have used "flags" too. The problem with the
flags is that the userspace cannot tell if a new flag is unset because
the kernel doesn't support it, or because it doesn't need to be set. But
is it really an issue? If the kernel doesn't support it, the userspace
will probably not know what to do anyway. So fine to use flags? Or not
needed, because it is unlikely to add new attributes later on?

If yes, I suggest:
- switching 'flags' to u32 instead of u16 because it doesn't change
  anything, and it is currently not used
- deprecating the 'server-side' attribute, and adding it in the flags
  (but keeping it for the moment, maybe we can remove it in a few
  versions?)
- adding deny-join-id0 as a flag: MPTCP_PM_EVENT_FLAG_DENY_JOIN_ID0

If no, we can use what is proposed here.

In any cases, I would like to add a test. I initially added one in
userspace_pm.sh, but due to "mptcp: pm: userspace: respect deny_join_id0
attr", it is no longer possible to set allow_join_initial_addr_port
sysctl knob to 0 there, as the value is no longer ignored by the
userspace PM. Probably best to add a test in mptcp_join.sh.
---
 Documentation/netlink/specs/mptcp_pm.yaml | 7 +++++--
 include/uapi/linux/mptcp_pm.h             | 5 +++--
 net/mptcp/pm_netlink.c                    | 4 ++++
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/Documentation/netlink/specs/mptcp_pm.yaml b/Documentation/netlink/specs/mptcp_pm.yaml
index d15335684ec3d6256505f2b3887ce5818eb57462..0b53d8b5b4524b6026009cfa4510e7a6e141acf1 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.
+          dport, server-side, deny-join-id0.
       -
         name: established
         doc: >-
           A MPTCP connection is established (can start new subflows).
           Attributes: token, family, saddr4 | saddr6, daddr4 | daddr6, sport,
-          dport, server-side.
+          dport, server-side, deny-join-id0.
       -
         name: closed
         doc: >-
@@ -266,6 +266,9 @@ attribute-sets:
       -
         name: server-side
         type: u8
+      -
+        name: deny-join-id0
+        type: u8
 
 operations:
   list:
diff --git a/include/uapi/linux/mptcp_pm.h b/include/uapi/linux/mptcp_pm.h
index 6ac84b2f636ca22935c191c645449fb62b673899..6c751c488e51d6ab711607189041d5e2ea222e4e 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.
+ *   sport, dport, server-side, deny-join-id0.
  * @MPTCP_EVENT_ESTABLISHED: A MPTCP connection is established (can start new
  *   subflows). Attributes: token, family, saddr4 | saddr6, daddr4 | daddr6,
- *   sport, dport, server-side.
+ *   sport, dport, server-side, deny-join-id0.
  * @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].
@@ -126,6 +126,7 @@ enum mptcp_event_attr {
 	MPTCP_ATTR_RESET_REASON,
 	MPTCP_ATTR_RESET_FLAGS,
 	MPTCP_ATTR_SERVER_SIDE,
+	MPTCP_ATTR_DENY_JOIN_ID0,
 
 	__MPTCP_ATTR_MAX
 };
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 50aaf259959aeaf36e7ab954c6f7957eaf2bc390..93455b63ef80395ff19f40d84cd28438a3578b98 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -415,6 +415,10 @@ static int mptcp_event_created(struct sk_buff *skb,
 	if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, READ_ONCE(msk->pm.server_side)))
 		return -EMSGSIZE;
 
+	if (nla_put_u8(skb, MPTCP_ATTR_DENY_JOIN_ID0,
+		       READ_ONCE(msk->pm.remote_deny_join_id0)))
+		return -EMSGSIZE;
+
 	return mptcp_event_add_subflow(skb, ssk);
 }
 

-- 
2.50.1

Re: [PATCH mptcp-net 2/7] mptcp: pm: userspace: respect deny_join_id0 attr

[Christoph Paasch]

On Fri, Aug 29, 2025 at 1:33 PM Matthieu Baerts (NGI0)
<matttbe@kernel.org> wrote:
>
> During the connection establishment, a peer can tell the other that it
> cannot establish new subflows to the initial IP address and port by
> setting the 'C' flag [1]. The RFC8684 is strict about that:
>
>   (...) therefore the receiver MUST NOT try to open any additional
>   subflows toward this address and port.
>
> It is then important not to let the userspace PM establishing such
> subflows, and return an error (ECONNREFUSED) if it tries to do so.
>
> Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow establishment")
> Link: https://datatracker.ietf.org/doc/html/rfc8684\#section-3.1-20.6 [1]
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  net/mptcp/pm_userspace.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 1911fe1799fa38a53381247a830a9a0daf1c4492..9968dc9a8b45112114953f66848ea22c971136d6 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -391,6 +391,12 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
>         if (err < 0)
>                 goto create_err;
>
> +       if (READ_ONCE(msk->pm.remote_deny_join_id0) && addr_r.id == 0) {

If userspace wants to, it could still work around that by still using
the initial subflow's IP-address and just use a non-zero address-ID.

Is that really a concern ? Sure, the IETF-draft says "MUST NOT", and
so, shouldn't we leave it up to the userspace PM to make that decision
?


Christoph

> +               GENL_SET_ERR_MSG(info, "deny join id0");
> +               err = -ECONNREFUSED;
> +               goto create_err;
> +       }
> +
>         if (!mptcp_pm_addr_families_match(sk, &entry.addr, &addr_r)) {
>                 GENL_SET_ERR_MSG(info, "families mismatch");
>                 err = -EINVAL;
>
> --
> 2.50.1
>
>

Re: [PATCH mptcp-net 0/7] mptcp: pm: nl: announce deny-join-id0 attribute

[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_connect 🔴
- KVM Validation: debug: Unstable: 3 failed test(s): packetdrill_dss selftest_mptcp_connect_checksum selftest_mptcp_join 🔴
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/17334080753

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/1fe573a443a0
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=997044


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-normal

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 (NGI0 Core)

Re: [PATCH mptcp-net 2/7] mptcp: pm: userspace: respect deny_join_id0 attr

[Mat Martineau]

[-- Attachment #1: Type: text/plain, Size: 2232 bytes --]

On Fri, 29 Aug 2025, Christoph Paasch wrote:

> On Fri, Aug 29, 2025 at 1:33 PM Matthieu Baerts (NGI0)
> <matttbe@kernel.org> wrote:
>>
>> During the connection establishment, a peer can tell the other that it
>> cannot establish new subflows to the initial IP address and port by
>> setting the 'C' flag [1]. The RFC8684 is strict about that:
>>
>>   (...) therefore the receiver MUST NOT try to open any additional
>>   subflows toward this address and port.
>>
>> It is then important not to let the userspace PM establishing such
>> subflows, and return an error (ECONNREFUSED) if it tries to do so.
>>
>> Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow establishment")
>> Link: https://datatracker.ietf.org/doc/html/rfc8684\#section-3.1-20.6 [1]
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>>  net/mptcp/pm_userspace.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
>> index 1911fe1799fa38a53381247a830a9a0daf1c4492..9968dc9a8b45112114953f66848ea22c971136d6 100644
>> --- a/net/mptcp/pm_userspace.c
>> +++ b/net/mptcp/pm_userspace.c
>> @@ -391,6 +391,12 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
>>         if (err < 0)
>>                 goto create_err;
>>
>> +       if (READ_ONCE(msk->pm.remote_deny_join_id0) && addr_r.id == 0) {
>
> If userspace wants to, it could still work around that by still using
> the initial subflow's IP-address and just use a non-zero address-ID.
>
> Is that really a concern ? Sure, the IETF-draft says "MUST NOT", and
> so, shouldn't we leave it up to the userspace PM to make that decision
> ?
>

I concur with Christoph on this one, I think the PM (userspace or kernel) 
is responsible for this aspect of the protocol so it doesn't need to be 
double-checked here.

- Mat

>
>> +               GENL_SET_ERR_MSG(info, "deny join id0");
>> +               err = -ECONNREFUSED;
>> +               goto create_err;
>> +       }
>> +
>>         if (!mptcp_pm_addr_families_match(sk, &entry.addr, &addr_r)) {
>>                 GENL_SET_ERR_MSG(info, "families mismatch");
>>                 err = -EINVAL;
>>
>> --
>> 2.50.1
>>
>>
>
>

Re: [PATCH mptcp-net 2/7] mptcp: pm: userspace: respect deny_join_id0 attr

[Matthieu Baerts]

Hi Christoph, Mat,

On 30/08/2025 02:58, Mat Martineau wrote:
> On Fri, 29 Aug 2025, Christoph Paasch wrote:
> 
>> On Fri, Aug 29, 2025 at 1:33 PM Matthieu Baerts (NGI0)
>> <matttbe@kernel.org> wrote:
>>>
>>> During the connection establishment, a peer can tell the other that it
>>> cannot establish new subflows to the initial IP address and port by
>>> setting the 'C' flag [1]. The RFC8684 is strict about that:
>>>
>>>   (...) therefore the receiver MUST NOT try to open any additional
>>>   subflows toward this address and port.
>>>
>>> It is then important not to let the userspace PM establishing such
>>> subflows, and return an error (ECONNREFUSED) if it tries to do so.
>>>
>>> Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow
>>> establishment")
>>> Link: https://datatracker.ietf.org/doc/html/rfc8684\#section-3.1-20.6
>>> [1]
>>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>>> ---
>>>  net/mptcp/pm_userspace.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
>>> index
>>> 1911fe1799fa38a53381247a830a9a0daf1c4492..9968dc9a8b45112114953f66848ea22c971136d6 100644
>>> --- a/net/mptcp/pm_userspace.c
>>> +++ b/net/mptcp/pm_userspace.c
>>> @@ -391,6 +391,12 @@ int mptcp_pm_nl_subflow_create_doit(struct
>>> sk_buff *skb, struct genl_info *info)
>>>         if (err < 0)
>>>                 goto create_err;
>>>
>>> +       if (READ_ONCE(msk->pm.remote_deny_join_id0) && addr_r.id == 0) {
>>
>> If userspace wants to, it could still work around that by still using
>> the initial subflow's IP-address and just use a non-zero address-ID.

Good point, I missed that!

>> Is that really a concern ? Sure, the IETF-draft says "MUST NOT", and
>> so, shouldn't we leave it up to the userspace PM to make that decision
>> ?
>>
> 
> I concur with Christoph on this one, I think the PM (userspace or
> kernel) is responsible for this aspect of the protocol so it doesn't
> need to be double-checked here.

Indeed, we could leave it up to the different PMs to make that decision.
Initially, I didn't have this patch, and I had a simple patch setting
the corresponding sysctl to 0 in userspace_pm.sh, just to check the
event was correctly announced. But when writing the last patch's commit
message ("mptcp: pm: nl: announce deny-join-id0 attribute"), I realised
that the main missing feature was a way for the userspace daemon to know
that the C flag was set. We cannot backport patch 7/7, but we can
backport this one, and have a way to know this info on older kernels. I
should have mentioned that in the commit message.

In this case, I think we have a few options:

- Keep this patch, but update the commit message to make it clearer
what's its purpose, see [1]

- Same, but revert it after patch 7/7 on net-next.

- Keep it, but also check for the address and port.

- Drop it.

What would you prefer?

Personally, I think we should keep it, at least for stable, but we don't
need to check for the address and port: even if it is easy to check, it
might be good to let a way for the userspace to still create such subflow.

[1]: The last paragraph in this commit message would then be:

> It is then important to let the userspace daemon know it is not allowed
> to establish such subflows, and return an error (ECONNREFUSED) if it
> tries to do so. Note that the userspace daemon will still be able to
> establish subflows to the initial IP address and port, but by giving a
> different remote ID: that's OK, that's its responsibility ; worst case,
> the MP_JOIN is rejected by the other peer.

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

Re: [PATCH mptcp-net 2/7] mptcp: pm: userspace: respect deny_join_id0 attr

[Christoph Paasch]

On Mon, Sep 1, 2025 at 3:08 AM Matthieu Baerts <matttbe@kernel.org> wrote:
>
> Hi Christoph, Mat,
>
> On 30/08/2025 02:58, Mat Martineau wrote:
> > On Fri, 29 Aug 2025, Christoph Paasch wrote:
> >
> >> On Fri, Aug 29, 2025 at 1:33 PM Matthieu Baerts (NGI0)
> >> <matttbe@kernel.org> wrote:
> >>>
> >>> During the connection establishment, a peer can tell the other that it
> >>> cannot establish new subflows to the initial IP address and port by
> >>> setting the 'C' flag [1]. The RFC8684 is strict about that:
> >>>
> >>>   (...) therefore the receiver MUST NOT try to open any additional
> >>>   subflows toward this address and port.
> >>>
> >>> It is then important not to let the userspace PM establishing such
> >>> subflows, and return an error (ECONNREFUSED) if it tries to do so.
> >>>
> >>> Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow
> >>> establishment")
> >>> Link: https://datatracker.ietf.org/doc/html/rfc8684\#section-3.1-20.6
> >>> [1]
> >>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> >>> ---
> >>>  net/mptcp/pm_userspace.c | 6 ++++++
> >>>  1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> >>> index
> >>> 1911fe1799fa38a53381247a830a9a0daf1c4492..9968dc9a8b45112114953f66848ea22c971136d6 100644
> >>> --- a/net/mptcp/pm_userspace.c
> >>> +++ b/net/mptcp/pm_userspace.c
> >>> @@ -391,6 +391,12 @@ int mptcp_pm_nl_subflow_create_doit(struct
> >>> sk_buff *skb, struct genl_info *info)
> >>>         if (err < 0)
> >>>                 goto create_err;
> >>>
> >>> +       if (READ_ONCE(msk->pm.remote_deny_join_id0) && addr_r.id == 0) {
> >>
> >> If userspace wants to, it could still work around that by still using
> >> the initial subflow's IP-address and just use a non-zero address-ID.
>
> Good point, I missed that!
>
> >> Is that really a concern ? Sure, the IETF-draft says "MUST NOT", and
> >> so, shouldn't we leave it up to the userspace PM to make that decision
> >> ?
> >>
> >
> > I concur with Christoph on this one, I think the PM (userspace or
> > kernel) is responsible for this aspect of the protocol so it doesn't
> > need to be double-checked here.
>
> Indeed, we could leave it up to the different PMs to make that decision.
> Initially, I didn't have this patch, and I had a simple patch setting
> the corresponding sysctl to 0 in userspace_pm.sh, just to check the
> event was correctly announced. But when writing the last patch's commit
> message ("mptcp: pm: nl: announce deny-join-id0 attribute"), I realised
> that the main missing feature was a way for the userspace daemon to know
> that the C flag was set. We cannot backport patch 7/7

Why not ? Yes, it is not typical to backport a new netlink message,
but I think it makes sense here.
It basically means that the fix involves not only the kernel but also
the userspace PMs.

> , but we can
> backport this one, and have a way to know this info on older kernels. I
> should have mentioned that in the commit message.

The problem is that even this here is not complete, right ? A
userspace pm can still attempt to connect to the IP of the initial
subflow simply by using another address-ID.


Christoph

> In this case, I think we have a few options:
>
> - Keep this patch, but update the commit message to make it clearer
> what's its purpose, see [1]
>
> - Same, but revert it after patch 7/7 on net-next.
>
> - Keep it, but also check for the address and port.
>
> - Drop it.
>
> What would you prefer?
>
> Personally, I think we should keep it, at least for stable, but we don't
> need to check for the address and port: even if it is easy to check, it
> might be good to let a way for the userspace to still create such subflow.
>
> [1]: The last paragraph in this commit message would then be:
>
> > It is then important to let the userspace daemon know it is not allowed
> > to establish such subflows, and return an error (ECONNREFUSED) if it
> > tries to do so. Note that the userspace daemon will still be able to
> > establish subflows to the initial IP address and port, but by giving a
> > different remote ID: that's OK, that's its responsibility ; worst case,
> > the MP_JOIN is rejected by the other peer.
>
> Cheers,
> Matt
> --
> Sponsored by the NGI0 Core fund.
>

Re: [PATCH mptcp-net 2/7] mptcp: pm: userspace: respect deny_join_id0 attr

[Matthieu Baerts]

On 02/09/2025 18:43, Christoph Paasch wrote:
> On Mon, Sep 1, 2025 at 3:08 AM Matthieu Baerts <matttbe@kernel.org> wrote:
>>
>> Hi Christoph, Mat,
>>
>> On 30/08/2025 02:58, Mat Martineau wrote:
>>> On Fri, 29 Aug 2025, Christoph Paasch wrote:
>>>
>>>> On Fri, Aug 29, 2025 at 1:33 PM Matthieu Baerts (NGI0)
>>>> <matttbe@kernel.org> wrote:
>>>>>
>>>>> During the connection establishment, a peer can tell the other that it
>>>>> cannot establish new subflows to the initial IP address and port by
>>>>> setting the 'C' flag [1]. The RFC8684 is strict about that:
>>>>>
>>>>>   (...) therefore the receiver MUST NOT try to open any additional
>>>>>   subflows toward this address and port.
>>>>>
>>>>> It is then important not to let the userspace PM establishing such
>>>>> subflows, and return an error (ECONNREFUSED) if it tries to do so.
>>>>>
>>>>> Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow
>>>>> establishment")
>>>>> Link: https://datatracker.ietf.org/doc/html/rfc8684\#section-3.1-20.6
>>>>> [1]
>>>>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>>>>> ---
>>>>>  net/mptcp/pm_userspace.c | 6 ++++++
>>>>>  1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
>>>>> index
>>>>> 1911fe1799fa38a53381247a830a9a0daf1c4492..9968dc9a8b45112114953f66848ea22c971136d6 100644
>>>>> --- a/net/mptcp/pm_userspace.c
>>>>> +++ b/net/mptcp/pm_userspace.c
>>>>> @@ -391,6 +391,12 @@ int mptcp_pm_nl_subflow_create_doit(struct
>>>>> sk_buff *skb, struct genl_info *info)
>>>>>         if (err < 0)
>>>>>                 goto create_err;
>>>>>
>>>>> +       if (READ_ONCE(msk->pm.remote_deny_join_id0) && addr_r.id == 0) {
>>>>
>>>> If userspace wants to, it could still work around that by still using
>>>> the initial subflow's IP-address and just use a non-zero address-ID.
>>
>> Good point, I missed that!
>>
>>>> Is that really a concern ? Sure, the IETF-draft says "MUST NOT", and
>>>> so, shouldn't we leave it up to the userspace PM to make that decision
>>>> ?
>>>>
>>>
>>> I concur with Christoph on this one, I think the PM (userspace or
>>> kernel) is responsible for this aspect of the protocol so it doesn't
>>> need to be double-checked here.
>>
>> Indeed, we could leave it up to the different PMs to make that decision.
>> Initially, I didn't have this patch, and I had a simple patch setting
>> the corresponding sysctl to 0 in userspace_pm.sh, just to check the
>> event was correctly announced. But when writing the last patch's commit
>> message ("mptcp: pm: nl: announce deny-join-id0 attribute"), I realised
>> that the main missing feature was a way for the userspace daemon to know
>> that the C flag was set. We cannot backport patch 7/7
> 
> Why not ? Yes, it is not typical to backport a new netlink message,
> but I think it makes sense here.
> It basically means that the fix involves not only the kernel but also
> the userspace PMs.

Yes, I guess we could backport it.

I still don't know if we should use the (existing but not used) "flags"
attribute, or a new dedicated one.

>> , but we can
>> backport this one, and have a way to know this info on older kernels. I
>> should have mentioned that in the commit message.
> 
> The problem is that even this here is not complete, right ? A
> userspace pm can still attempt to connect to the IP of the initial
> subflow simply by using another address-ID.

Yes, but that's not an issue for me: that's because the userspace PM
didn't use the expected ID, so it is the userspace PM daemon
responsibility :)

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