From: Geliang Tang <tanggeliang@kylinos.cn>

Generally, in the path manager interfaces, the local address is defined
as an mptcp_pm_addr_entry type address, while the remote address is
defined as an mptcp_addr_info type one:

    (struct mptcp_pm_addr_entry *local, struct mptcp_addr_info *remote)

In order to make these interfaces more flexible and extensible, a struct
mptcp_pm_param is defined here to pass parameters. "entry" can be used
as the local address entry, and "addr" can be used as the remote address.

Also add a new helper mptcp_pm_param_set_contexts() to set a struct
mptcp_pm_param type parameter.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 include/net/mptcp.h  | 13 +++++++++++++
 net/mptcp/pm.c       | 10 ++++++++++
 net/mptcp/protocol.h | 11 +++--------
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 72d6e6597add..a41d6c74760f 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -121,6 +121,19 @@ struct mptcp_sched_ops {
 	void (*release)(struct mptcp_sock *msk);
 } ____cacheline_aligned_in_smp;
 
+struct mptcp_pm_addr_entry {
+	struct list_head	list;
+	struct mptcp_addr_info	addr;
+	u8			flags;
+	int			ifindex;
+	struct socket		*lsk;
+};
+
+struct mptcp_pm_param {
+	struct mptcp_pm_addr_entry	entry;
+	struct mptcp_addr_info		addr;
+};
+
 #ifdef CONFIG_MPTCP
 void mptcp_init(void);
 
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 94620ab172b7..6a504c870e1a 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -401,6 +401,16 @@ bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
 	return ret;
 }
 
+void mptcp_pm_param_set_contexts(struct mptcp_pm_param *param,
+				 const struct mptcp_pm_addr_entry *entry,
+				 const struct mptcp_addr_info *addr)
+{
+	if (entry)
+		param->entry = *entry;
+	if (addr)
+		param->addr = *addr;
+}
+
 int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
 {
 	struct mptcp_pm_addr_entry skc_local;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index ef1d43406f9b..dbcf4b84e0f0 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -246,14 +246,6 @@ struct mptcp_pm_local {
 	int			ifindex;
 };
 
-struct mptcp_pm_addr_entry {
-	struct list_head	list;
-	struct mptcp_addr_info	addr;
-	u8			flags;
-	int			ifindex;
-	struct socket		*lsk;
-};
-
 struct mptcp_data_frag {
 	struct list_head list;
 	u64 data_seq;
@@ -1125,6 +1117,9 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, const struct sk_buff *skb,
 			      bool *drop_other_suboptions);
 bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
 			     struct mptcp_rm_list *rm_list);
+void mptcp_pm_param_set_contexts(struct mptcp_pm_param *param,
+				 const struct mptcp_pm_addr_entry *entry,
+				 const struct mptcp_addr_info *addr);
 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_pm_addr_entry *skc);
-- 
2.43.0

Hi Geliang,

On 20/02/2025 03:57, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Generally, in the path manager interfaces, the local address is defined
> as an mptcp_pm_addr_entry type address, while the remote address is
> defined as an mptcp_addr_info type one:
> 
>     (struct mptcp_pm_addr_entry *local, struct mptcp_addr_info *remote)
> 
> In order to make these interfaces more flexible and extensible, a struct
> mptcp_pm_param is defined here to pass parameters. "entry" can be used
> as the local address entry, and "addr" can be used as the remote address.

Mmh, it is not clear to me why you cannot use only the parameters that
are needed per interfaces, e.g. "local" and "remote". These parameters
are not even always needed, e.g. when an address has been announced,
"remote" doesn't make sense here. Same for get_local_id and
get_priority. Similarly, when an address has been removed, only the ID
is needed for the PM, etc.

I'm not convinced by the "flexible and extensible" reasons for the
future: "local" and "remote" will likely not change. If we want to add
more parameters, will we require them for all interfaces?

Also, if you use similar signatures as the ones being used, no need to
do many changes in the current in-kernel and userspace PM, e.g. no need
to introduce the new helpers in patches 5 and 6. Instead, these patches
could directly set the available helpers, and we can reduce the number
of patches, e.g. patches 8 and 9 (and more) could all come in one, no?

Plus, it feels strange to copy data from one structure to another just
to pass parameters, no?

Are you sure we need this patch? (and the next one?)

> Also add a new helper mptcp_pm_param_set_contexts() to set a struct
> mptcp_pm_param type parameter.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  include/net/mptcp.h  | 13 +++++++++++++
>  net/mptcp/pm.c       | 10 ++++++++++
>  net/mptcp/protocol.h | 11 +++--------
>  3 files changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index 72d6e6597add..a41d6c74760f 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -121,6 +121,19 @@ struct mptcp_sched_ops {
>  	void (*release)(struct mptcp_sock *msk);
>  } ____cacheline_aligned_in_smp;
>  
> +struct mptcp_pm_addr_entry {
> +	struct list_head	list;
> +	struct mptcp_addr_info	addr;
> +	u8			flags;
> +	int			ifindex;
> +	struct socket		*lsk;
> +};
> +
> +struct mptcp_pm_param {
> +	struct mptcp_pm_addr_entry	entry;
> +	struct mptcp_addr_info		addr;
If this structure is really needed, it seems clearer to use "local" and
"remote" instead of "entry" and "addr".

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