[PATCH mptcp-next 02/12] mptcp: implement mptcp_userspace_pm_dump_addr

Geliang Tang posted 12 patches 2 years, 1 month ago
Maintainers: Matthieu Baerts <matttbe@kernel.org>, Mat Martineau <martineau@kernel.org>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Shuah Khan <shuah@kernel.org>
There is a newer version of this series
[PATCH mptcp-next 02/12] mptcp: implement mptcp_userspace_pm_dump_addr
Posted by Geliang Tang 2 years, 1 month ago
This patch implements mptcp_userspace_pm_dump_addr() to dump addresses
from userspace pm address list. For each msk in this net, if userspace
PM is enabled in it, traverse each address entry in address list, put
every entry to userspace using mptcp_pm_nl_put_entry_msg().

Signed-off-by: Geliang Tang <geliang.tang@linux.dev>
---
 net/mptcp/pm_userspace.c | 29 +++++++++++++++++++++++++++++
 net/mptcp/protocol.h     |  2 ++
 2 files changed, 31 insertions(+)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index efecbe3cf415..6f659a78c637 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -550,3 +550,32 @@ int mptcp_userspace_pm_set_flags(struct net *net, struct nlattr *token,
 	sock_put(sk);
 	return ret;
 }
+
+int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
+				 struct netlink_callback *cb)
+{
+	struct net *net = sock_net(msg->sk);
+	struct mptcp_pm_addr_entry *entry;
+	long s_slot = 0, s_num = 0;
+	struct mptcp_sock *msk;
+
+	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
+		struct sock *sk = (struct sock *)msk;
+
+		if (mptcp_pm_is_userspace(msk)) {
+			lock_sock(sk);
+			spin_lock_bh(&msk->pm.lock);
+			list_for_each_entry(entry, &msk->pm.userspace_pm_local_addr_list, list) {
+				if (mptcp_pm_nl_put_entry_msg(msg, cb, entry))
+					break;
+			}
+			spin_unlock_bh(&msk->pm.lock);
+			release_sock(sk);
+		}
+
+		sock_put(sk);
+		cond_resched();
+	}
+
+	return msg->len;
+}
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 23d4742f3f30..a05a6745bc31 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1028,6 +1028,8 @@ int mptcp_pm_nl_put_entry_msg(struct sk_buff *msg,
 int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);
 int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
 int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
+int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
+				 struct netlink_callback *cb);
 
 void __init mptcp_pm_nl_init(void);
 void mptcp_pm_nl_work(struct mptcp_sock *msk);
-- 
2.35.3
Re: [PATCH mptcp-next 02/12] mptcp: implement mptcp_userspace_pm_dump_addr
Posted by Mat Martineau 2 years, 1 month ago
On Mon, 11 Dec 2023, Geliang Tang wrote:

> This patch implements mptcp_userspace_pm_dump_addr() to dump addresses
> from userspace pm address list. For each msk in this net, if userspace
> PM is enabled in it, traverse each address entry in address list, put
> every entry to userspace using mptcp_pm_nl_put_entry_msg().
>
> Signed-off-by: Geliang Tang <geliang.tang@linux.dev>
> ---
> net/mptcp/pm_userspace.c | 29 +++++++++++++++++++++++++++++
> net/mptcp/protocol.h     |  2 ++
> 2 files changed, 31 insertions(+)
>
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index efecbe3cf415..6f659a78c637 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -550,3 +550,32 @@ int mptcp_userspace_pm_set_flags(struct net *net, struct nlattr *token,
> 	sock_put(sk);
> 	return ret;
> }
> +
> +int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
> +				 struct netlink_callback *cb)

Hi Geliang -

The entire list of entries may not fit in one skb. The cb struct is used 
to store context so the netlink framework can call the dump function 
multiple times to fill multiple skbs. This is why the original 
mptcp_pm_nl_get_addr_dumpit() reads and stores the id to/from cb->args[0]. 
That cb->args[0] context is used to skip the ids that have already been 
dumped.

With the function body below, if the list does not fit in one skb it will 
do the same thing on every call and create duplicate entries, and will 
never return 0 to tell the caller that it is finished.

There is also nothing in the netlink output to indicate which MPTCP 
connection each subflow id is associated with, or whether the entry is 
from a userspace-managed socket or the in-kernel pm, so the dumped 
information is not useful to a userspace PM.

Netlink dump commands can have attributes in the request (see 
netdev_nl_napi_get_dumpit()), I think it will be simpler to add a 
connection token to the dump request and only dump the id list for that 
single MPTCP connection.

- Mat


> +{
> +	struct net *net = sock_net(msg->sk);
> +	struct mptcp_pm_addr_entry *entry;
> +	long s_slot = 0, s_num = 0;
> +	struct mptcp_sock *msk;
> +
> +	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
> +		struct sock *sk = (struct sock *)msk;
> +
> +		if (mptcp_pm_is_userspace(msk)) {
> +			lock_sock(sk);
> +			spin_lock_bh(&msk->pm.lock);
> +			list_for_each_entry(entry, &msk->pm.userspace_pm_local_addr_list, list) {
> +				if (mptcp_pm_nl_put_entry_msg(msg, cb, entry))
> +					break;
> +			}
> +			spin_unlock_bh(&msk->pm.lock);
> +			release_sock(sk);
> +		}
> +
> +		sock_put(sk);
> +		cond_resched();
> +	}
> +
> +	return msg->len;
> +}
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 23d4742f3f30..a05a6745bc31 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -1028,6 +1028,8 @@ int mptcp_pm_nl_put_entry_msg(struct sk_buff *msg,
> int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);
> int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
> int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
> +int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
> +				 struct netlink_callback *cb);
>
> void __init mptcp_pm_nl_init(void);
> void mptcp_pm_nl_work(struct mptcp_sock *msk);
> -- 
> 2.35.3
>
>
>