Hi Geliang,
On 16/02/2024 08:42, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> Just like MPTCP_PM_ADDR_FLAG_SIGNAL flag is checked in userspace PM
> announce mptcp_pm_nl_announce_doit(), PM flags should be checked in
> mptcp_pm_nl_subflow_create_doit() too.
>
> If MPTCP_PM_ADDR_FLAG_SUBFLOW flag is not set, there's no flags field
> in the output of dump_addr. This looks a bit strange:
>
> id 10 flags 10.0.3.2
>
> This patch uses mptcp_pm_parse_entry() instead of mptcp_pm_parse_addr()
> to get the PM flags of the entry and check it. MPTCP_PM_ADDR_FLAG_SIGNAL
> flag shouldn't be set here, and if MPTCP_PM_ADDR_FLAG_SUBFLOW flag is
> missing from the netlink attribute, always set this flag.
>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> net/mptcp/pm_userspace.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index d6b7be3afbe5..73ad286247f1 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -360,11 +360,20 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
> goto create_err;
> }
>
> - err = mptcp_pm_parse_addr(laddr, info, &addr_l);
> + err = mptcp_pm_parse_entry(laddr, info, true, &local);
> if (err < 0) {
> NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr");
> goto create_err;
> }
> + addr_l = local.addr;
Detail: do we still need addr_l? Can we not use local.addr directly below?
> +
> + if (local.flags & MPTCP_PM_ADDR_FLAG_SIGNAL) {
> + GENL_SET_ERR_MSG(info, "invalid addr flags");
> + err = -EINVAL;
> + goto create_err;
> + }
> + if (!(local.flags & MPTCP_PM_ADDR_FLAG_SUBFLOW))
> + local.flags |= MPTCP_PM_ADDR_FLAG_SUBFLOW;
(not sure the 'if' is needed: why not always setting it?)
>
> err = mptcp_pm_parse_addr(raddr, info, &addr_r);
> if (err < 0) {
> @@ -378,7 +387,6 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
> goto create_err;
> }
>
> - local.addr = addr_l;
> err = mptcp_userspace_pm_append_new_local_addr(msk, &local, false);
> if (err < 0) {
> GENL_SET_ERR_MSG(info, "did not match address and id");
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.