[PATCH mptcp-next 3/4] mptcp: unify pm get_flags_and_ifindex_by_id

Geliang Tang posted 4 patches 2 years, 6 months ago
Maintainers: Matthieu Baerts <matthieu.baerts@tessares.net>, 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>
[PATCH mptcp-next 3/4] mptcp: unify pm get_flags_and_ifindex_by_id
Posted by Geliang Tang 2 years, 6 months ago
This patch unifies the three PM get_flags_and_ifindex_by_id() interfaces:

mptcp_pm_nl_get_flags_and_ifindex_by_id() in mptcp/pm_netlink.c for the
in-kernel PM and mptcp_userspace_pm_get_flags_and_ifindex_by_id() in
mptcp/pm_userspace.c for the userspace PM.

They'll be switched in the common PM infterface
mptcp_pm_get_flags_and_ifindex_by_id() in mptcp/pm.c.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/pm.c         |  8 ++++++++
 net/mptcp/pm_netlink.c | 10 ++--------
 net/mptcp/protocol.h   |  2 ++
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index e37df2f45c70..8499196b8789 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -424,6 +424,14 @@ int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
 	return mptcp_pm_nl_get_local_id(msk, skc);
 }
 
+int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id,
+					 u8 *flags, int *ifindex)
+{
+	if (id && mptcp_pm_is_userspace(msk))
+		return mptcp_userspace_pm_get_flags_and_ifindex_by_id(msk, id, flags, ifindex);
+	return mptcp_pm_nl_get_flags_and_ifindex_by_id(msk, id, flags, ifindex);
+}
+
 void mptcp_pm_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk)
 {
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index deb097f16abc..ab4f0483d9d8 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1359,8 +1359,8 @@ static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info)
 	return ret;
 }
 
-int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id,
-					 u8 *flags, int *ifindex)
+int mptcp_pm_nl_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id,
+					    u8 *flags, int *ifindex)
 {
 	struct mptcp_pm_addr_entry *entry;
 	struct sock *sk = (struct sock *)msk;
@@ -1370,12 +1370,6 @@ int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id
 	*ifindex = 0;
 
 	if (id) {
-		if (mptcp_pm_is_userspace(msk))
-			return mptcp_userspace_pm_get_flags_and_ifindex_by_id(msk,
-									      id,
-									      flags,
-									      ifindex);
-
 		rcu_read_lock();
 		entry = __lookup_addr_by_id(pm_nl_get_pernet(net), id);
 		if (entry) {
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 58989303470f..c91c9387f42d 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -838,6 +838,8 @@ mptcp_lookup_anno_list_by_saddr(const struct mptcp_sock *msk,
 int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
 					 unsigned int id,
 					 u8 *flags, int *ifindex);
+int mptcp_pm_nl_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id,
+					    u8 *flags, int *ifindex);
 int mptcp_userspace_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
 						   unsigned int id,
 						   u8 *flags, int *ifindex);
-- 
2.35.3
Re: [PATCH mptcp-next 3/4] mptcp: unify pm get_flags_and_ifindex_by_id
Posted by Matthieu Baerts 2 years, 6 months ago
Hi Geliang,

On 25/05/2023 10:07, Geliang Tang wrote:
> This patch unifies the three PM get_flags_and_ifindex_by_id() interfaces:
> 
> mptcp_pm_nl_get_flags_and_ifindex_by_id() in mptcp/pm_netlink.c for the
> in-kernel PM and mptcp_userspace_pm_get_flags_and_ifindex_by_id() in
> mptcp/pm_userspace.c for the userspace PM.
> 
> They'll be switched in the common PM infterface
> mptcp_pm_get_flags_and_ifindex_by_id() in mptcp/pm.c.
> 
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  net/mptcp/pm.c         |  8 ++++++++
>  net/mptcp/pm_netlink.c | 10 ++--------
>  net/mptcp/protocol.h   |  2 ++
>  3 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index e37df2f45c70..8499196b8789 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -424,6 +424,14 @@ int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
>  	return mptcp_pm_nl_get_local_id(msk, skc);
>  }
>  
> +int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id,
> +					 u8 *flags, int *ifindex)
> +{
> +	if (id && mptcp_pm_is_userspace(msk))

It looks strange to call mptcp_pm_nl_() if the userspace PM is used but
id == 0. Maybe add this check in the userspace function?

Or maybe better to do:

  int mptcp_pm_get_flags_and_ifindex_by_id(...)
  {
      *flags = 0;
      *ifindex = 0;

      if (id == 0)
          return 0;

      if (mptcp_pm_is_userspace(msk))
          return mptcp_userspace_pm_(...);
      return return mptcp_pm_nl_(...);
  }

By doing that, you can remove the init of flags and ifindex frmo the two
PMs + the check of 'id' from the Netlink PM. WDYT?

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net