[PATCH mptcp-net v4 0/8] mptcp: pm: userspace: properly deal with the ID0 case

[PATCH mptcp-net v4 0/8] mptcp: pm: userspace: properly deal with the ID0 case

[Matthieu Baerts (NGI0)]

In MPTCP, the local address and port used by the initial subflow has the
ID "0". It means that when this address and port are used for some
operations -- e.g. creating a new subflow -- they should be linked to
the ID0, and no other addresses and ports can get this special ID while
the initial IP address and port is used.

Instead of dealing with the ID0 case as an exception, add it to the
local addr list, and deal with it like the others, with minor
exceptions. That way, it seems easier to maintain instead of adding new
exceptions at a few places, at the cost of a few more bytes, which seems
OK in this mode.

The first patch modifies add the initial address to the list, and the
others remove exceptions, and validate that in the selftests.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Changes in v4:
- patch 2: avoid two iterations (Geliang)
- patch 3: move the declaration of the new helper to one line (Geliang)
- Link to v3: https://patch.msgid.link/20260807-mptcp-pm-userspace-id0-case-v3-0-de9088549924@kernel.org

Changes in v3:
- patch 1: split exceptions + set msk port to 0 like before this series
- patches 2-3: new: from patch 1 + fix
- patch 6: use new is_init_id0 helper
- patch 8: avoid conflicts with WIP BPF MPTCP PM patches
- Link to v2: https://patch.msgid.link/20260806-mptcp-pm-userspace-id0-case-v2-0-fea1808e1c57@kernel.org

Changes in v2:
- patch 1: commit msg + local_addr_used and needs_id exceptions
- patch 2: commit msg
- Link to v1: https://patch.msgid.link/20260727-mptcp-pm-userspace-id0-case-v1-0-9877f02a9bae@kernel.org

---
Matthieu Baerts (NGI0) (8):
      mptcp: pm: userspace: properly handle the ID0 case
      mptcp: pm: userspace: lookup: match port in priority
      mptcp: pm: userspace: ID0 is not part of local_addr_used
      mptcp: pm: userspace: allow announcing ID0 addr
      mptcp: pm: userspace: no ID0 exception for RM_ADDR
      mptcp: pm: userspace: don't dump initial ID0
      selftests: mptcp: join: new ID0 subflow from the right IP
      mptcp: pm: restrict in-kernel worker actions to this PM

 net/mptcp/pm.c                                  |  8 ++-
 net/mptcp/pm_userspace.c                        | 92 ++++++++++++-------------
 net/mptcp/protocol.h                            |  1 +
 tools/testing/selftests/net/mptcp/mptcp_join.sh |  7 +-
 4 files changed, 55 insertions(+), 53 deletions(-)
---
base-commit: bcc5638fabf418e763621a432840cf55287e604d
change-id: 20260724-mptcp-pm-userspace-id0-case-f074f64466c8

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

[PATCH mptcp-net v4 1/8] mptcp: pm: userspace: properly handle the ID0 case

[Matthieu Baerts (NGI0)]

In MPTCP, the local address and port used by the initial subflow has the
ID "0". It means that when this address and port are used for some
operations -- e.g. creating a new subflow -- they should be linked to
the ID0, and no other addresses and ports can get this special ID while
the initial IP address and port is used.

So far, the ID0 case was handled as an exception: each operation dealing
with the ID0 had to be handled differently. Except that this was done in
some places like removing the ID0, but not everywhere the list of local
addresses was iterated. This way of handling the ID0 is prone to bugs
and harder to maintain. Instead, the initial local address corresponding
to ID0 can be added to the list when a connection is created, and the
number of exceptions can be dramatically reduced, handling this case
like any others with existing local address. The existing exceptions are
going to be removed in the following patches.

