From: Geliang Tang <tanggeliang@kylinos.cn>

This patch adds a helper mptcp_pm_validate() to check whether required
ops are defined. It will be invoked in .validate of struct bpf_struct_ops.

Currently mandatory ops of mptcp_pm_ops are get_local_id and get_priority.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm.c       | 16 ++++++++++++++++
 net/mptcp/protocol.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 29bc903658f7..5d666b0891b3 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -1045,8 +1045,24 @@ struct mptcp_pm_ops *mptcp_pm_find(const char *name)
 	return NULL;
 }
 
+int mptcp_pm_validate(struct mptcp_pm_ops *pm_ops)
+{
+	if (!pm_ops->get_local_id || !pm_ops->get_priority) {
+		pr_err("%s does not implement required ops\n", pm_ops->name);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 int mptcp_pm_register(struct mptcp_pm_ops *pm_ops)
 {
+	int ret;
+
+	ret = mptcp_pm_validate(pm_ops);
+	if (ret)
+		return ret;
+
 	spin_lock(&mptcp_pm_list_lock);
 	if (mptcp_pm_find(pm_ops->name)) {
 		spin_unlock(&mptcp_pm_list_lock);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index d26e89d960a1..1ce6c22cb295 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1059,6 +1059,7 @@ extern struct mptcp_pm_ops mptcp_pm_kernel;
 struct mptcp_pm_ops *mptcp_pm_find(const char *name);
 int mptcp_pm_register(struct mptcp_pm_ops *pm_ops);
 void mptcp_pm_unregister(struct mptcp_pm_ops *pm_ops);
+int mptcp_pm_validate(struct mptcp_pm_ops *pm_ops);
 
 void mptcp_userspace_pm_free_local_addr_list(struct mptcp_sock *msk);
 
-- 
2.43.0

Hi Geliang,

On 06/03/2025 12:01, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch adds a helper mptcp_pm_validate() to check whether required
> ops are defined. It will be invoked in .validate of struct bpf_struct_ops.
> 
> Currently mandatory ops of mptcp_pm_ops are get_local_id and get_priority.

Because more ops will be added soon, it looks a bit strange to introduce
this helper now, no? If you have to change something else in this
series, maybe best to squash this in patch 2/12 ("mptcp: pm: define
struct mptcp_pm_ops") but ...

> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm.c       | 16 ++++++++++++++++
>  net/mptcp/protocol.h |  1 +
>  2 files changed, 17 insertions(+)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 29bc903658f7..5d666b0891b3 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -1045,8 +1045,24 @@ struct mptcp_pm_ops *mptcp_pm_find(const char *name)
>  	return NULL;
>  }
>  
> +int mptcp_pm_validate(struct mptcp_pm_ops *pm_ops)
> +{
> +	if (!pm_ops->get_local_id || !pm_ops->get_priority) {
> +		pr_err("%s does not implement required ops\n", pm_ops->name);
> +		return -EINVAL;
> +	}

... without this if-statement: this can be introduced when adding each
mandatory ops. I think this will help better understanding that they are
mandatory when they are being introduced (patch 7 and 8/12). WDYT?

(Please then add something like this in the commit message of patch 2/12
after the squash:)

  mptcp_pm_validate() will be invoked in .validate of struct
  bpf_struct_ops. That's why this function is exported.

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