Re: [PATCH mptcp-next v14 3/6] mptcp: pm: in-kernel: add the subflow-retries limit

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] mptcp: pm: mixed limit validation in mptcp_pm_nl_set_limits_doit()
--

commit a548b0ce8ac5f215ffb57499482e189af72f45ad
Author: Anil Kaushik <anilkaushikwireless@gmail.com>

mptcp: pm: in-kernel: add the subflow-retries limit

This commit adds a new subflow-retries limit to the in-kernel path manager
for configuring the number of SYN retries when re-creating a subflow after
a timeout, and exposes it in struct mptcp_info.

> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index d3014bf57bf3c..7fb4c78743409 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c

[ ... ]

> @@ -1419,7 +1428,8 @@ static int parse_limit(struct genl_info *info, int id, unsigned int *limit)
>  int mptcp_pm_nl_set_limits_doit(struct sk_buff *skb, struct genl_info *info)
>  {
>  	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
> -	unsigned int rcv_addrs, subflows;
> +	unsigned int rcv_addrs, subflows, retries;
> +	struct nlattr *attr;
>  	int ret;
>  
>  	spin_lock_bh(&pernet->lock);
> @@ -1433,8 +1443,13 @@ int mptcp_pm_nl_set_limits_doit(struct sk_buff *skb, struct genl_info *info)
>  	if (ret)
>  		goto unlock;
>  
> +	/* the maximum, MAX_TCP_SYNCNT, is checked by the policy */
> +	attr = info->attrs[MPTCP_PM_ATTR_SUBFLOW_RETRIES];
> +	retries = nla_get_u32_default(attr, pernet->limit_subflow_retries);

[Severity: Low]
As reported by Matthieu Baerts on v7 [1], mptcp_pm_nl_set_limits_doit() in
net/mptcp/pm_kernel.c now mixes two different limit validation approaches:
MPTCP_PM_ATTR_RCV_ADD_ADDRS and MPTCP_PM_ATTR_SUBFLOWS are checked via
parse_limit(), while MPTCP_PM_ATTR_SUBFLOW_RETRIES relies on the netlink
policy and nla_get_u32_default().

Should a prerequisite patch convert the existing two limits to use netlink
policy validation and nla_get_u32_default() as well (or should this new
limit continue using parse_limit()) so all limits in
mptcp_pm_nl_set_limits_doit() are handled uniformly?

[1] https://lore.kernel.org/r/56679372-0ae6-43bf-b0fa-a6b853a1b68c@kernel.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101072051.3853492-1-anilkaushikwireless@gmail.com?part=3