The main downside of this is that each connection will now have one
allocated entry added the list, possibly one more than before. But that
seems OK to do that with the userspace PM where the path management is
done per connection, with many Netlink messages sent back and forth.
Adding a few more bytes per connections on such setup seems acceptable.

Fixes: 4638de5aefe5 ("mptcp: handle local addrs announced by userspace PMs")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
sashiko: check the next patches before pointing at existing issues.
v2:
 - List new exceptions in the commit message.
 - Handle local_addr_used with the ID0 case properly. (Sashiko)
 - Handle needs_id where the addrs don't match, but the IDs (0) does. (S)
v3:
 - Split local_addr_used case: that's fixing another commit.
 - Set port to 0 (msk) instead of adding exceptions in lookups.
---
 net/mptcp/pm.c           |  4 ++++
 net/mptcp/pm_userspace.c | 20 ++++++++++++++++++++
 net/mptcp/protocol.h     |  1 +
 3 files changed, 25 insertions(+)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index d7c5b50b34cc..e9a538296200 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -551,6 +551,10 @@ void mptcp_pm_new_connection(struct mptcp_sock *msk, const struct sock *ssk, int
 	pr_debug("msk=%p, token=%u side=%d\n", msk, READ_ONCE(msk->token), server_side);
 
 	WRITE_ONCE(pm->server_side, server_side);
+
+	if (mptcp_pm_is_userspace(msk))
+		mptcp_pm_userspace_created(msk, ssk);
+
 	mptcp_event(MPTCP_EVENT_CREATED, msk, ssk, GFP_ATOMIC);
 }
 
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index f723a134356f..45faee894ac3 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -707,6 +707,26 @@ int mptcp_userspace_pm_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
 	return ret;
 }
 
