On Mon, 2024-12-30 at 14:24 +0100, Matthieu Baerts (NGI0) wrote:
> Since its introduction in commit 892f396c8e68 ("mptcp: netlink: issue
> MP_PRIO signals from userspace PMs"), it was mandatory to specify the
> remote address, because of the 'if (rem->addr.family == AF_UNSPEC)'
> check done later one.
>
> In theory, this attribute can be optional, but it sounds better to be
> precise to avoid sending the MP_PRIO on the wrong subflow, e.g. if
> there
> are multiple subflows attached to the same local ID. This can be
> relaxed
> later on if there is a need to act on multiple subflows with one
> command.
>
> For the moment, the check to see if attr_rem is NULL can be removed,
> because mptcp_pm_parse_entry() will do this check as well, no need to
> do
> that differently here.
>
> While at it, move the parsing after the check linked to the local
> attribute.
>
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> net/mptcp/pm_userspace.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index
> a8f73b082460ba00f7fa998cf7646368f8201e2e..2f82ac49db35ec34dcc0b9208f4
> ac12edc6ab254 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -601,12 +601,6 @@ int mptcp_userspace_pm_set_flags(struct sk_buff
> *skb, struct genl_info *info)
> if (ret < 0)
> goto set_flags_err;
>
> - if (attr_rem) {
> - ret = mptcp_pm_parse_entry(attr_rem, info, false,
> &rem);
> - if (ret < 0)
> - goto set_flags_err;
> - }
I did not remove this 'if (attr_rem)' check in v7. I moved this code to
a common place for userspace PM and in-kernel PM. For in-kernel PM,
this check is required.
> -
> if (loc.addr.family == AF_UNSPEC) {
> NL_SET_ERR_MSG_ATTR(info->extack, attr,
> "invalid local address family");
> @@ -614,6 +608,10 @@ int mptcp_userspace_pm_set_flags(struct sk_buff
> *skb, struct genl_info *info)
> goto set_flags_err;
> }
>
> + ret = mptcp_pm_parse_entry(attr_rem, info, false, &rem);
> + if (ret < 0)
> + goto set_flags_err;
> +
> if (rem.addr.family == AF_UNSPEC) {
> NL_SET_ERR_MSG_ATTR(info->extack, attr_rem,
> "invalid remote address
> family");
>