Re: [PATCH mptcp-next v10 08/12] mptcp: pm: validate mandatory ops

[Matthieu Baerts <matttbe@kernel.org>]

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.