+/* Add the initial local address (ID0) to the local list: easier that way */
+void mptcp_pm_userspace_created(struct mptcp_sock *msk, const struct sock *ssk)
+{
+	struct mptcp_pm_addr_entry *entry;
+
+	entry = sock_kmalloc((struct sock *)msk, sizeof(*entry), GFP_ATOMIC);
+	/* Fine not to handle the ID0 case in memory pressure */
+	if (!entry)
+		return;
+
+	memset(entry, 0, sizeof(*entry));
+	mptcp_local_address((struct sock_common *)ssk, &entry->addr);
+	entry->addr.port = 0; /* msk port */
+
+	spin_lock_bh(&msk->pm.lock);
+	list_add_tail_rcu(&entry->list, &msk->pm.userspace_pm_local_addr_list);
+	/* The initial address ID doesn't increment local_addr_used */
+	spin_unlock_bh(&msk->pm.lock);
+}
+
 static struct mptcp_pm_ops mptcp_pm_userspace = {
 	.get_local_id		= mptcp_pm_userspace_get_local_id,
 	.get_priority		= mptcp_pm_userspace_get_priority,
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 3be60da12924..8c219fabfc9f 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1260,6 +1260,7 @@ void __init mptcp_pm_userspace_register(void);
 void __init mptcp_pm_nl_init(void);
 void mptcp_pm_worker(struct mptcp_sock *msk);
 void __mptcp_pm_kernel_worker(struct mptcp_sock *msk);
+void mptcp_pm_userspace_created(struct mptcp_sock *msk, const struct sock *ssk);
 u8 mptcp_pm_get_endp_signal_max(const struct mptcp_sock *msk);
 u8 mptcp_pm_get_endp_subflow_max(const struct mptcp_sock *msk);
 u8 mptcp_pm_get_endp_laminar_max(const struct mptcp_sock *msk);

-- 
2.55.0

[PATCH mptcp-net v4 2/8] mptcp: pm: userspace: lookup: match port in priority

[Matthieu Baerts (NGI0)]

In the local address list, there can be entries with the port set to 0
-- corresponding to the source port used by the initial subflow -- and
others with a specific port.

When performing a lookup, it is important to compare the ports to pick
the right entry: when a specific port is given, then try to match it
first. If no match is found, try to find entries with the port set to 0.

We need to handle the case where a subflow got created without forcing
the source port: we used the same ID as another entry, and we still need
to match it, even if here the source port is likely not the same as the
msk one (or a subflow attached to the same ID), but that's OK.

Fixes: 24430f8bf516 ("mptcp: add address into userspace pm list")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
sashiko: check the next patches before pointing at existing issues.
- v3: new (Sashiko)
- v4: avoid 2 loops (Geliang)
---
 net/mptcp/pm_userspace.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 45faee894ac3..68a02b15a4c5 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -31,13 +31,19 @@ static struct mptcp_pm_addr_entry *
 mptcp_userspace_pm_lookup_addr(struct mptcp_sock *msk,
 			       const struct mptcp_addr_info *addr)
 {
-	struct mptcp_pm_addr_entry *entry;
+	struct mptcp_pm_addr_entry *entry, *wildcard = NULL;
 
 	mptcp_for_each_userspace_pm_addr(msk, entry) {
-		if (mptcp_addresses_equal(&entry->addr, addr, false))
-			return entry;
+		if (mptcp_addresses_equal(&entry->addr, addr, false)) {
+			if (addr->port == entry->addr.port)
+				return entry;
+
+			if (!wildcard && (!addr->port || !entry->addr.port))
+				wildcard = entry;
+		}
 	}
-	return NULL;
+
+	return wildcard;
 }
 
 static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,

-- 
2.55.0

[PATCH mptcp-net v4 3/8] mptcp: pm: userspace: ID0 is not part of local_addr_used

[Matthieu Baerts (NGI0)]

The PM's local_addr_used counter doesn't take into account the ID0,
similar to what is done with the in-kernel PM.

When deleting a local address, only decrement the counter if it is not
linked to the (initial) ID0. Similarly, do not increment it when
re-adding the exact same entry.

Fixes: 77e4b94a3de6 ("mptcp: update userspace pm infos")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
sashiko: check the next patches before pointing at existing issues.
v3:
 - split from patch 1.
 - do the same check when incrementing it to avoid a "leak". (Sashiko)
 - use a new dedicated helper, clearer.
v4: move to one line when declaring is_init_id0. (Geliang)
---
 net/mptcp/pm_userspace.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 68a02b15a4c5..74969b86fb81 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -27,6 +27,11 @@ void mptcp_userspace_pm_free_local_addr_list(struct mptcp_sock *msk)
 	}
 }
 
+static bool is_init_id0(struct mptcp_pm_addr_entry *entry)
+{
+	return entry->addr.id == 0 && entry->addr.port == 0;
+}
+
 static struct mptcp_pm_addr_entry *
 mptcp_userspace_pm_lookup_addr(struct mptcp_sock *msk,
 			       const struct mptcp_addr_info *addr)
@@ -99,7 +104,9 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
 
 		e->addr.id = id;
 		list_add_tail_rcu(&e->list, &msk->pm.userspace_pm_local_addr_list);
-		msk->pm.local_addr_used++;
+		/* Just to avoid this counter not to decrease when deleted */
+		if (!is_init_id0(e))
+			msk->pm.local_addr_used++;
 		ret = e->addr.id;
 	} else if (addr_match && id_match) {
 		ret = entry->addr.id;
@@ -125,12 +132,15 @@ static int mptcp_userspace_pm_delete_local_addr(struct mptcp_sock *msk,
 	if (!entry)
 		return -EINVAL;
 
+	/* The initial address ID doesn't increment local_addr_used */
+	if (!is_init_id0(entry))
+		msk->pm.local_addr_used--;
+
 	/* TODO: a refcount is needed because the entry can
 	 * be used multiple times (e.g. fullmesh mode).
 	 */
 	list_del_rcu(&entry->list);
 	sock_kfree_s(sk, entry, sizeof(*entry));
-	msk->pm.local_addr_used--;
 	return 0;
 }
 

-- 
2.55.0

[PATCH mptcp-net v4 4/8] mptcp: pm: userspace: allow announcing ID0 addr

[Matthieu Baerts (NGI0)]

It is valid to announce an address with the ID0, the in-kernel PM allows
to do that when re-announcing the initial IP address after having been
deleted.

So no need to have such exception. If the ID is set to 0, but the
address doesn't match with the existing one, an error will be returned
by mptcp_userspace_pm_append_new_local_addr().

Note that once removed, the ID0 entry could be replaced by another IP
address. The RFC8684 doesn't specify this specific case with ID0, but it
says [1]: "A host wishing to replace an existing Address ID MUST first
remove the existing one". In these unclear conditions, better to let the
responsibility to the userspace daemon to pick the same IP or another
one, similar to what was in place before.

Fixes: 9ab4807c84a4 ("mptcp: netlink: Add MPTCP_PM_CMD_ANNOUNCE")
Link: https://datatracker.ietf.org/doc/html/rfc8684#section-3.4.1-13 [1]
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
sashiko: check the next patches before pointing at existing issues.
v2: Add note about re-using the ID0 with another address (Sashiko)
---
 net/mptcp/pm_userspace.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 74969b86fb81..66dbf67fd6d8 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -239,12 +239,6 @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 	if (err < 0)
 		goto announce_err;
 
-	if (addr_val.addr.id == 0) {
-		NL_SET_ERR_MSG_ATTR(info->extack, addr, "invalid addr id");
-		err = -EINVAL;
-		goto announce_err;
-	}
-
 	if (!(addr_val.flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
 		NL_SET_ERR_MSG_ATTR(info->extack, addr, "invalid addr flags");
 		err = -EINVAL;

-- 
2.55.0

[PATCH mptcp-net v4 5/8] mptcp: pm: userspace: no ID0 exception for RM_ADDR

[Matthieu Baerts (NGI0)]

After the first patch of this series, the initial ID0 address is present
in the local addr list when the connection has been created.

Then, no need to have an exception to delete ID0, this can be done like
with other IDs.

Fixes: 84c531f54ad9 ("mptcp: userspace pm send RM_ADDR for ID 0")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
sashiko: check the next patches before pointing at existing issues.
---
 net/mptcp/pm_userspace.c | 36 ------------------------------------
 1 file changed, 36 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 66dbf67fd6d8..0f6400bc3bf8 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -270,37 +270,6 @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 	return err;
 }
 
-static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk)
-{
-	struct mptcp_rm_list list = { .nr = 0 };
-	struct mptcp_subflow_context *subflow;
-	struct sock *sk = (struct sock *)msk;
-	bool has_id_0 = false;
-	int err = -EINVAL;
-
-	lock_sock(sk);
-	mptcp_for_each_subflow(msk, subflow) {
-		if (READ_ONCE(subflow->local_id) == 0) {
-			has_id_0 = true;
-			break;
-		}
-	}
-	if (!has_id_0)
-		goto remove_err;
-
-	list.ids[list.nr++] = 0;
-
-	spin_lock_bh(&msk->pm.lock);
-	mptcp_pm_remove_addr(msk, &list);
-	spin_unlock_bh(&msk->pm.lock);
-
-	err = 0;
-
-remove_err:
-	release_sock(sk);
-	return err;
-}
-
 static void
 mptcp_userspace_pm_remove_addr_entry(struct mptcp_sock *msk,
 				     struct mptcp_pm_addr_entry *entry)
@@ -343,11 +312,6 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 
 	sk = (struct sock *)msk;
 
-	if (id_val == 0) {
-		err = mptcp_userspace_pm_remove_id_zero_address(msk);
-		goto out;
-	}
-
 	lock_sock(sk);
 
 	spin_lock_bh(&msk->pm.lock);

-- 
2.55.0

[PATCH mptcp-net v4 6/8] mptcp: pm: userspace: don't dump initial ID0

[Matthieu Baerts (NGI0)]

After the first patch of this series, the initial ID0 address is present
in the local addr list when the connection has been created. Not to
change the previous behaviour, but also to have a similar behaviour than
what is done with the in-kernel PM, the initial ID0 address is not
dumped with the rest.

If the address is removed, then re-added later with a new subflow, it
can be dumped.

Fixes: 34e74a5cf3b7 ("mptcp: implement mptcp_userspace_pm_dump_addr")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
v3:
 - use is_init_id0() helper, to check the port and not the flags
   (prev workaround, but doesn't work if modified later)
---
 net/mptcp/pm_userspace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 0f6400bc3bf8..282d7f57c32a 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -637,7 +637,7 @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
 	lock_sock(sk);
 	spin_lock_bh(&msk->pm.lock);
 	mptcp_for_each_userspace_pm_addr(msk, entry) {
-		if (test_bit(entry->addr.id, bitmap->map))
+		if (is_init_id0(entry) || test_bit(entry->addr.id, bitmap->map))
 			continue;
 
 		if (mptcp_pm_genl_fill_addr(msg, cb, entry) < 0)

-- 
2.55.0

[PATCH mptcp-net v4 7/8] selftests: mptcp: join: new ID0 subflow from the right IP

[Matthieu Baerts (NGI0)]

In MPTCP, the local address and port used by the initial subflow has the
ID "0". It means that when this address and port are used for some
operations -- e.g. creating a new subflow -- they should be linked to
the ID0, and no other addresses and ports can get this special ID while
the initial IP address and port is used.

Hence, this is not correct here to try to create a subflow with the ID
"0", but from a different IP address. With the first patch of this
series, this action now returns an error.

Instead of simply using the right IP address, continue to also use the
wrong one to check that the kernel is correctly not allowing the
creation of an ID0 subflow with the wrong IP address.

While at it, fix the dumped address: the initial address is not supposed
to be dumped, similar to what is being done with the in-kernel PM. So no
"extra" addresses should be printed there.

Fixes: b2e2248f365a ("selftests: mptcp: userspace pm create id 0 subflow")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index c3ccce96b45b..190ab48acbc6 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -4214,10 +4214,11 @@ userspace_tests()
 		wait_event ns2 MPTCP_LIB_EVENT_ESTABLISHED 1
 		chk_mptcp_info subflows 0 subflows 0
 		chk_subflows_total 1 1
-		userspace_pm_add_sf $ns2 10.0.3.2 0
+		# from an IP not linked to ID0: failure expected, no new MPJ
+		userspace_pm_add_sf $ns2 10.0.3.2 0 2>/dev/null
+		userspace_pm_add_sf $ns2 10.0.1.2 0
 		wait_event ns2 MPTCP_LIB_EVENT_SUB_ESTABLISHED 1
-		userspace_pm_chk_dump_addr "${ns2}" \
-			"id 0 flags subflow 10.0.3.2" "id 0 subflow"
+		userspace_pm_chk_dump_addr "${ns2}" "" "id 0 subflow"
 		chk_join_nr 1 1 1
 		chk_mptcp_info subflows 1 subflows 1
 		chk_subflows_total 2 2

-- 
2.55.0

[PATCH mptcp-net v4 8/8] mptcp: pm: restrict in-kernel worker actions to this PM

[Matthieu Baerts (NGI0)]

No need to check them for the userspace PM.

Fixes: a49eb8ae95b8 ("mptcp: pm: worker: split in-kernel and common tasks")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
v3: use !mptcp_pm_is_userspace(), to handle WIP BPF MPTCP PM
---
 net/mptcp/pm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index e9a538296200..e38fe835abc0 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -1154,7 +1154,9 @@ void mptcp_pm_worker(struct mptcp_sock *msk)
 		pm->status &= ~BIT(MPTCP_PM_RM_ADDR_RECEIVED);
 		mptcp_pm_rm_addr_recv(msk);
 	}
-	__mptcp_pm_kernel_worker(msk);
+
+	if (!mptcp_pm_is_userspace(msk))
+		__mptcp_pm_kernel_worker(msk);
 
 	spin_unlock_bh(&msk->pm.lock);
 }

-- 
2.55.0

Re: [PATCH mptcp-net v4 1/8] mptcp: pm: userspace: properly handle the ID0 case

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mptcp: pm: userspace: ID 0 entry breaks mptcp_pm_userspace_get_local_id()
--

commit 626a00aaeaa0d778f8ba3b1e9d1ca84367bbdac3
Author: Matthieu Baerts (NGI0) <matttbe@kernel.org>

mptcp: pm: userspace: properly handle the ID0 case

This commit adds the initial subflow's local address (with ID 0 and port 0)
to the userspace path manager's local address list when a connection is
created so that ID 0 can be handled uniformly with other local addresses.

> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index f723a134356f3..45faee894ac3d 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -707,6 +707,26 @@ int mptcp_userspace_pm_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
>  	return ret;
>  }
>  
> +/* Add the initial local address (ID0) to the local list: easier that way */
> +void mptcp_pm_userspace_created(struct mptcp_sock *msk, const struct sock *ssk)
> +{
> +	struct mptcp_pm_addr_entry *entry;
> +
> +	entry = sock_kmalloc((struct sock *)msk, sizeof(*entry), GFP_ATOMIC);
> +	/* Fine not to handle the ID0 case in memory pressure */
> +	if (!entry)
> +		return;
> +
> +	memset(entry, 0, sizeof(*entry));
> +	mptcp_local_address((struct sock_common *)ssk, &entry->addr);
> +	entry->addr.port = 0; /* msk port */
> +
> +	spin_lock_bh(&msk->pm.lock);
> +	list_add_tail_rcu(&entry->list, &msk->pm.userspace_pm_local_addr_list);
> +	/* The initial address ID doesn't increment local_addr_used */
> +	spin_unlock_bh(&msk->pm.lock);
> +}

[Severity: High]
Does adding an entry with entry->addr.id == 0 to
msk->pm.userspace_pm_local_addr_list cause a regression in
mptcp_pm_userspace_get_local_id() when allocating a local ID for a new,
unannounced address?

When mptcp_pm_get_local_id() -> mptcp_pm_userspace_get_local_id() is called
for a subflow whose local IP address differs from the initial subflow's IP
address and is not yet in userspace_pm_local_addr_list, skc->addr.id is 0 and
it calls mptcp_userspace_pm_append_new_local_addr(msk, skc, true):

net/mptcp/pm_userspace.c:mptcp_pm_userspace_get_local_id() {
	...
	if (skc->addr.port == msk_sport)
		skc->addr.port = 0;

	return mptcp_userspace_pm_append_new_local_addr(msk, skc, true);
}

Inside mptcp_userspace_pm_append_new_local_addr(), the loop iterates over
userspace_pm_local_addr_list and encounters the initial ID 0 entry added by
mptcp_pm_userspace_created():

net/mptcp/pm_userspace.c:mptcp_userspace_pm_append_new_local_addr() {
	...
	mptcp_for_each_userspace_pm_addr(msk, e) {
		addr_match = mptcp_addresses_equal(&e->addr, &entry->addr, true);
		if (addr_match && entry->addr.id == 0 && needs_id)
			entry->addr.id = e->addr.id;
		id_match = (e->addr.id == entry->addr.id);
		if (addr_match || id_match)
			break;
		__set_bit(e->addr.id, id_bitmap);
	}

	if (!addr_match && !id_match) {
		...
	} else if (addr_match && id_match) {
		ret = entry->addr.id;
	}

append_err:
	spin_unlock_bh(&msk->pm.lock);
	return ret;
}

Because the new address does not match the initial subflow's address,
addr_match is false and entry->addr.id remains 0. However, e->addr.id is
also 0 on the initial entry, so id_match becomes true and the loop breaks
with addr_match == false and id_match == true, causing the function to
return -EINVAL instead of assigning a new ID from id_bitmap.

Although the v2 changelog notes mention handling needs_id where the addresses
do not match but the IDs (0) do, that check does not appear to be present in
this commit or later in the series.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-mptcp-pm-userspace-id0-case-v4-0-555066fa4292@kernel.org?part=1

Re: [PATCH mptcp-net v4 6/8] mptcp: pm: userspace: don't dump initial ID0

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mptcp: pm: userspace: is_init_id0() skips re-added ID 0 without port
--

commit 06caddae470be561eadf1499e52b7564e7ab99b8
Author: Matthieu Baerts (NGI0) <matttbe@kernel.org>

mptcp: pm: userspace: don't dump initial ID0

This patch skips the initial ID 0 address entry in
mptcp_userspace_pm_dump_addr() by checking is_init_id0(entry) so that it is
not dumped when the connection is first created, while intending to allow it
to be dumped if removed and re-added later.

> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 0f6400bc3bf8e..282d7f57c32ab 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -637,7 +637,7 @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
>  	lock_sock(sk);
>  	spin_lock_bh(&msk->pm.lock);
>  	mptcp_for_each_userspace_pm_addr(msk, entry) {
> -		if (test_bit(entry->addr.id, bitmap->map))
> +		if (is_init_id0(entry) || test_bit(entry->addr.id, bitmap->map))
>  			continue;

[Severity: Medium]
Does is_init_id0(entry) also skip a re-added ID 0 address when userspace does
not specify an explicit local port?

In net/mptcp/pm_userspace.c, is_init_id0() checks:

static bool is_init_id0(struct mptcp_pm_addr_entry *entry)
{
	return entry->addr.id == 0 && entry->addr.port == 0;
}

When the initial ID 0 entry is removed via mptcp_pm_nl_remove_doit() or
mptcp_pm_nl_subflow_destroy_doit(), and later re-added with a new subflow via
mptcp_pm_nl_subflow_create_doit() (or announced via
mptcp_pm_nl_announce_doit()) without the optional MPTCP_PM_ADDR_ATTR_PORT
attribute on the local address, mptcp_pm_parse_entry() leaves
entry.addr.port == 0:

net/mptcp/pm_userspace.c:mptcp_pm_nl_subflow_create_doit() {
	...
	err = mptcp_pm_parse_entry(laddr, info, true, &entry);
	...
	entry.flags |= MPTCP_PM_ADDR_FLAG_SUBFLOW;
	...
	err = mptcp_userspace_pm_append_new_local_addr(msk, &entry, false);
	...
	local.addr = entry.addr;
	local.flags = entry.flags;
	local.ifindex = entry.ifindex;
	...
	err = __mptcp_subflow_connect(sk, &local, &addr_r);
}

Because mptcp_userspace_pm_append_new_local_addr() stores the entry before
__mptcp_subflow_connect() runs on a stack copy, the entry in
msk->pm.userspace_pm_local_addr_list retains entry->addr.id == 0 and
entry->addr.port == 0.

When mptcp_userspace_pm_dump_addr() subsequently iterates over the list,
wouldn't is_init_id0(entry) return true for this re-added ID 0 entry and skip
it during the dump, contrary to the commit description?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-mptcp-pm-userspace-id0-case-v4-0-555066fa4292@kernel.org?part=6

Re: [PATCH mptcp-net v4 0/8] mptcp: pm: userspace: properly deal with the ID0 case

[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 (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Perf: Unstable: 1 failed test(s): validation:sched-bbr ⚠️ 
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/40961992710

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


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)