Series comparison

-[PATCH net-next 00/15] mptcp: pm: misc cleanups, part 2
+[PATCH net-next v3 00/15] mptcp: pm: misc cleanups, part 2
 ...
 The behaviour when there are no errors should then not be modified.
 Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 ---
+Changes in v3:
+- Patch 11: a variable was no longer assigned in pm_userspace.c, but
+  still used in this patch (and no longer in the next one). (Geliang)
+- Rebased on top of the latest net-next.
+- Link to v2: https://lore.kernel.org/r/20250117-net-next-mptcp-pm-misc-cleanup-2-v2-0-61d4fe0586e8@kernel.org
+Changes in v2:
+- Patch 11: a variable was no longer assigned in pm_netlink.c, but still
+  used in this patch (and no longer in the next one). (Simon)
+- Link to v1: https://lore.kernel.org/r/20250116-net-next-mptcp-pm-misc-cleanup-2-v1-0-c0b43f18fe06@kernel.org
+---
 Geliang Tang (9):
       mptcp: pm: drop info of userspace_pm_remove_id_zero_address
       mptcp: pm: userspace: use GENL_REQ_ATTR_CHECK
       mptcp: pm: make three pm wrappers static
       mptcp: pm: drop skb parameter of get_addr
 ...
  net/mptcp/pm_netlink.c   | 129 ++++++++++-------------------
  net/mptcp/pm_userspace.c | 209 +++++++++++++++++++++--------------------------
  net/mptcp/protocol.h     |  14 ++--
 files changed, 225 insertions(+), 213 deletions(-)
 ---
-base-commit: b44e27b4df1a1cd3fd84cf26c82156ed0301575f
+base-commit: 26db4dbb747813b5946aff31485873f071a10332
 change-id: 20250116-net-next-mptcp-pm-misc-cleanup-2-b0f50eff8084
 Best regards,
 --
 Matthieu Baerts (NGI0) <matttbe@kernel.org>

-[PATCH net-next 01/15] mptcp: pm: drop info of userspace_pm_remove_id_zero_address
+[PATCH net-next v3 01/15] mptcp: pm: drop info of userspace_pm_remove_id_zero_address
 From: Geliang Tang <tanggeliang@kylinos.cn>
 The only use of 'info' parameter of userspace_pm_remove_id_zero_address()
 is to set an error message into it.
 Plus, this helper will only fail when it cannot find any subflows with a
 local address ID 0.
 This patch drops this parameter and sets the error message where this
 function is called in mptcp_pm_nl_remove_doit().
 Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
 Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 ---
  net/mptcp/pm_userspace.c | 15 ++++++++-------
 file changed, 8 insertions(+), 7 deletions(-)
 diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm_userspace.c
 +++ b/net/mptcp/pm_userspace.c
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
      return err;
  }
 -static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
 -                             struct genl_info *info)
 +static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk)
  {
      struct mptcp_rm_list list = { .nr = 0 };
      struct mptcp_subflow_context *subflow;
 @@ -XXX,XX +XXX,XX @@ static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
              break;
          }
      }
 -    if (!has_id_0) {
 -        GENL_SET_ERR_MSG(info, "address with id 0 not found");
 +    if (!has_id_0)
          goto remove_err;
 -    }
      list.ids[list.nr++] = 0;
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
      sk = (struct sock *)msk;
      if (id_val == 0) {
 -        err = mptcp_userspace_pm_remove_id_zero_address(msk, info);
 +        err = mptcp_userspace_pm_remove_id_zero_address(msk);
          goto out;
      }
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
      spin_lock_bh(&msk->pm.lock);
      match = mptcp_userspace_pm_lookup_addr_by_id(msk, id_val);
      if (!match) {
 -        GENL_SET_ERR_MSG(info, "address with specified id not found");
          spin_unlock_bh(&msk->pm.lock);
          release_sock(sk);
          goto out;
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
      err = 0;
  out:
 +    if (err)
 +        GENL_SET_ERR_MSG_FMT(info,
 +                     "address with id %u not found",
 +                     id_val);
 +
      sock_put(sk);
      return err;
  }
 --
 .47.1

-[PATCH net-next 02/15] mptcp: pm: userspace: flags: clearer msg if no remote addr
+[PATCH net-next v3 02/15] mptcp: pm: userspace: flags: clearer msg if no remote addr
 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.
 Reviewed-by: Geliang Tang <geliang@kernel.org>
 Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 ---
  net/mptcp/pm_userspace.c | 8 +++-----
 file changed, 3 insertions(+), 5 deletions(-)
 diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm_userspace.c
 +++ b/net/mptcp/pm_userspace.c
 @@ -XXX,XX +XXX,XX @@ 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;
 -    }
 +    ret = mptcp_pm_parse_entry(attr_rem, info, false, &rem);
 +    if (ret < 0)
 +        goto set_flags_err;
      if (loc.addr.family == AF_UNSPEC ||
          rem.addr.family == AF_UNSPEC) {
 --
 .47.1

-[PATCH net-next 03/15] mptcp: pm: more precise error messages
+[PATCH net-next v3 03/15] mptcp: pm: more precise error messages
 Some errors reported by the userspace PM were vague: "this or that is
 invalid".
 It is easier for the userspace to know which part is wrong, instead of
 having to guess that.
 While at it, in mptcp_userspace_pm_set_flags() move the parsing after
 the check linked to the local attribute.
 Reviewed-by: Geliang Tang <geliang@kernel.org>
 Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 ---
  net/mptcp/pm_userspace.c | 31 ++++++++++++++++++++++++-------
 file changed, 24 insertions(+), 7 deletions(-)
 diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm_userspace.c
 +++ b/net/mptcp/pm_userspace.c
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
          goto announce_err;
      }
 -    if (addr_val.addr.id == 0 || !(addr_val.flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
 -        GENL_SET_ERR_MSG(info, "invalid addr id or flags");
 +    if (addr_val.addr.id == 0) {
 +        GENL_SET_ERR_MSG(info, "invalid addr id");
 +        err = -EINVAL;
 +        goto announce_err;
 +    }
 +
 +    if (!(addr_val.flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
 +        GENL_SET_ERR_MSG(info, "invalid addr flags");
          err = -EINVAL;
          goto announce_err;
      }
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
          goto destroy_err;
      }
 -    if (!addr_l.addr.port || !addr_r.port) {
 -        GENL_SET_ERR_MSG(info, "missing local or remote port");
 +    if (!addr_l.addr.port) {
 +        GENL_SET_ERR_MSG(info, "missing local port");
 +        err = -EINVAL;
 +        goto destroy_err;
 +    }
 +
 +    if (!addr_r.port) {
 +        GENL_SET_ERR_MSG(info, "missing remote port");
          err = -EINVAL;
          goto destroy_err;
      }
 @@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
      if (ret < 0)
          goto set_flags_err;
 +    if (loc.addr.family == AF_UNSPEC) {
 +        GENL_SET_ERR_MSG(info, "invalid local address family");
 +        ret = -EINVAL;
 +        goto set_flags_err;
 +    }
 +
      ret = mptcp_pm_parse_entry(attr_rem, info, false, &rem);
      if (ret < 0)
          goto set_flags_err;
 -    if (loc.addr.family == AF_UNSPEC ||
 -        rem.addr.family == AF_UNSPEC) {
 -        GENL_SET_ERR_MSG(info, "invalid address families");
 +    if (rem.addr.family == AF_UNSPEC) {
 +        GENL_SET_ERR_MSG(info, "invalid remote address family");
          ret = -EINVAL;
          goto set_flags_err;
      }
 --
 .47.1

-[PATCH net-next 04/15] mptcp: pm: improve error messages
+[PATCH net-next v3 04/15] mptcp: pm: improve error messages
 Some error messages were:
  - too generic: "missing input", "invalid request"
  - not precise enough: "limit greater than maximum" but what's the max?
  - missing: subflow not found, or connect error.
 This can be easily improved by being more precise, or adding new error
 messages.
 Reviewed-by: Geliang Tang <geliang@kernel.org>
 Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 ---
  net/mptcp/pm_netlink.c   |  6 ++++--
  net/mptcp/pm_userspace.c | 10 +++++++++-
 files changed, 13 insertions(+), 3 deletions(-)
 diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm_netlink.c
 +++ b/net/mptcp/pm_netlink.c
 @@ -XXX,XX +XXX,XX @@ static int parse_limit(struct genl_info *info, int id, unsigned int *limit)
      *limit = nla_get_u32(attr);
      if (*limit > MPTCP_PM_ADDR_MAX) {
 -        GENL_SET_ERR_MSG(info, "limit greater than maximum");
 +        NL_SET_ERR_MSG_ATTR_FMT(info->extack, attr,
 +                    "limit greater than maximum (%u)",
 +                    MPTCP_PM_ADDR_MAX);
          return -EINVAL;
      }
      return 0;
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info)
      if (addr.addr.family == AF_UNSPEC) {
          lookup_by_id = 1;
          if (!addr.addr.id) {
 -            GENL_SET_ERR_MSG(info, "missing required inputs");
 +            GENL_SET_ERR_MSG(info, "missing address ID");
              return -EOPNOTSUPP;
          }
      }
 diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm_userspace.c
 +++ b/net/mptcp/pm_userspace.c
 @@ -XXX,XX +XXX,XX @@ static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *in
      }
      if (!mptcp_pm_is_userspace(msk)) {
 -        GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected");
 +        GENL_SET_ERR_MSG(info, "userspace PM not selected");
          sock_put((struct sock *)msk);
          return NULL;
      }
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
      err = __mptcp_subflow_connect(sk, &local, &addr_r);
      release_sock(sk);
 +    if (err)
 +        GENL_SET_ERR_MSG_FMT(info, "connect error: %d", err);
 +
      spin_lock_bh(&msk->pm.lock);
      if (err)
          mptcp_userspace_pm_delete_local_addr(msk, &entry);
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
      lock_sock(sk);
      ssk = mptcp_nl_find_ssk(msk, &addr_l.addr, &addr_r);
      if (!ssk) {
 +        GENL_SET_ERR_MSG(info, "subflow not found");
          err = -ESRCH;
          goto release_sock;
      }
 @@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
      ret = mptcp_pm_nl_mp_prio_send_ack(msk, &loc.addr, &rem.addr, bkup);
      release_sock(sk);
 +    /* mptcp_pm_nl_mp_prio_send_ack() only fails in one case */
 +    if (ret < 0)
 +        GENL_SET_ERR_MSG(info, "subflow not found");
 +
  set_flags_err:
      sock_put(sk);
      return ret;
 --
 .47.1

-[PATCH net-next 05/15] mptcp: pm: userspace: use GENL_REQ_ATTR_CHECK
+[PATCH net-next v3 05/15] mptcp: pm: userspace: use GENL_REQ_ATTR_CHECK
 From: Geliang Tang <tanggeliang@kylinos.cn>
 A more general way to check if MPTCP_PM_ATTR_* exists in 'info'
 is to use GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_*) instead of
 directly reading info->attrs[MPTCP_PM_ATTR_*] and then checking
 if it's NULL.
 So this patch uses GENL_REQ_ATTR_CHECK() for userspace PM in
 mptcp_pm_nl_announce_doit(), mptcp_pm_nl_remove_doit(),
 mptcp_pm_nl_subflow_create_doit(), mptcp_pm_nl_subflow_destroy_doit()
 and mptcp_userspace_pm_get_sock().
 Suggested-by: Jakub Kicinski <kuba@kernel.org>
 Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
 Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 ---
  net/mptcp/pm_userspace.c | 41 +++++++++++++++++++----------------------
 file changed, 19 insertions(+), 22 deletions(-)
 diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm_userspace.c
 +++ b/net/mptcp/pm_userspace.c
 @@ -XXX,XX +XXX,XX @@ bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk,
  static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *info)
  {
 -    struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
      struct mptcp_sock *msk;
 +    struct nlattr *token;
 -    if (!token) {
 -        GENL_SET_ERR_MSG(info, "missing required token");
 +    if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_TOKEN))
          return NULL;
 -    }
 +    token = info->attrs[MPTCP_PM_ATTR_TOKEN];
      msk = mptcp_token_get_sock(genl_info_net(info), nla_get_u32(token));
      if (!msk) {
          NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token");
 @@ -XXX,XX +XXX,XX @@ static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *in
  int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
  {
 -    struct nlattr *addr = info->attrs[MPTCP_PM_ATTR_ADDR];
      struct mptcp_pm_addr_entry addr_val;
      struct mptcp_sock *msk;
 +    struct nlattr *addr;
      int err = -EINVAL;
      struct sock *sk;
 -    if (!addr) {
 -        GENL_SET_ERR_MSG(info, "missing required address");
 +    if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR))
          return err;
 -    }
      msk = mptcp_userspace_pm_get_sock(info);
      if (!msk)
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
      sk = (struct sock *)msk;
 +    addr = info->attrs[MPTCP_PM_ATTR_ADDR];
      err = mptcp_pm_parse_entry(addr, info, true, &addr_val);
      if (err < 0) {
          GENL_SET_ERR_MSG(info, "error parsing local address");
 @@ -XXX,XX +XXX,XX @@ void mptcp_pm_remove_addr_entry(struct mptcp_sock *msk,
  int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
  {
 -    struct nlattr *id = info->attrs[MPTCP_PM_ATTR_LOC_ID];
      struct mptcp_pm_addr_entry *match;
      struct mptcp_sock *msk;
 +    struct nlattr *id;
      int err = -EINVAL;
      struct sock *sk;
      u8 id_val;
 -    if (!id) {
 -        GENL_SET_ERR_MSG(info, "missing required ID");
 +    if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_LOC_ID))
          return err;
 -    }
 +    id = info->attrs[MPTCP_PM_ATTR_LOC_ID];
      id_val = nla_get_u8(id);
      msk = mptcp_userspace_pm_get_sock(info);
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
  int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
  {
 -    struct nlattr *raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
 -    struct nlattr *laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
      struct mptcp_pm_addr_entry entry = { 0 };
      struct mptcp_addr_info addr_r;
 +    struct nlattr *raddr, *laddr;
      struct mptcp_pm_local local;
      struct mptcp_sock *msk;
      int err = -EINVAL;
      struct sock *sk;
 -    if (!laddr || !raddr) {
 -        GENL_SET_ERR_MSG(info, "missing required address(es)");
 +    if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR) ||
 +        GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE))
          return err;
 -    }
      msk = mptcp_userspace_pm_get_sock(info);
      if (!msk)
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
      sk = (struct sock *)msk;
 +    laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
      err = mptcp_pm_parse_entry(laddr, info, true, &entry);
      if (err < 0) {
          NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr");
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
      }
      entry.flags |= MPTCP_PM_ADDR_FLAG_SUBFLOW;
 +    raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
      err = mptcp_pm_parse_addr(raddr, info, &addr_r);
      if (err < 0) {
          NL_SET_ERR_MSG_ATTR(info->extack, raddr, "error parsing remote addr");
 @@ -XXX,XX +XXX,XX @@ static struct sock *mptcp_nl_find_ssk(struct mptcp_sock *msk,
  int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info)
  {
 -    struct nlattr *raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
 -    struct nlattr *laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
      struct mptcp_pm_addr_entry addr_l;
      struct mptcp_addr_info addr_r;
 +    struct nlattr *raddr, *laddr;
      struct mptcp_sock *msk;
      struct sock *sk, *ssk;
      int err = -EINVAL;
 -    if (!laddr || !raddr) {
 -        GENL_SET_ERR_MSG(info, "missing required address(es)");
 +    if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR) ||
 +        GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE))
          return err;
 -    }
      msk = mptcp_userspace_pm_get_sock(info);
      if (!msk)
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
      sk = (struct sock *)msk;
 +    laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
      err = mptcp_pm_parse_entry(laddr, info, true, &addr_l);
      if (err < 0) {
          NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr");
          goto destroy_err;
      }
 +    raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
      err = mptcp_pm_parse_addr(raddr, info, &addr_r);
      if (err < 0) {
          NL_SET_ERR_MSG_ATTR(info->extack, raddr, "error parsing remote addr");
 --
 .47.1

-[PATCH net-next 06/15] mptcp: pm: remove duplicated error messages
+[PATCH net-next v3 06/15] mptcp: pm: remove duplicated error messages
 mptcp_pm_parse_entry() and mptcp_pm_parse_addr() will already set a
 error message in case of parsing issue.
 Then, no need to override this error message with another less precise
 one: "error parsing address".
 Reviewed-by: Geliang Tang <geliang@kernel.org>
 Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 ---
  net/mptcp/pm_userspace.c | 20 +++++---------------
 file changed, 5 insertions(+), 15 deletions(-)
 diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm_userspace.c
 +++ b/net/mptcp/pm_userspace.c
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
      addr = info->attrs[MPTCP_PM_ATTR_ADDR];
      err = mptcp_pm_parse_entry(addr, info, true, &addr_val);
 -    if (err < 0) {
 -        GENL_SET_ERR_MSG(info, "error parsing local address");
 +    if (err < 0)
          goto announce_err;
 -    }
      if (addr_val.addr.id == 0) {
          GENL_SET_ERR_MSG(info, "invalid addr id");
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
      laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
      err = mptcp_pm_parse_entry(laddr, info, true, &entry);
 -    if (err < 0) {
 -        NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr");
 +    if (err < 0)
          goto create_err;
 -    }
      if (entry.flags & MPTCP_PM_ADDR_FLAG_SIGNAL) {
          GENL_SET_ERR_MSG(info, "invalid addr flags");
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
      raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
      err = mptcp_pm_parse_addr(raddr, info, &addr_r);
 -    if (err < 0) {
 -        NL_SET_ERR_MSG_ATTR(info->extack, raddr, "error parsing remote addr");
 +    if (err < 0)
          goto create_err;
 -    }
      if (!mptcp_pm_addr_families_match(sk, &entry.addr, &addr_r)) {
          GENL_SET_ERR_MSG(info, "families mismatch");
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
      laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
      err = mptcp_pm_parse_entry(laddr, info, true, &addr_l);
 -    if (err < 0) {
 -        NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr");
 +    if (err < 0)
          goto destroy_err;
 -    }
      raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
      err = mptcp_pm_parse_addr(raddr, info, &addr_r);
 -    if (err < 0) {
 -        NL_SET_ERR_MSG_ATTR(info->extack, raddr, "error parsing remote addr");
 +    if (err < 0)
          goto destroy_err;
 -    }
  #if IS_ENABLED(CONFIG_MPTCP_IPV6)
      if (addr_l.addr.family == AF_INET && ipv6_addr_v4mapped(&addr_r.addr6)) {
 --
 .47.1

-[PATCH net-next 07/15] mptcp: pm: mark missing address attributes
+[PATCH net-next v3 07/15] mptcp: pm: mark missing address attributes
 mptcp_pm_parse_entry() will check if the given attribute is defined. If
 not, it will return a generic error: "missing address info".
 It might then not be clear for the userspace developer which attribute
 is missing, especially when the command takes multiple addresses.
 By using GENL_REQ_ATTR_CHECK(), the userspace will get a hint about
 which attribute is missing, making thing clearer. Note that this is what
 was already done for most of the other MPTCP NL commands, this patch
 simply adds the missing ones.
 Reviewed-by: Geliang Tang <geliang@kernel.org>
 Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 ---
  net/mptcp/pm_netlink.c   | 24 ++++++++++++++++++++----
  net/mptcp/pm_userspace.c | 15 ++++++++++++---
 files changed, 32 insertions(+), 7 deletions(-)
 diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm_netlink.c
 +++ b/net/mptcp/pm_netlink.c
 @@ -XXX,XX +XXX,XX @@ static bool mptcp_pm_has_addr_attr_id(const struct nlattr *attr,
  int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
  {
 -    struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
      struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
      struct mptcp_pm_addr_entry addr, *entry;
 +    struct nlattr *attr;
      int ret;
 +    if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
 +        return -EINVAL;
 +
 +    attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
      ret = mptcp_pm_parse_entry(attr, info, true, &addr);
      if (ret < 0)
          return ret;
 @@ -XXX,XX +XXX,XX @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
  int mptcp_pm_nl_del_addr_doit(struct sk_buff *skb, struct genl_info *info)
  {
 -    struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
      struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
      struct mptcp_pm_addr_entry addr, *entry;
      unsigned int addr_max;
 +    struct nlattr *attr;
      int ret;
 +    if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
 +        return -EINVAL;
 +
 +    attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
      ret = mptcp_pm_parse_entry(attr, info, false, &addr);
      if (ret < 0)
          return ret;
 @@ -XXX,XX +XXX,XX @@ int mptcp_nl_fill_addr(struct sk_buff *skb,
  int mptcp_pm_nl_get_addr(struct sk_buff *skb, struct genl_info *info)
  {
 -    struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
      struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
      struct mptcp_pm_addr_entry addr, *entry;
      struct sk_buff *msg;
 +    struct nlattr *attr;
      void *reply;
      int ret;
 +    if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
 +        return -EINVAL;
 +
 +    attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
      ret = mptcp_pm_parse_entry(attr, info, false, &addr);
      if (ret < 0)
          return ret;
 @@ -XXX,XX +XXX,XX @@ static int mptcp_nl_set_flags(struct net *net,
  int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info)
  {
      struct mptcp_pm_addr_entry addr = { .addr = { .family = AF_UNSPEC }, };
 -    struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
      u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP |
                 MPTCP_PM_ADDR_FLAG_FULLMESH;
      struct net *net = sock_net(skb->sk);
      struct mptcp_pm_addr_entry *entry;
      struct pm_nl_pernet *pernet;
 +    struct nlattr *attr;
      u8 lookup_by_id = 0;
      u8 bkup = 0;
      int ret;
 +    if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR))
 +        return -EINVAL;
 +
      pernet = pm_nl_get_pernet(net);
 +    attr = info->attrs[MPTCP_PM_ATTR_ADDR];
      ret = mptcp_pm_parse_entry(attr, info, false, &addr);
      if (ret < 0)
          return ret;
 diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm_userspace.c
 +++ b/net/mptcp/pm_userspace.c
 @@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
  {
      struct mptcp_pm_addr_entry loc = { .addr = { .family = AF_UNSPEC }, };
      struct mptcp_pm_addr_entry rem = { .addr = { .family = AF_UNSPEC }, };
 -    struct nlattr *attr_rem = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
 -    struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
      struct mptcp_pm_addr_entry *entry;
 +    struct nlattr *attr, *attr_rem;
      struct mptcp_sock *msk;
      int ret = -EINVAL;
      struct sock *sk;
      u8 bkup = 0;
 +    if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR) ||
 +        GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE))
 +        return ret;
 +
      msk = mptcp_userspace_pm_get_sock(info);
      if (!msk)
          return ret;
      sk = (struct sock *)msk;
 +    attr = info->attrs[MPTCP_PM_ATTR_ADDR];
      ret = mptcp_pm_parse_entry(attr, info, false, &loc);
      if (ret < 0)
          goto set_flags_err;
 @@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
          goto set_flags_err;
      }
 +    attr_rem = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
      ret = mptcp_pm_parse_entry(attr_rem, info, false, &rem);
      if (ret < 0)
          goto set_flags_err;
 @@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
  int mptcp_userspace_pm_get_addr(struct sk_buff *skb,
                  struct genl_info *info)
  {
 -    struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
      struct mptcp_pm_addr_entry addr, *entry;
      struct mptcp_sock *msk;
      struct sk_buff *msg;
 +    struct nlattr *attr;
      int ret = -EINVAL;
      struct sock *sk;
      void *reply;
 +    if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
 +        return ret;
 +
      msk = mptcp_userspace_pm_get_sock(info);
      if (!msk)
          return ret;
      sk = (struct sock *)msk;
 +    attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
      ret = mptcp_pm_parse_entry(attr, info, false, &addr);
      if (ret < 0)
          goto out;
 --
 .47.1

-[PATCH net-next 08/15] mptcp: pm: use NL_SET_ERR_MSG_ATTR when possible
+[PATCH net-next v3 08/15] mptcp: pm: use NL_SET_ERR_MSG_ATTR when possible
 ...
 -        GENL_SET_ERR_MSG(info, "address not found");
 +        NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
          return -EINVAL;
      }
      if ((addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH) &&
-         (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
+         (entry->flags & (MPTCP_PM_ADDR_FLAG_SIGNAL |
                   MPTCP_PM_ADDR_FLAG_IMPLICIT))) {
          spin_unlock_bh(&pernet->lock);
 -        GENL_SET_ERR_MSG(info, "invalid addr flags");
 +        NL_SET_ERR_MSG_ATTR(info->extack, attr, "invalid addr flags");
          return -EINVAL;
      }
 ...

-[PATCH net-next 09/15] mptcp: pm: make three pm wrappers static
+[PATCH net-next v3 09/15] mptcp: pm: make three pm wrappers static
 From: Geliang Tang <tanggeliang@kylinos.cn>
 Three netlink functions:
     mptcp_pm_nl_get_addr_doit()
     mptcp_pm_nl_get_addr_dumpit()
     mptcp_pm_nl_set_flags_doit()
 are generic, implemented for each PM, in-kernel PM and userspace PM. It's
 clearer to move them from pm_netlink.c to pm.c.
 And the linked three path manager wrappers
     mptcp_pm_get_addr()
     mptcp_pm_dump_addr()
     mptcp_pm_set_flags()
 can be changed as static functions, no need to export them in protocol.h.
 Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
 Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 ---
  net/mptcp/pm.c         | 23 ++++++++++++++++++++---
  net/mptcp/pm_netlink.c | 16 ----------------
  net/mptcp/protocol.h   |  3 ---
 files changed, 20 insertions(+), 22 deletions(-)
 diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm.c
 +++ b/net/mptcp/pm.c
 @@ -XXX,XX +XXX,XX @@
  #include "protocol.h"
  #include "mib.h"
 +#include "mptcp_pm_gen.h"
  /* path manager command handlers */
 @@ -XXX,XX +XXX,XX @@ bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc)
      return mptcp_pm_nl_is_backup(msk, &skc_local);
  }
 -int mptcp_pm_get_addr(struct sk_buff *skb, struct genl_info *info)
 +static int mptcp_pm_get_addr(struct sk_buff *skb, struct genl_info *info)
  {
      if (info->attrs[MPTCP_PM_ATTR_TOKEN])
          return mptcp_userspace_pm_get_addr(skb, info);
      return mptcp_pm_nl_get_addr(skb, info);
  }
 -int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb)
 +int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info)
 +{
 +    return mptcp_pm_get_addr(skb, info);
 +}
 +
 +static int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb)
  {
      const struct genl_info *info = genl_info_dump(cb);
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb)
      return mptcp_pm_nl_dump_addr(msg, cb);
  }
 -int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 +int mptcp_pm_nl_get_addr_dumpit(struct sk_buff *msg,
 +                struct netlink_callback *cb)
 +{
 +    return mptcp_pm_dump_addr(msg, cb);
 +}
 +
 +static int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
  {
      if (info->attrs[MPTCP_PM_ATTR_TOKEN])
          return mptcp_userspace_pm_set_flags(skb, info);
      return mptcp_pm_nl_set_flags(skb, info);
  }
 +int mptcp_pm_nl_set_flags_doit(struct sk_buff *skb, struct genl_info *info)
 +{
 +    return mptcp_pm_set_flags(skb, info);
 +}
 +
  void mptcp_pm_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk)
  {
      struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
 diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm_netlink.c
 +++ b/net/mptcp/pm_netlink.c
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_get_addr(struct sk_buff *skb, struct genl_info *info)
      return ret;
  }
 -int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info)
 -{
 -    return mptcp_pm_get_addr(skb, info);
 -}
 -
  int mptcp_pm_nl_dump_addr(struct sk_buff *msg,
                struct netlink_callback *cb)
  {
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_dump_addr(struct sk_buff *msg,
      return msg->len;
  }
 -int mptcp_pm_nl_get_addr_dumpit(struct sk_buff *msg,
 -                struct netlink_callback *cb)
 -{
 -    return mptcp_pm_dump_addr(msg, cb);
 -}
 -
  static int parse_limit(struct genl_info *info, int id, unsigned int *limit)
  {
      struct nlattr *attr = info->attrs[id];
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info)
      return 0;
  }
 -int mptcp_pm_nl_set_flags_doit(struct sk_buff *skb, struct genl_info *info)
 -{
 -    return mptcp_pm_set_flags(skb, info);
 -}
 -
  static void mptcp_nl_mcast_send(struct net *net, struct sk_buff *nlskb, gfp_t gfp)
  {
      genlmsg_multicast_netns(&mptcp_genl_family, net,
 diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/protocol.h
 +++ b/net/mptcp/protocol.h
 @@ -XXX,XX +XXX,XX @@ bool mptcp_lookup_subflow_by_saddr(const struct list_head *list,
                     const struct mptcp_addr_info *saddr);
  bool mptcp_remove_anno_list_by_saddr(struct mptcp_sock *msk,
                       const struct mptcp_addr_info *addr);
 -int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info);
  int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info);
  int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info);
  int mptcp_pm_announce_addr(struct mptcp_sock *msk,
 @@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_in
  bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc);
  bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
  bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
 -int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb);
  int mptcp_pm_nl_dump_addr(struct sk_buff *msg,
                struct netlink_callback *cb);
  int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
                   struct netlink_callback *cb);
 -int mptcp_pm_get_addr(struct sk_buff *skb, struct genl_info *info);
  int mptcp_pm_nl_get_addr(struct sk_buff *skb, struct genl_info *info);
  int mptcp_userspace_pm_get_addr(struct sk_buff *skb,
                  struct genl_info *info);
 --
 .47.1

-[PATCH net-next 10/15] mptcp: pm: drop skb parameter of get_addr
+[PATCH net-next v3 10/15] mptcp: pm: drop skb parameter of get_addr
 From: Geliang Tang <tanggeliang@kylinos.cn>
 The first parameters 'skb' of get_addr() interfaces are now useless
 since mptcp_userspace_pm_get_sock() helper is used. This patch drops
 these useless parameters of them.
 Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
 Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 ---
  net/mptcp/pm.c           | 8 ++++----
  net/mptcp/pm_netlink.c   | 2 +-
  net/mptcp/pm_userspace.c | 3 +--
  net/mptcp/protocol.h     | 5 ++---
 files changed, 8 insertions(+), 10 deletions(-)
 diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm.c
 +++ b/net/mptcp/pm.c
 @@ -XXX,XX +XXX,XX @@ bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc)
      return mptcp_pm_nl_is_backup(msk, &skc_local);
  }
 -static int mptcp_pm_get_addr(struct sk_buff *skb, struct genl_info *info)
 +static int mptcp_pm_get_addr(struct genl_info *info)
  {
      if (info->attrs[MPTCP_PM_ATTR_TOKEN])
 -        return mptcp_userspace_pm_get_addr(skb, info);
 -    return mptcp_pm_nl_get_addr(skb, info);
 +        return mptcp_userspace_pm_get_addr(info);
 +    return mptcp_pm_nl_get_addr(info);
  }
  int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info)
  {
 -    return mptcp_pm_get_addr(skb, info);
 +    return mptcp_pm_get_addr(info);
  }
  static int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb)
 diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm_netlink.c
 +++ b/net/mptcp/pm_netlink.c
 @@ -XXX,XX +XXX,XX @@ int mptcp_nl_fill_addr(struct sk_buff *skb,
      return -EMSGSIZE;
  }
 -int mptcp_pm_nl_get_addr(struct sk_buff *skb, struct genl_info *info)
 +int mptcp_pm_nl_get_addr(struct genl_info *info)
  {
      struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
      struct mptcp_pm_addr_entry addr, *entry;
 diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm_userspace.c
 +++ b/net/mptcp/pm_userspace.c
 @@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
      return ret;
  }
 -int mptcp_userspace_pm_get_addr(struct sk_buff *skb,
 -                struct genl_info *info)
 +int mptcp_userspace_pm_get_addr(struct genl_info *info)
  {
      struct mptcp_pm_addr_entry addr, *entry;
      struct mptcp_sock *msk;
 diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/protocol.h
 +++ b/net/mptcp/protocol.h
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_dump_addr(struct sk_buff *msg,
                struct netlink_callback *cb);
  int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
                   struct netlink_callback *cb);
 -int mptcp_pm_nl_get_addr(struct sk_buff *skb, struct genl_info *info);
 -int mptcp_userspace_pm_get_addr(struct sk_buff *skb,
 -                struct genl_info *info);
 +int mptcp_pm_nl_get_addr(struct genl_info *info);
 +int mptcp_userspace_pm_get_addr(struct genl_info *info);
  static inline u8 subflow_get_local_id(const struct mptcp_subflow_context *subflow)
  {
 --
 .47.1

-[PATCH net-next 11/15] mptcp: pm: add id parameter for get_addr
+[PATCH net-next v3 11/15] mptcp: pm: add id parameter for get_addr
 ...
 Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
 Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 ---
+v2:
+- Fix 'attr' no longer being set in mptcp_pm_nl_get_addr(), but still
+  used in this patch (no longer in the next one). (Simon)
+v3:
+- Same fix, but in mptcp_userspace_pm_get_addr().
+---
  net/mptcp/pm.c           | 20 ++++++++++++++++----
- net/mptcp/pm_netlink.c   | 14 +++-----------
+ net/mptcp/pm_netlink.c   | 16 ++++------------
- net/mptcp/pm_userspace.c | 14 +++-----------
+ net/mptcp/pm_userspace.c | 16 ++++------------
  net/mptcp/protocol.h     |  4 ++--
-files changed, 24 insertions(+), 28 deletions(-)
+files changed, 26 insertions(+), 30 deletions(-)
 diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm.c
 +++ b/net/mptcp/pm.c
 ...
  }
 -int mptcp_pm_nl_get_addr(struct genl_info *info)
 +int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info)
  {
++    struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
      struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
 -    struct mptcp_pm_addr_entry addr, *entry;
 +    struct mptcp_pm_addr_entry *entry;
      struct sk_buff *msg;
-     struct nlattr *attr;
+-    struct nlattr *attr;
      void *reply;
      int ret;
 -    if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
 -        return -EINVAL;
 ...
 -int mptcp_userspace_pm_get_addr(struct genl_info *info)
 +int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info)
  {
 -    struct mptcp_pm_addr_entry addr, *entry;
++    struct nlattr *attr = attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 +    struct mptcp_pm_addr_entry *entry;
      struct mptcp_sock *msk;
      struct sk_buff *msg;
-     struct nlattr *attr;
+-    struct nlattr *attr;
-@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_get_addr(struct genl_info *info)
+     int ret = -EINVAL;
      struct sock *sk;
      void *reply;
 -    if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
 -        return ret;
 ...

-[PATCH net-next 12/15] mptcp: pm: reuse sending nlmsg code in get_addr
+[PATCH net-next v3 12/15] mptcp: pm: reuse sending nlmsg code in get_addr
 ...
 -int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info)
 +int mptcp_pm_nl_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
 +             struct genl_info *info)
  {
+-    struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
      struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
      struct mptcp_pm_addr_entry *entry;
 -    struct sk_buff *msg;
--    struct nlattr *attr;
 -    void *reply;
 -    int ret;
 -
 -    msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 -    if (!msg)
 ...
 -int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info)
 +int mptcp_userspace_pm_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
 +                struct genl_info *info)
  {
+-    struct nlattr *attr = attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
      struct mptcp_pm_addr_entry *entry;
      struct mptcp_sock *msk;
 -    struct sk_buff *msg;
--    struct nlattr *attr;
      int ret = -EINVAL;
      struct sock *sk;
 -    void *reply;
      msk = mptcp_userspace_pm_get_sock(info);
 ...

-[PATCH net-next 13/15] mptcp: pm: drop skb parameter of set_flags
+[PATCH net-next v3 13/15] mptcp: pm: drop skb parameter of set_flags
 From: Geliang Tang <tanggeliang@kylinos.cn>
 The first parameter 'skb' in mptcp_pm_nl_set_flags() is only used to
 obtained the network namespace, which can also be obtained through the
 second parameters 'info' by using genl_info_net() helper.
 This patch drops these useless parameters 'skb' in all three set_flags()
 interfaces.
 Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
 Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 ---
  net/mptcp/pm.c           | 8 ++++----
  net/mptcp/pm_netlink.c   | 4 ++--
  net/mptcp/pm_userspace.c | 2 +-
  net/mptcp/protocol.h     | 4 ++--
 files changed, 9 insertions(+), 9 deletions(-)
 diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm.c
 +++ b/net/mptcp/pm.c
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_get_addr_dumpit(struct sk_buff *msg,
      return mptcp_pm_dump_addr(msg, cb);
  }
 -static int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 +static int mptcp_pm_set_flags(struct genl_info *info)
  {
      if (info->attrs[MPTCP_PM_ATTR_TOKEN])
 -        return mptcp_userspace_pm_set_flags(skb, info);
 -    return mptcp_pm_nl_set_flags(skb, info);
 +        return mptcp_userspace_pm_set_flags(info);
 +    return mptcp_pm_nl_set_flags(info);
  }
  int mptcp_pm_nl_set_flags_doit(struct sk_buff *skb, struct genl_info *info)
  {
 -    return mptcp_pm_set_flags(skb, info);
 +    return mptcp_pm_set_flags(info);
  }
  void mptcp_pm_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk)
 diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm_netlink.c
 +++ b/net/mptcp/pm_netlink.c
 @@ -XXX,XX +XXX,XX @@ static int mptcp_nl_set_flags(struct net *net,
      return ret;
  }
 -int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info)
 +int mptcp_pm_nl_set_flags(struct genl_info *info)
  {
      struct mptcp_pm_addr_entry addr = { .addr = { .family = AF_UNSPEC }, };
      u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP |
                 MPTCP_PM_ADDR_FLAG_FULLMESH;
 -    struct net *net = sock_net(skb->sk);
 +    struct net *net = genl_info_net(info);
      struct mptcp_pm_addr_entry *entry;
      struct pm_nl_pernet *pernet;
      struct nlattr *attr;
 diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm_userspace.c
 +++ b/net/mptcp/pm_userspace.c
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
      return err;
  }
 -int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 +int mptcp_userspace_pm_set_flags(struct genl_info *info)
  {
      struct mptcp_pm_addr_entry loc = { .addr = { .family = AF_UNSPEC }, };
      struct mptcp_pm_addr_entry rem = { .addr = { .family = AF_UNSPEC }, };
 diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/protocol.h
 +++ b/net/mptcp/protocol.h
 @@ -XXX,XX +XXX,XX @@ bool mptcp_lookup_subflow_by_saddr(const struct list_head *list,
                     const struct mptcp_addr_info *saddr);
  bool mptcp_remove_anno_list_by_saddr(struct mptcp_sock *msk,
                       const struct mptcp_addr_info *addr);
 -int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info);
 -int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info);
 +int mptcp_pm_nl_set_flags(struct genl_info *info);
 +int mptcp_userspace_pm_set_flags(struct genl_info *info);
  int mptcp_pm_announce_addr(struct mptcp_sock *msk,
                 const struct mptcp_addr_info *addr,
                 bool echo);
 --
 .47.1

-[PATCH net-next 14/15] mptcp: pm: change rem type of set_flags
+[PATCH net-next v3 14/15] mptcp: pm: change rem type of set_flags
 From: Geliang Tang <tanggeliang@kylinos.cn>
 Generally, in the path manager interfaces, the local address is defined
 as an mptcp_pm_addr_entry type address, while the remote address is
 defined as an mptcp_addr_info type one:
     (struct mptcp_pm_addr_entry *local, struct mptcp_addr_info *remote)
 But the set_flags() interface uses two mptcp_pm_addr_entry type
 parameters.
 This patch changes the second one to mptcp_addr_info type and use helper
 mptcp_pm_parse_addr() to parse it instead of using mptcp_pm_parse_entry().
 Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
 Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
 ---
  net/mptcp/pm_userspace.c | 8 ++++----
 file changed, 4 insertions(+), 4 deletions(-)
 diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
 index XXXXXXX..XXXXXXX 100644
 --- a/net/mptcp/pm_userspace.c
 +++ b/net/mptcp/pm_userspace.c
 @@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
  int mptcp_userspace_pm_set_flags(struct genl_info *info)
  {
      struct mptcp_pm_addr_entry loc = { .addr = { .family = AF_UNSPEC }, };
 -    struct mptcp_pm_addr_entry rem = { .addr = { .family = AF_UNSPEC }, };
 +    struct mptcp_addr_info rem = { .family = AF_UNSPEC, };
      struct mptcp_pm_addr_entry *entry;
      struct nlattr *attr, *attr_rem;
      struct mptcp_sock *msk;
 @@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct genl_info *info)
      }
      attr_rem = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
 -    ret = mptcp_pm_parse_entry(attr_rem, info, false, &rem);
 +    ret = mptcp_pm_parse_addr(attr_rem, info, &rem);
      if (ret < 0)
          goto set_flags_err;
 -    if (rem.addr.family == AF_UNSPEC) {
 +    if (rem.family == AF_UNSPEC) {
          NL_SET_ERR_MSG_ATTR(info->extack, attr_rem,
                      "invalid remote address family");
          ret = -EINVAL;
 @@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct genl_info *info)
      spin_unlock_bh(&msk->pm.lock);
      lock_sock(sk);
 -    ret = mptcp_pm_nl_mp_prio_send_ack(msk, &loc.addr, &rem.addr, bkup);
 +    ret = mptcp_pm_nl_mp_prio_send_ack(msk, &loc.addr, &rem, bkup);
      release_sock(sk);
      /* mptcp_pm_nl_mp_prio_send_ack() only fails in one case */
 --
 .47.1

-[PATCH net-next 15/15] mptcp: pm: add local parameter for set_flags
+[PATCH net-next v3 15/15] mptcp: pm: add local parameter for set_flags
 ...
          NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
          return -EINVAL;
      }
 -    if ((addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH) &&
 +    if ((local->flags & MPTCP_PM_ADDR_FLAG_FULLMESH) &&
-         (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
+         (entry->flags & (MPTCP_PM_ADDR_FLAG_SIGNAL |
                   MPTCP_PM_ADDR_FLAG_IMPLICIT))) {
          spin_unlock_bh(&pernet->lock);
-         NL_SET_ERR_MSG_ATTR(info->extack, attr, "invalid addr flags");
+@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_set_flags(struct genl_info *info)
          return -EINVAL;
      }
 -    changed = (addr.flags ^ entry->flags) & mask;
 -    entry->flags = (entry->flags & ~mask) | (addr.flags & mask);
 ...

These cleanups lead the way to the unification of the path-manager
interfaces, and allow future extensions. The following patches are not
all linked to each others, but are all related to the path-managers.

- Patch 1: drop unneeded parameter in a function helper.

- Patch 2: clearer NL error message when an NL attribute is missing.

- Patch 3: more precise NL messages by avoiding 'this or that is NOK'.

- Patch 4: improve too vague or missing NL err messages.

- Patch 5: use GENL_REQ_ATTR_CHECK to look for mandatory NL attributes.

- Patch 6: avoid overriding the error message.

- Patch 7: check all mandatory NL attributes with GENL_REQ_ATTR_CHECK.

- Patch 8: use NL_SET_ERR_MSG_ATTR instead of GENL_SET_ERR_MSG

- Patch 9: move doit callbacks used for both PM to pm.c.

- Patch 10: drop another unneeded parameter in a function helper.

- Patch 11: share the ID parsing code for the 'get_addr' callback.

- Patch 12: share sending NL code for the 'get_addr' callback.

- Patch 13: drop yet another unneeded parameter in a function helper.

- Patch 14: pick the usual structure type for the remote address.

- Patch 15: share the local addr parsing code for the 'set_flags' cb.

The behaviour when there are no errors should then not be modified.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Geliang Tang (9):
      mptcp: pm: drop info of userspace_pm_remove_id_zero_address
      mptcp: pm: userspace: use GENL_REQ_ATTR_CHECK
      mptcp: pm: make three pm wrappers static
      mptcp: pm: drop skb parameter of get_addr
      mptcp: pm: add id parameter for get_addr
      mptcp: pm: reuse sending nlmsg code in get_addr
      mptcp: pm: drop skb parameter of set_flags
      mptcp: pm: change rem type of set_flags
      mptcp: pm: add local parameter for set_flags

Matthieu Baerts (NGI0) (6):
      mptcp: pm: userspace: flags: clearer msg if no remote addr
      mptcp: pm: more precise error messages
      mptcp: pm: improve error messages
      mptcp: pm: remove duplicated error messages
      mptcp: pm: mark missing address attributes
      mptcp: pm: use NL_SET_ERR_MSG_ATTR when possible

net/mptcp/pm.c           |  86 +++++++++++++++++--
 net/mptcp/pm_netlink.c   | 129 ++++++++++-------------------
 net/mptcp/pm_userspace.c | 209 +++++++++++++++++++++--------------------------
 net/mptcp/protocol.h     |  14 ++--
 4 files changed, 225 insertions(+), 213 deletions(-)
---
base-commit: b44e27b4df1a1cd3fd84cf26c82156ed0301575f
change-id: 20250116-net-next-mptcp-pm-misc-cleanup-2-b0f50eff8084

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>

From: Geliang Tang <tanggeliang@kylinos.cn>

The only use of 'info' parameter of userspace_pm_remove_id_zero_address()
is to set an error message into it.

Plus, this helper will only fail when it cannot find any subflows with a
local address ID 0.

This patch drops this parameter and sets the error message where this
function is called in mptcp_pm_nl_remove_doit().

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 	return err;
 }
 
-static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
-						     struct genl_info *info)
+static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk)
 {
 	struct mptcp_rm_list list = { .nr = 0 };
 	struct mptcp_subflow_context *subflow;
@@ -XXX,XX +XXX,XX @@ static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
 			break;
 		}
 	}
-	if (!has_id_0) {
-		GENL_SET_ERR_MSG(info, "address with id 0 not found");
+	if (!has_id_0)
 		goto remove_err;
-	}
 
 	list.ids[list.nr++] = 0;
 
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 	sk = (struct sock *)msk;
 
 	if (id_val == 0) {
-		err = mptcp_userspace_pm_remove_id_zero_address(msk, info);
+		err = mptcp_userspace_pm_remove_id_zero_address(msk);
 		goto out;
 	}
 
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 	spin_lock_bh(&msk->pm.lock);
 	match = mptcp_userspace_pm_lookup_addr_by_id(msk, id_val);
 	if (!match) {
-		GENL_SET_ERR_MSG(info, "address with specified id not found");
 		spin_unlock_bh(&msk->pm.lock);
 		release_sock(sk);
 		goto out;
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 
 	err = 0;
 out:
+	if (err)
+		GENL_SET_ERR_MSG_FMT(info,
+				     "address with id %u not found",
+				     id_val);
+
 	sock_put(sk);
 	return err;
 }

-- 
2.47.1

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.

Reviewed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_userspace.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ 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;
-	}
+	ret = mptcp_pm_parse_entry(attr_rem, info, false, &rem);
+	if (ret < 0)
+		goto set_flags_err;
 
 	if (loc.addr.family == AF_UNSPEC ||
 	    rem.addr.family == AF_UNSPEC) {

-- 
2.47.1

Some errors reported by the userspace PM were vague: "this or that is
invalid".

It is easier for the userspace to know which part is wrong, instead of
having to guess that.

While at it, in mptcp_userspace_pm_set_flags() move the parsing after
the check linked to the local attribute.

Reviewed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_userspace.c | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 		goto announce_err;
 	}
 
-	if (addr_val.addr.id == 0 || !(addr_val.flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
-		GENL_SET_ERR_MSG(info, "invalid addr id or flags");
+	if (addr_val.addr.id == 0) {
+		GENL_SET_ERR_MSG(info, "invalid addr id");
+		err = -EINVAL;
+		goto announce_err;
+	}
+
+	if (!(addr_val.flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
+		GENL_SET_ERR_MSG(info, "invalid addr flags");
 		err = -EINVAL;
 		goto announce_err;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 		goto destroy_err;
 	}
 
-	if (!addr_l.addr.port || !addr_r.port) {
-		GENL_SET_ERR_MSG(info, "missing local or remote port");
+	if (!addr_l.addr.port) {
+		GENL_SET_ERR_MSG(info, "missing local port");
+		err = -EINVAL;
+		goto destroy_err;
+	}
+
+	if (!addr_r.port) {
+		GENL_SET_ERR_MSG(info, "missing remote port");
 		err = -EINVAL;
 		goto destroy_err;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 	if (ret < 0)
 		goto set_flags_err;
 
+	if (loc.addr.family == AF_UNSPEC) {
+		GENL_SET_ERR_MSG(info, "invalid local address family");
+		ret = -EINVAL;
+		goto set_flags_err;
+	}
+
 	ret = mptcp_pm_parse_entry(attr_rem, info, false, &rem);
 	if (ret < 0)
 		goto set_flags_err;
 
-	if (loc.addr.family == AF_UNSPEC ||
-	    rem.addr.family == AF_UNSPEC) {
-		GENL_SET_ERR_MSG(info, "invalid address families");
+	if (rem.addr.family == AF_UNSPEC) {
+		GENL_SET_ERR_MSG(info, "invalid remote address family");
 		ret = -EINVAL;
 		goto set_flags_err;
 	}

-- 
2.47.1

Some error messages were:

- too generic: "missing input", "invalid request"

- not precise enough: "limit greater than maximum" but what's the max?

- missing: subflow not found, or connect error.

This can be easily improved by being more precise, or adding new error
messages.

Reviewed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_netlink.c   |  6 ++++--
 net/mptcp/pm_userspace.c | 10 +++++++++-
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -XXX,XX +XXX,XX @@ static int parse_limit(struct genl_info *info, int id, unsigned int *limit)
 
 	*limit = nla_get_u32(attr);
 	if (*limit > MPTCP_PM_ADDR_MAX) {
-		GENL_SET_ERR_MSG(info, "limit greater than maximum");
+		NL_SET_ERR_MSG_ATTR_FMT(info->extack, attr,
+					"limit greater than maximum (%u)",
+					MPTCP_PM_ADDR_MAX);
 		return -EINVAL;
 	}
 	return 0;
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info)
 	if (addr.addr.family == AF_UNSPEC) {
 		lookup_by_id = 1;
 		if (!addr.addr.id) {
-			GENL_SET_ERR_MSG(info, "missing required inputs");
+			GENL_SET_ERR_MSG(info, "missing address ID");
 			return -EOPNOTSUPP;
 		}
 	}
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *in
 	}
 
 	if (!mptcp_pm_is_userspace(msk)) {
-		GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected");
+		GENL_SET_ERR_MSG(info, "userspace PM not selected");
 		sock_put((struct sock *)msk);
 		return NULL;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 	err = __mptcp_subflow_connect(sk, &local, &addr_r);
 	release_sock(sk);
 
+	if (err)
+		GENL_SET_ERR_MSG_FMT(info, "connect error: %d", err);
+
 	spin_lock_bh(&msk->pm.lock);
 	if (err)
 		mptcp_userspace_pm_delete_local_addr(msk, &entry);
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 	lock_sock(sk);
 	ssk = mptcp_nl_find_ssk(msk, &addr_l.addr, &addr_r);
 	if (!ssk) {
+		GENL_SET_ERR_MSG(info, "subflow not found");
 		err = -ESRCH;
 		goto release_sock;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 	ret = mptcp_pm_nl_mp_prio_send_ack(msk, &loc.addr, &rem.addr, bkup);
 	release_sock(sk);
 
+	/* mptcp_pm_nl_mp_prio_send_ack() only fails in one case */
+	if (ret < 0)
+		GENL_SET_ERR_MSG(info, "subflow not found");
+
 set_flags_err:
 	sock_put(sk);
 	return ret;

-- 
2.47.1

From: Geliang Tang <tanggeliang@kylinos.cn>

A more general way to check if MPTCP_PM_ATTR_* exists in 'info'
is to use GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_*) instead of
directly reading info->attrs[MPTCP_PM_ATTR_*] and then checking
if it's NULL.

So this patch uses GENL_REQ_ATTR_CHECK() for userspace PM in
mptcp_pm_nl_announce_doit(), mptcp_pm_nl_remove_doit(),
mptcp_pm_nl_subflow_create_doit(), mptcp_pm_nl_subflow_destroy_doit()
and mptcp_userspace_pm_get_sock().

Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_userspace.c | 41 +++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk,
 
 static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *info)
 {
-	struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
 	struct mptcp_sock *msk;
+	struct nlattr *token;
 
-	if (!token) {
-		GENL_SET_ERR_MSG(info, "missing required token");
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_TOKEN))
 		return NULL;
-	}
 
+	token = info->attrs[MPTCP_PM_ATTR_TOKEN];
 	msk = mptcp_token_get_sock(genl_info_net(info), nla_get_u32(token));
 	if (!msk) {
 		NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token");
@@ -XXX,XX +XXX,XX @@ static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *in
 
 int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	struct nlattr *addr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	struct mptcp_pm_addr_entry addr_val;
 	struct mptcp_sock *msk;
+	struct nlattr *addr;
 	int err = -EINVAL;
 	struct sock *sk;
 
-	if (!addr) {
-		GENL_SET_ERR_MSG(info, "missing required address");
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR))
 		return err;
-	}
 
 	msk = mptcp_userspace_pm_get_sock(info);
 	if (!msk)
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 
 	sk = (struct sock *)msk;
 
+	addr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	err = mptcp_pm_parse_entry(addr, info, true, &addr_val);
 	if (err < 0) {
 		GENL_SET_ERR_MSG(info, "error parsing local address");
@@ -XXX,XX +XXX,XX @@ void mptcp_pm_remove_addr_entry(struct mptcp_sock *msk,
 
 int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	struct nlattr *id = info->attrs[MPTCP_PM_ATTR_LOC_ID];
 	struct mptcp_pm_addr_entry *match;
 	struct mptcp_sock *msk;
+	struct nlattr *id;
 	int err = -EINVAL;
 	struct sock *sk;
 	u8 id_val;
 
-	if (!id) {
-		GENL_SET_ERR_MSG(info, "missing required ID");
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_LOC_ID))
 		return err;
-	}
 
+	id = info->attrs[MPTCP_PM_ATTR_LOC_ID];
 	id_val = nla_get_u8(id);
 
 	msk = mptcp_userspace_pm_get_sock(info);
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 
 int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	struct nlattr *raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
-	struct nlattr *laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	struct mptcp_pm_addr_entry entry = { 0 };
 	struct mptcp_addr_info addr_r;
+	struct nlattr *raddr, *laddr;
 	struct mptcp_pm_local local;
 	struct mptcp_sock *msk;
 	int err = -EINVAL;
 	struct sock *sk;
 
-	if (!laddr || !raddr) {
-		GENL_SET_ERR_MSG(info, "missing required address(es)");
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR) ||
+	    GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE))
 		return err;
-	}
 
 	msk = mptcp_userspace_pm_get_sock(info);
 	if (!msk)
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 
 	sk = (struct sock *)msk;
 
+	laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	err = mptcp_pm_parse_entry(laddr, info, true, &entry);
 	if (err < 0) {
 		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr");
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 	}
 	entry.flags |= MPTCP_PM_ADDR_FLAG_SUBFLOW;
 
+	raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
 	err = mptcp_pm_parse_addr(raddr, info, &addr_r);
 	if (err < 0) {
 		NL_SET_ERR_MSG_ATTR(info->extack, raddr, "error parsing remote addr");
@@ -XXX,XX +XXX,XX @@ static struct sock *mptcp_nl_find_ssk(struct mptcp_sock *msk,
 
 int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	struct nlattr *raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
-	struct nlattr *laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	struct mptcp_pm_addr_entry addr_l;
 	struct mptcp_addr_info addr_r;
+	struct nlattr *raddr, *laddr;
 	struct mptcp_sock *msk;
 	struct sock *sk, *ssk;
 	int err = -EINVAL;
 
-	if (!laddr || !raddr) {
-		GENL_SET_ERR_MSG(info, "missing required address(es)");
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR) ||
+	    GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE))
 		return err;
-	}
 
 	msk = mptcp_userspace_pm_get_sock(info);
 	if (!msk)
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 
 	sk = (struct sock *)msk;
 
+	laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	err = mptcp_pm_parse_entry(laddr, info, true, &addr_l);
 	if (err < 0) {
 		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr");
 		goto destroy_err;
 	}
 
+	raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
 	err = mptcp_pm_parse_addr(raddr, info, &addr_r);
 	if (err < 0) {
 		NL_SET_ERR_MSG_ATTR(info->extack, raddr, "error parsing remote addr");

-- 
2.47.1

mptcp_pm_parse_entry() and mptcp_pm_parse_addr() will already set a
error message in case of parsing issue.

Then, no need to override this error message with another less precise
one: "error parsing address".

Reviewed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_userspace.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 
 	addr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	err = mptcp_pm_parse_entry(addr, info, true, &addr_val);
-	if (err < 0) {
-		GENL_SET_ERR_MSG(info, "error parsing local address");
+	if (err < 0)
 		goto announce_err;
-	}
 
 	if (addr_val.addr.id == 0) {
 		GENL_SET_ERR_MSG(info, "invalid addr id");
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 
 	laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	err = mptcp_pm_parse_entry(laddr, info, true, &entry);
-	if (err < 0) {
-		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr");
+	if (err < 0)
 		goto create_err;
-	}
 
 	if (entry.flags & MPTCP_PM_ADDR_FLAG_SIGNAL) {
 		GENL_SET_ERR_MSG(info, "invalid addr flags");
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 
 	raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
 	err = mptcp_pm_parse_addr(raddr, info, &addr_r);
-	if (err < 0) {
-		NL_SET_ERR_MSG_ATTR(info->extack, raddr, "error parsing remote addr");
+	if (err < 0)
 		goto create_err;
-	}
 
 	if (!mptcp_pm_addr_families_match(sk, &entry.addr, &addr_r)) {
 		GENL_SET_ERR_MSG(info, "families mismatch");
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 
 	laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	err = mptcp_pm_parse_entry(laddr, info, true, &addr_l);
-	if (err < 0) {
-		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr");
+	if (err < 0)
 		goto destroy_err;
-	}
 
 	raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
 	err = mptcp_pm_parse_addr(raddr, info, &addr_r);
-	if (err < 0) {
-		NL_SET_ERR_MSG_ATTR(info->extack, raddr, "error parsing remote addr");
+	if (err < 0)
 		goto destroy_err;
-	}
 
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
 	if (addr_l.addr.family == AF_INET && ipv6_addr_v4mapped(&addr_r.addr6)) {

-- 
2.47.1

mptcp_pm_parse_entry() will check if the given attribute is defined. If
not, it will return a generic error: "missing address info".

It might then not be clear for the userspace developer which attribute
is missing, especially when the command takes multiple addresses.

By using GENL_REQ_ATTR_CHECK(), the userspace will get a hint about
which attribute is missing, making thing clearer. Note that this is what
was already done for most of the other MPTCP NL commands, this patch
simply adds the missing ones.

Reviewed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_netlink.c   | 24 ++++++++++++++++++++----
 net/mptcp/pm_userspace.c | 15 ++++++++++++---
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -XXX,XX +XXX,XX @@ static bool mptcp_pm_has_addr_attr_id(const struct nlattr *attr,
 
 int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
 	struct mptcp_pm_addr_entry addr, *entry;
+	struct nlattr *attr;
 	int ret;
 
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
+		return -EINVAL;
+
+	attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	ret = mptcp_pm_parse_entry(attr, info, true, &addr);
 	if (ret < 0)
 		return ret;
@@ -XXX,XX +XXX,XX @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
 
 int mptcp_pm_nl_del_addr_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
 	struct mptcp_pm_addr_entry addr, *entry;
 	unsigned int addr_max;
+	struct nlattr *attr;
 	int ret;
 
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
+		return -EINVAL;
+
+	attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	ret = mptcp_pm_parse_entry(attr, info, false, &addr);
 	if (ret < 0)
 		return ret;
@@ -XXX,XX +XXX,XX @@ int mptcp_nl_fill_addr(struct sk_buff *skb,
 
 int mptcp_pm_nl_get_addr(struct sk_buff *skb, struct genl_info *info)
 {
-	struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
 	struct mptcp_pm_addr_entry addr, *entry;
 	struct sk_buff *msg;
+	struct nlattr *attr;
 	void *reply;
 	int ret;
 
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
+		return -EINVAL;
+
+	attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	ret = mptcp_pm_parse_entry(attr, info, false, &addr);
 	if (ret < 0)
 		return ret;
@@ -XXX,XX +XXX,XX @@ static int mptcp_nl_set_flags(struct net *net,
 int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info)
 {
 	struct mptcp_pm_addr_entry addr = { .addr = { .family = AF_UNSPEC }, };
-	struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP |
 			   MPTCP_PM_ADDR_FLAG_FULLMESH;
 	struct net *net = sock_net(skb->sk);
 	struct mptcp_pm_addr_entry *entry;
 	struct pm_nl_pernet *pernet;
+	struct nlattr *attr;
 	u8 lookup_by_id = 0;
 	u8 bkup = 0;
 	int ret;
 
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR))
+		return -EINVAL;
+
 	pernet = pm_nl_get_pernet(net);
 
+	attr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	ret = mptcp_pm_parse_entry(attr, info, false, &addr);
 	if (ret < 0)
 		return ret;
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 {
 	struct mptcp_pm_addr_entry loc = { .addr = { .family = AF_UNSPEC }, };
 	struct mptcp_pm_addr_entry rem = { .addr = { .family = AF_UNSPEC }, };
-	struct nlattr *attr_rem = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
-	struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	struct mptcp_pm_addr_entry *entry;
+	struct nlattr *attr, *attr_rem;
 	struct mptcp_sock *msk;
 	int ret = -EINVAL;
 	struct sock *sk;
 	u8 bkup = 0;
 
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR) ||
+	    GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE))
+		return ret;
+
 	msk = mptcp_userspace_pm_get_sock(info);
 	if (!msk)
 		return ret;
 
 	sk = (struct sock *)msk;
 
+	attr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	ret = mptcp_pm_parse_entry(attr, info, false, &loc);
 	if (ret < 0)
 		goto set_flags_err;
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 		goto set_flags_err;
 	}
 
+	attr_rem = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
 	ret = mptcp_pm_parse_entry(attr_rem, info, false, &rem);
 	if (ret < 0)
 		goto set_flags_err;
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
 int mptcp_userspace_pm_get_addr(struct sk_buff *skb,
 				struct genl_info *info)
 {
-	struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	struct mptcp_pm_addr_entry addr, *entry;
 	struct mptcp_sock *msk;
 	struct sk_buff *msg;
+	struct nlattr *attr;
 	int ret = -EINVAL;
 	struct sock *sk;
 	void *reply;
 
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
+		return ret;
+
 	msk = mptcp_userspace_pm_get_sock(info);
 	if (!msk)
 		return ret;
 
 	sk = (struct sock *)msk;
 
+	attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	ret = mptcp_pm_parse_entry(attr, info, false, &addr);
 	if (ret < 0)
 		goto out;

-- 
2.47.1

Instead of only returning a text message with GENL_SET_ERR_MSG(),
NL_SET_ERR_MSG_ATTR() can help the userspace developers by also
reporting which attribute is faulty.

When the error is specific to an attribute, NL_SET_ERR_MSG_ATTR() is now
used. The error messages have not been modified in this commit.

Reviewed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_netlink.c   | 20 ++++++++++++--------
 net/mptcp/pm_userspace.c | 33 +++++++++++++++++++--------------
 2 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
 		return ret;
 
 	if (addr.addr.port && !address_use_port(&addr)) {
-		GENL_SET_ERR_MSG(info, "flags must have signal and not subflow when using port");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr,
+				    "flags must have signal and not subflow when using port");
 		return -EINVAL;
 	}
 
 	if (addr.flags & MPTCP_PM_ADDR_FLAG_SIGNAL &&
 	    addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH) {
-		GENL_SET_ERR_MSG(info, "flags mustn't have both signal and fullmesh");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr,
+				    "flags mustn't have both signal and fullmesh");
 		return -EINVAL;
 	}
 
 	if (addr.flags & MPTCP_PM_ADDR_FLAG_IMPLICIT) {
-		GENL_SET_ERR_MSG(info, "can't create IMPLICIT endpoint");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr,
+				    "can't create IMPLICIT endpoint");
 		return -EINVAL;
 	}
 
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_del_addr_doit(struct sk_buff *skb, struct genl_info *info)
 	spin_lock_bh(&pernet->lock);
 	entry = __lookup_addr_by_id(pernet, addr.addr.id);
 	if (!entry) {
-		GENL_SET_ERR_MSG(info, "address not found");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		spin_unlock_bh(&pernet->lock);
 		return -EINVAL;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_get_addr(struct sk_buff *skb, struct genl_info *info)
 	rcu_read_lock();
 	entry = __lookup_addr_by_id(pernet, addr.addr.id);
 	if (!entry) {
-		GENL_SET_ERR_MSG(info, "address not found");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		ret = -EINVAL;
 		goto unlock_fail;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info)
 	if (addr.addr.family == AF_UNSPEC) {
 		lookup_by_id = 1;
 		if (!addr.addr.id) {
-			GENL_SET_ERR_MSG(info, "missing address ID");
+			NL_SET_ERR_MSG_ATTR(info->extack, attr,
+					    "missing address ID");
 			return -EOPNOTSUPP;
 		}
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info)
 			       __lookup_addr(pernet, &addr.addr);
 	if (!entry) {
 		spin_unlock_bh(&pernet->lock);
-		GENL_SET_ERR_MSG(info, "address not found");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		return -EINVAL;
 	}
 	if ((addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH) &&
 	    (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
 		spin_unlock_bh(&pernet->lock);
-		GENL_SET_ERR_MSG(info, "invalid addr flags");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr, "invalid addr flags");
 		return -EINVAL;
 	}
 
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *in
 	}
 
 	if (!mptcp_pm_is_userspace(msk)) {
-		GENL_SET_ERR_MSG(info, "userspace PM not selected");
+		NL_SET_ERR_MSG_ATTR(info->extack, token,
+				    "userspace PM not selected");
 		sock_put((struct sock *)msk);
 		return NULL;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 		goto announce_err;
 
 	if (addr_val.addr.id == 0) {
-		GENL_SET_ERR_MSG(info, "invalid addr id");
+		NL_SET_ERR_MSG_ATTR(info->extack, addr, "invalid addr id");
 		err = -EINVAL;
 		goto announce_err;
 	}
 
 	if (!(addr_val.flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
-		GENL_SET_ERR_MSG(info, "invalid addr flags");
+		NL_SET_ERR_MSG_ATTR(info->extack, addr, "invalid addr flags");
 		err = -EINVAL;
 		goto announce_err;
 	}
 
 	err = mptcp_userspace_pm_append_new_local_addr(msk, &addr_val, false);
 	if (err < 0) {
-		GENL_SET_ERR_MSG(info, "did not match address and id");
+		NL_SET_ERR_MSG_ATTR(info->extack, addr,
+				    "did not match address and id");
 		goto announce_err;
 	}
 
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 	err = 0;
 out:
 	if (err)
-		GENL_SET_ERR_MSG_FMT(info,
-				     "address with id %u not found",
-				     id_val);
+		NL_SET_ERR_MSG_ATTR_FMT(info->extack, id,
+					"address with id %u not found",
+					id_val);
 
 	sock_put(sk);
 	return err;
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 		goto create_err;
 
 	if (entry.flags & MPTCP_PM_ADDR_FLAG_SIGNAL) {
-		GENL_SET_ERR_MSG(info, "invalid addr flags");
+		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "invalid addr flags");
 		err = -EINVAL;
 		goto create_err;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 
 	err = mptcp_userspace_pm_append_new_local_addr(msk, &entry, false);
 	if (err < 0) {
-		GENL_SET_ERR_MSG(info, "did not match address and id");
+		NL_SET_ERR_MSG_ATTR(info->extack, laddr,
+				    "did not match address and id");
 		goto create_err;
 	}
 
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 	}
 
 	if (!addr_l.addr.port) {
-		GENL_SET_ERR_MSG(info, "missing local port");
+		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "missing local port");
 		err = -EINVAL;
 		goto destroy_err;
 	}
 
 	if (!addr_r.port) {
-		GENL_SET_ERR_MSG(info, "missing remote port");
+		NL_SET_ERR_MSG_ATTR(info->extack, raddr, "missing remote port");
 		err = -EINVAL;
 		goto destroy_err;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 		goto set_flags_err;
 
 	if (loc.addr.family == AF_UNSPEC) {
-		GENL_SET_ERR_MSG(info, "invalid local address family");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr,
+				    "invalid local address family");
 		ret = -EINVAL;
 		goto set_flags_err;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 		goto set_flags_err;
 
 	if (rem.addr.family == AF_UNSPEC) {
-		GENL_SET_ERR_MSG(info, "invalid remote address family");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr_rem,
+				    "invalid remote address family");
 		ret = -EINVAL;
 		goto set_flags_err;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_get_addr(struct sk_buff *skb,
 	spin_lock_bh(&msk->pm.lock);
 	entry = mptcp_userspace_pm_lookup_addr_by_id(msk, addr.addr.id);
 	if (!entry) {
-		GENL_SET_ERR_MSG(info, "address not found");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		ret = -EINVAL;
 		goto unlock_fail;
 	}

-- 
2.47.1

From: Geliang Tang <tanggeliang@kylinos.cn>

Three netlink functions:

mptcp_pm_nl_get_addr_doit()
	mptcp_pm_nl_get_addr_dumpit()
	mptcp_pm_nl_set_flags_doit()

are generic, implemented for each PM, in-kernel PM and userspace PM. It's
clearer to move them from pm_netlink.c to pm.c.

And the linked three path manager wrappers

mptcp_pm_get_addr()
	mptcp_pm_dump_addr()
	mptcp_pm_set_flags()

can be changed as static functions, no need to export them in protocol.h.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm.c         | 23 ++++++++++++++++++++---
 net/mptcp/pm_netlink.c | 16 ----------------
 net/mptcp/protocol.h   |  3 ---
 3 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -XXX,XX +XXX,XX @@
 #include "protocol.h"
 
 #include "mib.h"
+#include "mptcp_pm_gen.h"
 
 /* path manager command handlers */
 
@@ -XXX,XX +XXX,XX @@ bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc)
 	return mptcp_pm_nl_is_backup(msk, &skc_local);
 }
 
-int mptcp_pm_get_addr(struct sk_buff *skb, struct genl_info *info)
+static int mptcp_pm_get_addr(struct sk_buff *skb, struct genl_info *info)
 {
 	if (info->attrs[MPTCP_PM_ATTR_TOKEN])
 		return mptcp_userspace_pm_get_addr(skb, info);
 	return mptcp_pm_nl_get_addr(skb, info);
 }
 
-int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb)
+int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	return mptcp_pm_get_addr(skb, info);
+}
+
+static int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb)
 {
 	const struct genl_info *info = genl_info_dump(cb);
 
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb)
 	return mptcp_pm_nl_dump_addr(msg, cb);
 }
 
-int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
+int mptcp_pm_nl_get_addr_dumpit(struct sk_buff *msg,
+				struct netlink_callback *cb)
+{
+	return mptcp_pm_dump_addr(msg, cb);
+}
+
+static int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 {
 	if (info->attrs[MPTCP_PM_ATTR_TOKEN])
 		return mptcp_userspace_pm_set_flags(skb, info);
 	return mptcp_pm_nl_set_flags(skb, info);
 }
 
+int mptcp_pm_nl_set_flags_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	return mptcp_pm_set_flags(skb, info);
+}
+
 void mptcp_pm_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk)
 {
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_get_addr(struct sk_buff *skb, struct genl_info *info)
 	return ret;
 }
 
-int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info)
-{
-	return mptcp_pm_get_addr(skb, info);
-}
-
 int mptcp_pm_nl_dump_addr(struct sk_buff *msg,
 			  struct netlink_callback *cb)
 {
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_dump_addr(struct sk_buff *msg,
 	return msg->len;
 }
 
-int mptcp_pm_nl_get_addr_dumpit(struct sk_buff *msg,
-				struct netlink_callback *cb)
-{
-	return mptcp_pm_dump_addr(msg, cb);
-}
-
 static int parse_limit(struct genl_info *info, int id, unsigned int *limit)
 {
 	struct nlattr *attr = info->attrs[id];
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info)
 	return 0;
 }
 
-int mptcp_pm_nl_set_flags_doit(struct sk_buff *skb, struct genl_info *info)
-{
-	return mptcp_pm_set_flags(skb, info);
-}
-
 static void mptcp_nl_mcast_send(struct net *net, struct sk_buff *nlskb, gfp_t gfp)
 {
 	genlmsg_multicast_netns(&mptcp_genl_family, net,
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -XXX,XX +XXX,XX @@ bool mptcp_lookup_subflow_by_saddr(const struct list_head *list,
 				   const struct mptcp_addr_info *saddr);
 bool mptcp_remove_anno_list_by_saddr(struct mptcp_sock *msk,
 				     const struct mptcp_addr_info *addr);
-int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info);
 int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info);
 int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info);
 int mptcp_pm_announce_addr(struct mptcp_sock *msk,
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_in
 bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc);
 bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
 bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
-int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb);
 int mptcp_pm_nl_dump_addr(struct sk_buff *msg,
 			  struct netlink_callback *cb);
 int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
 				 struct netlink_callback *cb);
-int mptcp_pm_get_addr(struct sk_buff *skb, struct genl_info *info);
 int mptcp_pm_nl_get_addr(struct sk_buff *skb, struct genl_info *info);
 int mptcp_userspace_pm_get_addr(struct sk_buff *skb,
 				struct genl_info *info);

-- 
2.47.1

From: Geliang Tang <tanggeliang@kylinos.cn>

The first parameters 'skb' of get_addr() interfaces are now useless
since mptcp_userspace_pm_get_sock() helper is used. This patch drops
these useless parameters of them.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm.c           | 8 ++++----
 net/mptcp/pm_netlink.c   | 2 +-
 net/mptcp/pm_userspace.c | 3 +--
 net/mptcp/protocol.h     | 5 ++---
 4 files changed, 8 insertions(+), 10 deletions(-)

-- 
2.47.1

From: Geliang Tang <tanggeliang@kylinos.cn>

The address id is parsed both in mptcp_pm_nl_get_addr() and
mptcp_userspace_pm_get_addr(), this makes the code somewhat repetitive.

So this patch adds a new parameter 'id' for all get_addr() interfaces.
The address id is only parsed in mptcp_pm_nl_get_addr_doit(), then pass
it to both mptcp_pm_nl_get_addr() and mptcp_userspace_pm_get_addr().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm.c           | 20 ++++++++++++++++----
 net/mptcp/pm_netlink.c   | 14 +++-----------
 net/mptcp/pm_userspace.c | 14 +++-----------
 net/mptcp/protocol.h     |  4 ++--
 4 files changed, 24 insertions(+), 28 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -XXX,XX +XXX,XX @@ bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc)
 	return mptcp_pm_nl_is_backup(msk, &skc_local);
 }
 
-static int mptcp_pm_get_addr(struct genl_info *info)
+static int mptcp_pm_get_addr(u8 id, struct genl_info *info)
 {
 	if (info->attrs[MPTCP_PM_ATTR_TOKEN])
-		return mptcp_userspace_pm_get_addr(info);
-	return mptcp_pm_nl_get_addr(info);
+		return mptcp_userspace_pm_get_addr(id, info);
+	return mptcp_pm_nl_get_addr(id, info);
 }
 
 int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	return mptcp_pm_get_addr(info);
+	struct mptcp_pm_addr_entry addr;
+	struct nlattr *attr;
+	int ret;
+
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
+		return -EINVAL;
+
+	attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
+	ret = mptcp_pm_parse_entry(attr, info, false, &addr);
+	if (ret < 0)
+		return ret;
+
+	return mptcp_pm_get_addr(addr.addr.id, info);
 }
 
 static int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -XXX,XX +XXX,XX @@ int mptcp_nl_fill_addr(struct sk_buff *skb,
 	return -EMSGSIZE;
 }
 
-int mptcp_pm_nl_get_addr(struct genl_info *info)
+int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info)
 {
 	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
-	struct mptcp_pm_addr_entry addr, *entry;
+	struct mptcp_pm_addr_entry *entry;
 	struct sk_buff *msg;
 	struct nlattr *attr;
 	void *reply;
 	int ret;
 
-	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
-		return -EINVAL;
-
-	attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
-	ret = mptcp_pm_parse_entry(attr, info, false, &addr);
-	if (ret < 0)
-		return ret;
-
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
 		return -ENOMEM;
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_get_addr(struct genl_info *info)
 	}
 
 	rcu_read_lock();
-	entry = __lookup_addr_by_id(pernet, addr.addr.id);
+	entry = __lookup_addr_by_id(pernet, id);
 	if (!entry) {
 		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		ret = -EINVAL;
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
 	return ret;
 }
 
-int mptcp_userspace_pm_get_addr(struct genl_info *info)
+int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info)
 {
-	struct mptcp_pm_addr_entry addr, *entry;
+	struct mptcp_pm_addr_entry *entry;
 	struct mptcp_sock *msk;
 	struct sk_buff *msg;
 	struct nlattr *attr;
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_get_addr(struct genl_info *info)
 	struct sock *sk;
 	void *reply;
 
-	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
-		return ret;
-
 	msk = mptcp_userspace_pm_get_sock(info);
 	if (!msk)
 		return ret;
 
 	sk = (struct sock *)msk;
 
-	attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
-	ret = mptcp_pm_parse_entry(attr, info, false, &addr);
-	if (ret < 0)
-		goto out;
-
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg) {
 		ret = -ENOMEM;
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_get_addr(struct genl_info *info)
 
 	lock_sock(sk);
 	spin_lock_bh(&msk->pm.lock);
-	entry = mptcp_userspace_pm_lookup_addr_by_id(msk, addr.addr.id);
+	entry = mptcp_userspace_pm_lookup_addr_by_id(msk, id);
 	if (!entry) {
 		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		ret = -EINVAL;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_dump_addr(struct sk_buff *msg,
 			  struct netlink_callback *cb);
 int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
 				 struct netlink_callback *cb);
-int mptcp_pm_nl_get_addr(struct genl_info *info);
-int mptcp_userspace_pm_get_addr(struct genl_info *info);
+int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info);
+int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info);
 
 static inline u8 subflow_get_local_id(const struct mptcp_subflow_context *subflow)
 {

-- 
2.47.1

From: Geliang Tang <tanggeliang@kylinos.cn>

The netlink messages are sent both in mptcp_pm_nl_get_addr() and
mptcp_userspace_pm_get_addr(), this makes the code somewhat repetitive.
This is because the netlink PM and userspace PM use different locks to
protect the address entry that needs to be sent via the netlink message.
The former uses rcu read lock, and the latter uses msk->pm.lock.

The current get_addr() flow looks like this:

lock();
	entry = get_entry();
	send_nlmsg(entry);
	unlock();

After holding the lock, get the entry from the list, send the entry, and
finally release the lock.

This patch changes the process by getting the entry while holding the lock,
then making a copy of the entry so that the lock can be released. Finally,
the copy of the entry is sent without locking:

lock();
	entry = get_entry();
	*copy = *entry;
	unlock();

send_nlmsg(copy);

This way we can reuse the send_nlmsg() code in get_addr() interfaces
between the netlink PM and userspace PM. They only need to implement their
own get_addr() interfaces to hold the different locks, get the entry from
the different lists, then release the locks.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm.c           | 39 +++++++++++++++++++++++++++++++++++----
 net/mptcp/pm_netlink.c   | 40 ++++++----------------------------------
 net/mptcp/pm_userspace.c | 42 +++++-------------------------------------
 net/mptcp/protocol.h     |  6 ++++--
 4 files changed, 50 insertions(+), 77 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -XXX,XX +XXX,XX @@ bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc)
 	return mptcp_pm_nl_is_backup(msk, &skc_local);
 }
 
-static int mptcp_pm_get_addr(u8 id, struct genl_info *info)
+static int mptcp_pm_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
+			     struct genl_info *info)
 {
 	if (info->attrs[MPTCP_PM_ATTR_TOKEN])
-		return mptcp_userspace_pm_get_addr(id, info);
-	return mptcp_pm_nl_get_addr(id, info);
+		return mptcp_userspace_pm_get_addr(id, addr, info);
+	return mptcp_pm_nl_get_addr(id, addr, info);
 }
 
 int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info)
 {
 	struct mptcp_pm_addr_entry addr;
 	struct nlattr *attr;
+	struct sk_buff *msg;
+	void *reply;
 	int ret;
 
 	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info)
 	if (ret < 0)
 		return ret;
 
-	return mptcp_pm_get_addr(addr.addr.id, info);
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	reply = genlmsg_put_reply(msg, info, &mptcp_genl_family, 0,
+				  info->genlhdr->cmd);
+	if (!reply) {
+		GENL_SET_ERR_MSG(info, "not enough space in Netlink message");
+		ret = -EMSGSIZE;
+		goto fail;
+	}
+
+	ret = mptcp_pm_get_addr(addr.addr.id, &addr, info);
+	if (ret) {
+		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
+		goto fail;
+	}
+
+	ret = mptcp_nl_fill_addr(msg, &addr);
+	if (ret)
+		goto fail;
+
+	genlmsg_end(msg, reply);
+	ret = genlmsg_reply(msg, info);
+	return ret;
+
+fail:
+	nlmsg_free(msg);
+	return ret;
 }
 
 static int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -XXX,XX +XXX,XX @@ int mptcp_nl_fill_addr(struct sk_buff *skb,
 	return -EMSGSIZE;
 }
 
-int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info)
+int mptcp_pm_nl_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
+			 struct genl_info *info)
 {
 	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
 	struct mptcp_pm_addr_entry *entry;
-	struct sk_buff *msg;
-	struct nlattr *attr;
-	void *reply;
-	int ret;
-
-	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
-	if (!msg)
-		return -ENOMEM;
-
-	reply = genlmsg_put_reply(msg, info, &mptcp_genl_family, 0,
-				  info->genlhdr->cmd);
-	if (!reply) {
-		GENL_SET_ERR_MSG(info, "not enough space in Netlink message");
-		ret = -EMSGSIZE;
-		goto fail;
-	}
+	int ret = -EINVAL;
 
 	rcu_read_lock();
 	entry = __lookup_addr_by_id(pernet, id);
-	if (!entry) {
-		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
-		ret = -EINVAL;
-		goto unlock_fail;
+	if (entry) {
+		*addr = *entry;
+		ret = 0;
 	}
-
-	ret = mptcp_nl_fill_addr(msg, entry);
-	if (ret)
-		goto unlock_fail;
-
-	genlmsg_end(msg, reply);
-	ret = genlmsg_reply(msg, info);
-	rcu_read_unlock();
-	return ret;
-
-unlock_fail:
 	rcu_read_unlock();
 
-fail:
-	nlmsg_free(msg);
 	return ret;
 }
 
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
 	return ret;
 }
 
-int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info)
+int mptcp_userspace_pm_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
+				struct genl_info *info)
 {
 	struct mptcp_pm_addr_entry *entry;
 	struct mptcp_sock *msk;
-	struct sk_buff *msg;
-	struct nlattr *attr;
 	int ret = -EINVAL;
 	struct sock *sk;
-	void *reply;
 
 	msk = mptcp_userspace_pm_get_sock(info);
 	if (!msk)
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info)
 
 	sk = (struct sock *)msk;
 
-	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
-	if (!msg) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	reply = genlmsg_put_reply(msg, info, &mptcp_genl_family, 0,
-				  info->genlhdr->cmd);
-	if (!reply) {
-		GENL_SET_ERR_MSG(info, "not enough space in Netlink message");
-		ret = -EMSGSIZE;
-		goto fail;
-	}
-
 	lock_sock(sk);
 	spin_lock_bh(&msk->pm.lock);
 	entry = mptcp_userspace_pm_lookup_addr_by_id(msk, id);
-	if (!entry) {
-		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
-		ret = -EINVAL;
-		goto unlock_fail;
+	if (entry) {
+		*addr = *entry;
+		ret = 0;
 	}
-
-	ret = mptcp_nl_fill_addr(msg, entry);
-	if (ret)
-		goto unlock_fail;
-
-	genlmsg_end(msg, reply);
-	ret = genlmsg_reply(msg, info);
 	spin_unlock_bh(&msk->pm.lock);
 	release_sock(sk);
-	sock_put(sk);
-	return ret;
 
-unlock_fail:
-	spin_unlock_bh(&msk->pm.lock);
-	release_sock(sk);
-fail:
-	nlmsg_free(msg);
-out:
 	sock_put(sk);
 	return ret;
 }
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_dump_addr(struct sk_buff *msg,
 			  struct netlink_callback *cb);
 int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
 				 struct netlink_callback *cb);
-int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info);
-int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info);
+int mptcp_pm_nl_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
+			 struct genl_info *info);
+int mptcp_userspace_pm_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
+				struct genl_info *info);
 
 static inline u8 subflow_get_local_id(const struct mptcp_subflow_context *subflow)
 {

-- 
2.47.1

From: Geliang Tang <tanggeliang@kylinos.cn>

The first parameter 'skb' in mptcp_pm_nl_set_flags() is only used to
obtained the network namespace, which can also be obtained through the
second parameters 'info' by using genl_info_net() helper.

This patch drops these useless parameters 'skb' in all three set_flags()
interfaces.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm.c           | 8 ++++----
 net/mptcp/pm_netlink.c   | 4 ++--
 net/mptcp/pm_userspace.c | 2 +-
 net/mptcp/protocol.h     | 4 ++--
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_get_addr_dumpit(struct sk_buff *msg,
 	return mptcp_pm_dump_addr(msg, cb);
 }
 
-static int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
+static int mptcp_pm_set_flags(struct genl_info *info)
 {
 	if (info->attrs[MPTCP_PM_ATTR_TOKEN])
-		return mptcp_userspace_pm_set_flags(skb, info);
-	return mptcp_pm_nl_set_flags(skb, info);
+		return mptcp_userspace_pm_set_flags(info);
+	return mptcp_pm_nl_set_flags(info);
 }
 
 int mptcp_pm_nl_set_flags_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	return mptcp_pm_set_flags(skb, info);
+	return mptcp_pm_set_flags(info);
 }
 
 void mptcp_pm_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -XXX,XX +XXX,XX @@ static int mptcp_nl_set_flags(struct net *net,
 	return ret;
 }
 
-int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info)
+int mptcp_pm_nl_set_flags(struct genl_info *info)
 {
 	struct mptcp_pm_addr_entry addr = { .addr = { .family = AF_UNSPEC }, };
 	u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP |
 			   MPTCP_PM_ADDR_FLAG_FULLMESH;
-	struct net *net = sock_net(skb->sk);
+	struct net *net = genl_info_net(info);
 	struct mptcp_pm_addr_entry *entry;
 	struct pm_nl_pernet *pernet;
 	struct nlattr *attr;
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 	return err;
 }
 
-int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
+int mptcp_userspace_pm_set_flags(struct genl_info *info)
 {
 	struct mptcp_pm_addr_entry loc = { .addr = { .family = AF_UNSPEC }, };
 	struct mptcp_pm_addr_entry rem = { .addr = { .family = AF_UNSPEC }, };
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -XXX,XX +XXX,XX @@ bool mptcp_lookup_subflow_by_saddr(const struct list_head *list,
 				   const struct mptcp_addr_info *saddr);
 bool mptcp_remove_anno_list_by_saddr(struct mptcp_sock *msk,
 				     const struct mptcp_addr_info *addr);
-int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info);
-int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info);
+int mptcp_pm_nl_set_flags(struct genl_info *info);
+int mptcp_userspace_pm_set_flags(struct genl_info *info);
 int mptcp_pm_announce_addr(struct mptcp_sock *msk,
 			   const struct mptcp_addr_info *addr,
 			   bool echo);

-- 
2.47.1

From: Geliang Tang <tanggeliang@kylinos.cn>

Generally, in the path manager interfaces, the local address is defined
as an mptcp_pm_addr_entry type address, while the remote address is
defined as an mptcp_addr_info type one:

(struct mptcp_pm_addr_entry *local, struct mptcp_addr_info *remote)

But the set_flags() interface uses two mptcp_pm_addr_entry type
parameters.

This patch changes the second one to mptcp_addr_info type and use helper
mptcp_pm_parse_addr() to parse it instead of using mptcp_pm_parse_entry().

-- 
2.47.1

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch updates the interfaces set_flags to reduce repetitive
code, adds a new parameter 'local' for them.

The local address is parsed in public helper mptcp_pm_nl_set_flags_doit(),
then pass it to mptcp_pm_nl_set_flags() and mptcp_userspace_pm_set_flags().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm.c           | 16 ++++++++++++++--
 net/mptcp/pm_netlink.c   | 35 +++++++++++++----------------------
 net/mptcp/pm_userspace.c | 19 +++++++------------
 net/mptcp/protocol.h     |  6 ++++--
 4 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_get_addr_dumpit(struct sk_buff *msg,
 
 static int mptcp_pm_set_flags(struct genl_info *info)
 {
+	struct mptcp_pm_addr_entry loc = { .addr = { .family = AF_UNSPEC }, };
+	struct nlattr *attr_loc;
+	int ret = -EINVAL;
+
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR))
+		return ret;
+
+	attr_loc = info->attrs[MPTCP_PM_ATTR_ADDR];
+	ret = mptcp_pm_parse_entry(attr_loc, info, false, &loc);
+	if (ret < 0)
+		return ret;
+
 	if (info->attrs[MPTCP_PM_ATTR_TOKEN])
-		return mptcp_userspace_pm_set_flags(info);
-	return mptcp_pm_nl_set_flags(info);
+		return mptcp_userspace_pm_set_flags(&loc, info);
+	return mptcp_pm_nl_set_flags(&loc, info);
 }
 
 int mptcp_pm_nl_set_flags_doit(struct sk_buff *skb, struct genl_info *info)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -XXX,XX +XXX,XX @@ static int mptcp_nl_set_flags(struct net *net,
 	return ret;
 }
 
-int mptcp_pm_nl_set_flags(struct genl_info *info)
+int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
+			  struct genl_info *info)
 {
-	struct mptcp_pm_addr_entry addr = { .addr = { .family = AF_UNSPEC }, };
+	struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP |
 			   MPTCP_PM_ADDR_FLAG_FULLMESH;
 	struct net *net = genl_info_net(info);
 	struct mptcp_pm_addr_entry *entry;
 	struct pm_nl_pernet *pernet;
-	struct nlattr *attr;
 	u8 lookup_by_id = 0;
 	u8 bkup = 0;
-	int ret;
-
-	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR))
-		return -EINVAL;
 
 	pernet = pm_nl_get_pernet(net);
 
-	attr = info->attrs[MPTCP_PM_ATTR_ADDR];
-	ret = mptcp_pm_parse_entry(attr, info, false, &addr);
-	if (ret < 0)
-		return ret;
-
-	if (addr.addr.family == AF_UNSPEC) {
+	if (local->addr.family == AF_UNSPEC) {
 		lookup_by_id = 1;
-		if (!addr.addr.id) {
+		if (!local->addr.id) {
 			NL_SET_ERR_MSG_ATTR(info->extack, attr,
 					    "missing address ID");
 			return -EOPNOTSUPP;
 		}
 	}
 
-	if (addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
+	if (local->flags & MPTCP_PM_ADDR_FLAG_BACKUP)
 		bkup = 1;
 
 	spin_lock_bh(&pernet->lock);
-	entry = lookup_by_id ? __lookup_addr_by_id(pernet, addr.addr.id) :
-			       __lookup_addr(pernet, &addr.addr);
+	entry = lookup_by_id ? __lookup_addr_by_id(pernet, local->addr.id) :
+			       __lookup_addr(pernet, &local->addr);
 	if (!entry) {
 		spin_unlock_bh(&pernet->lock);
 		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		return -EINVAL;
 	}
-	if ((addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH) &&
+	if ((local->flags & MPTCP_PM_ADDR_FLAG_FULLMESH) &&
 	    (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
 		spin_unlock_bh(&pernet->lock);
 		NL_SET_ERR_MSG_ATTR(info->extack, attr, "invalid addr flags");
 		return -EINVAL;
 	}
 
-	changed = (addr.flags ^ entry->flags) & mask;
-	entry->flags = (entry->flags & ~mask) | (addr.flags & mask);
-	addr = *entry;
+	changed = (local->flags ^ entry->flags) & mask;
+	entry->flags = (entry->flags & ~mask) | (local->flags & mask);
+	*local = *entry;
 	spin_unlock_bh(&pernet->lock);
 
-	mptcp_nl_set_flags(net, &addr.addr, bkup, changed);
+	mptcp_nl_set_flags(net, &local->addr, bkup, changed);
 	return 0;
 }
 
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 	return err;
 }
 
-int mptcp_userspace_pm_set_flags(struct genl_info *info)
+int mptcp_userspace_pm_set_flags(struct mptcp_pm_addr_entry *local,
+				 struct genl_info *info)
 {
-	struct mptcp_pm_addr_entry loc = { .addr = { .family = AF_UNSPEC }, };
 	struct mptcp_addr_info rem = { .family = AF_UNSPEC, };
 	struct mptcp_pm_addr_entry *entry;
 	struct nlattr *attr, *attr_rem;
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct genl_info *info)
 	struct sock *sk;
 	u8 bkup = 0;
 
-	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR) ||
-	    GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE))
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE))
 		return ret;
 
 	msk = mptcp_userspace_pm_get_sock(info);
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct genl_info *info)
 	sk = (struct sock *)msk;
 
 	attr = info->attrs[MPTCP_PM_ATTR_ADDR];
-	ret = mptcp_pm_parse_entry(attr, info, false, &loc);
-	if (ret < 0)
-		goto set_flags_err;
-
-	if (loc.addr.family == AF_UNSPEC) {
+	if (local->addr.family == AF_UNSPEC) {
 		NL_SET_ERR_MSG_ATTR(info->extack, attr,
 				    "invalid local address family");
 		ret = -EINVAL;
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct genl_info *info)
 		goto set_flags_err;
 	}
 
-	if (loc.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
+	if (local->flags & MPTCP_PM_ADDR_FLAG_BACKUP)
 		bkup = 1;
 
 	spin_lock_bh(&msk->pm.lock);
-	entry = mptcp_userspace_pm_lookup_addr(msk, &loc.addr);
+	entry = mptcp_userspace_pm_lookup_addr(msk, &local->addr);
 	if (entry) {
 		if (bkup)
 			entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP;
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct genl_info *info)
 	spin_unlock_bh(&msk->pm.lock);
 
 	lock_sock(sk);
-	ret = mptcp_pm_nl_mp_prio_send_ack(msk, &loc.addr, &rem, bkup);
+	ret = mptcp_pm_nl_mp_prio_send_ack(msk, &local->addr, &rem, bkup);
 	release_sock(sk);
 
 	/* mptcp_pm_nl_mp_prio_send_ack() only fails in one case */
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -XXX,XX +XXX,XX @@ bool mptcp_lookup_subflow_by_saddr(const struct list_head *list,
 				   const struct mptcp_addr_info *saddr);
 bool mptcp_remove_anno_list_by_saddr(struct mptcp_sock *msk,
 				     const struct mptcp_addr_info *addr);
-int mptcp_pm_nl_set_flags(struct genl_info *info);
-int mptcp_userspace_pm_set_flags(struct genl_info *info);
+int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
+			  struct genl_info *info);
+int mptcp_userspace_pm_set_flags(struct mptcp_pm_addr_entry *local,
+				 struct genl_info *info);
 int mptcp_pm_announce_addr(struct mptcp_sock *msk,
 			   const struct mptcp_addr_info *addr,
 			   bool echo);

-- 
2.47.1

- Patch 1: drop unneeded parameter in a function helper.

- Patch 2: clearer NL error message when an NL attribute is missing.

- Patch 3: more precise NL messages by avoiding 'this or that is NOK'.

- Patch 4: improve too vague or missing NL err messages.

- Patch 5: use GENL_REQ_ATTR_CHECK to look for mandatory NL attributes.

- Patch 6: avoid overriding the error message.

- Patch 7: check all mandatory NL attributes with GENL_REQ_ATTR_CHECK.

- Patch 8: use NL_SET_ERR_MSG_ATTR instead of GENL_SET_ERR_MSG

- Patch 9: move doit callbacks used for both PM to pm.c.

- Patch 10: drop another unneeded parameter in a function helper.

- Patch 11: share the ID parsing code for the 'get_addr' callback.

- Patch 12: share sending NL code for the 'get_addr' callback.

- Patch 13: drop yet another unneeded parameter in a function helper.

- Patch 14: pick the usual structure type for the remote address.

- Patch 15: share the local addr parsing code for the 'set_flags' cb.

The behaviour when there are no errors should then not be modified.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Changes in v3:
- Patch 11: a variable was no longer assigned in pm_userspace.c, but
  still used in this patch (and no longer in the next one). (Geliang)
- Rebased on top of the latest net-next.
- Link to v2: https://lore.kernel.org/r/20250117-net-next-mptcp-pm-misc-cleanup-2-v2-0-61d4fe0586e8@kernel.org

Changes in v2:
- Patch 11: a variable was no longer assigned in pm_netlink.c, but still
  used in this patch (and no longer in the next one). (Simon)
- Link to v1: https://lore.kernel.org/r/20250116-net-next-mptcp-pm-misc-cleanup-2-v1-0-c0b43f18fe06@kernel.org

---
Geliang Tang (9):
      mptcp: pm: drop info of userspace_pm_remove_id_zero_address
      mptcp: pm: userspace: use GENL_REQ_ATTR_CHECK
      mptcp: pm: make three pm wrappers static
      mptcp: pm: drop skb parameter of get_addr
      mptcp: pm: add id parameter for get_addr
      mptcp: pm: reuse sending nlmsg code in get_addr
      mptcp: pm: drop skb parameter of set_flags
      mptcp: pm: change rem type of set_flags
      mptcp: pm: add local parameter for set_flags

net/mptcp/pm.c           |  86 +++++++++++++++++--
 net/mptcp/pm_netlink.c   | 129 ++++++++++-------------------
 net/mptcp/pm_userspace.c | 209 +++++++++++++++++++++--------------------------
 net/mptcp/protocol.h     |  14 ++--
 4 files changed, 225 insertions(+), 213 deletions(-)
---
base-commit: 26db4dbb747813b5946aff31485873f071a10332
change-id: 20250116-net-next-mptcp-pm-misc-cleanup-2-b0f50eff8084

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>

From: Geliang Tang <tanggeliang@kylinos.cn>

The only use of 'info' parameter of userspace_pm_remove_id_zero_address()
is to set an error message into it.

Plus, this helper will only fail when it cannot find any subflows with a
local address ID 0.

This patch drops this parameter and sets the error message where this
function is called in mptcp_pm_nl_remove_doit().

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 	return err;
 }
 
-static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
-						     struct genl_info *info)
+static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk)
 {
 	struct mptcp_rm_list list = { .nr = 0 };
 	struct mptcp_subflow_context *subflow;
@@ -XXX,XX +XXX,XX @@ static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
 			break;
 		}
 	}
-	if (!has_id_0) {
-		GENL_SET_ERR_MSG(info, "address with id 0 not found");
+	if (!has_id_0)
 		goto remove_err;
-	}
 
 	list.ids[list.nr++] = 0;
 
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 	sk = (struct sock *)msk;
 
 	if (id_val == 0) {
-		err = mptcp_userspace_pm_remove_id_zero_address(msk, info);
+		err = mptcp_userspace_pm_remove_id_zero_address(msk);
 		goto out;
 	}
 
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 	spin_lock_bh(&msk->pm.lock);
 	match = mptcp_userspace_pm_lookup_addr_by_id(msk, id_val);
 	if (!match) {
-		GENL_SET_ERR_MSG(info, "address with specified id not found");
 		spin_unlock_bh(&msk->pm.lock);
 		release_sock(sk);
 		goto out;
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 
 	err = 0;
 out:
+	if (err)
+		GENL_SET_ERR_MSG_FMT(info,
+				     "address with id %u not found",
+				     id_val);
+
 	sock_put(sk);
 	return err;
 }

-- 
2.47.1

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.

Reviewed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_userspace.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ 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;
-	}
+	ret = mptcp_pm_parse_entry(attr_rem, info, false, &rem);
+	if (ret < 0)
+		goto set_flags_err;
 
 	if (loc.addr.family == AF_UNSPEC ||
 	    rem.addr.family == AF_UNSPEC) {

-- 
2.47.1

Some errors reported by the userspace PM were vague: "this or that is
invalid".

It is easier for the userspace to know which part is wrong, instead of
having to guess that.

While at it, in mptcp_userspace_pm_set_flags() move the parsing after
the check linked to the local attribute.

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 		goto announce_err;
 	}
 
-	if (addr_val.addr.id == 0 || !(addr_val.flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
-		GENL_SET_ERR_MSG(info, "invalid addr id or flags");
+	if (addr_val.addr.id == 0) {
+		GENL_SET_ERR_MSG(info, "invalid addr id");
+		err = -EINVAL;
+		goto announce_err;
+	}
+
+	if (!(addr_val.flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
+		GENL_SET_ERR_MSG(info, "invalid addr flags");
 		err = -EINVAL;
 		goto announce_err;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 		goto destroy_err;
 	}
 
-	if (!addr_l.addr.port || !addr_r.port) {
-		GENL_SET_ERR_MSG(info, "missing local or remote port");
+	if (!addr_l.addr.port) {
+		GENL_SET_ERR_MSG(info, "missing local port");
+		err = -EINVAL;
+		goto destroy_err;
+	}
+
+	if (!addr_r.port) {
+		GENL_SET_ERR_MSG(info, "missing remote port");
 		err = -EINVAL;
 		goto destroy_err;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 	if (ret < 0)
 		goto set_flags_err;
 
+	if (loc.addr.family == AF_UNSPEC) {
+		GENL_SET_ERR_MSG(info, "invalid local address family");
+		ret = -EINVAL;
+		goto set_flags_err;
+	}
+
 	ret = mptcp_pm_parse_entry(attr_rem, info, false, &rem);
 	if (ret < 0)
 		goto set_flags_err;
 
-	if (loc.addr.family == AF_UNSPEC ||
-	    rem.addr.family == AF_UNSPEC) {
-		GENL_SET_ERR_MSG(info, "invalid address families");
+	if (rem.addr.family == AF_UNSPEC) {
+		GENL_SET_ERR_MSG(info, "invalid remote address family");
 		ret = -EINVAL;
 		goto set_flags_err;
 	}

-- 
2.47.1

Some error messages were:

- too generic: "missing input", "invalid request"

- not precise enough: "limit greater than maximum" but what's the max?

- missing: subflow not found, or connect error.

This can be easily improved by being more precise, or adding new error
messages.

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -XXX,XX +XXX,XX @@ static int parse_limit(struct genl_info *info, int id, unsigned int *limit)
 
 	*limit = nla_get_u32(attr);
 	if (*limit > MPTCP_PM_ADDR_MAX) {
-		GENL_SET_ERR_MSG(info, "limit greater than maximum");
+		NL_SET_ERR_MSG_ATTR_FMT(info->extack, attr,
+					"limit greater than maximum (%u)",
+					MPTCP_PM_ADDR_MAX);
 		return -EINVAL;
 	}
 	return 0;
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info)
 	if (addr.addr.family == AF_UNSPEC) {
 		lookup_by_id = 1;
 		if (!addr.addr.id) {
-			GENL_SET_ERR_MSG(info, "missing required inputs");
+			GENL_SET_ERR_MSG(info, "missing address ID");
 			return -EOPNOTSUPP;
 		}
 	}
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *in
 	}
 
 	if (!mptcp_pm_is_userspace(msk)) {
-		GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected");
+		GENL_SET_ERR_MSG(info, "userspace PM not selected");
 		sock_put((struct sock *)msk);
 		return NULL;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 	err = __mptcp_subflow_connect(sk, &local, &addr_r);
 	release_sock(sk);
 
+	if (err)
+		GENL_SET_ERR_MSG_FMT(info, "connect error: %d", err);
+
 	spin_lock_bh(&msk->pm.lock);
 	if (err)
 		mptcp_userspace_pm_delete_local_addr(msk, &entry);
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 	lock_sock(sk);
 	ssk = mptcp_nl_find_ssk(msk, &addr_l.addr, &addr_r);
 	if (!ssk) {
+		GENL_SET_ERR_MSG(info, "subflow not found");
 		err = -ESRCH;
 		goto release_sock;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 	ret = mptcp_pm_nl_mp_prio_send_ack(msk, &loc.addr, &rem.addr, bkup);
 	release_sock(sk);
 
+	/* mptcp_pm_nl_mp_prio_send_ack() only fails in one case */
+	if (ret < 0)
+		GENL_SET_ERR_MSG(info, "subflow not found");
+
 set_flags_err:
 	sock_put(sk);
 	return ret;

-- 
2.47.1

From: Geliang Tang <tanggeliang@kylinos.cn>

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk,
 
 static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *info)
 {
-	struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
 	struct mptcp_sock *msk;
+	struct nlattr *token;
 
-	if (!token) {
-		GENL_SET_ERR_MSG(info, "missing required token");
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_TOKEN))
 		return NULL;
-	}
 
+	token = info->attrs[MPTCP_PM_ATTR_TOKEN];
 	msk = mptcp_token_get_sock(genl_info_net(info), nla_get_u32(token));
 	if (!msk) {
 		NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token");
@@ -XXX,XX +XXX,XX @@ static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *in
 
 int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	struct nlattr *addr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	struct mptcp_pm_addr_entry addr_val;
 	struct mptcp_sock *msk;
+	struct nlattr *addr;
 	int err = -EINVAL;
 	struct sock *sk;
 
-	if (!addr) {
-		GENL_SET_ERR_MSG(info, "missing required address");
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR))
 		return err;
-	}
 
 	msk = mptcp_userspace_pm_get_sock(info);
 	if (!msk)
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 
 	sk = (struct sock *)msk;
 
+	addr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	err = mptcp_pm_parse_entry(addr, info, true, &addr_val);
 	if (err < 0) {
 		GENL_SET_ERR_MSG(info, "error parsing local address");
@@ -XXX,XX +XXX,XX @@ void mptcp_pm_remove_addr_entry(struct mptcp_sock *msk,
 
 int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	struct nlattr *id = info->attrs[MPTCP_PM_ATTR_LOC_ID];
 	struct mptcp_pm_addr_entry *match;
 	struct mptcp_sock *msk;
+	struct nlattr *id;
 	int err = -EINVAL;
 	struct sock *sk;
 	u8 id_val;
 
-	if (!id) {
-		GENL_SET_ERR_MSG(info, "missing required ID");
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_LOC_ID))
 		return err;
-	}
 
+	id = info->attrs[MPTCP_PM_ATTR_LOC_ID];
 	id_val = nla_get_u8(id);
 
 	msk = mptcp_userspace_pm_get_sock(info);
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 
 int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	struct nlattr *raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
-	struct nlattr *laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	struct mptcp_pm_addr_entry entry = { 0 };
 	struct mptcp_addr_info addr_r;
+	struct nlattr *raddr, *laddr;
 	struct mptcp_pm_local local;
 	struct mptcp_sock *msk;
 	int err = -EINVAL;
 	struct sock *sk;
 
-	if (!laddr || !raddr) {
-		GENL_SET_ERR_MSG(info, "missing required address(es)");
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR) ||
+	    GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE))
 		return err;
-	}
 
 	msk = mptcp_userspace_pm_get_sock(info);
 	if (!msk)
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 
 	sk = (struct sock *)msk;
 
+	laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	err = mptcp_pm_parse_entry(laddr, info, true, &entry);
 	if (err < 0) {
 		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr");
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 	}
 	entry.flags |= MPTCP_PM_ADDR_FLAG_SUBFLOW;
 
+	raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
 	err = mptcp_pm_parse_addr(raddr, info, &addr_r);
 	if (err < 0) {
 		NL_SET_ERR_MSG_ATTR(info->extack, raddr, "error parsing remote addr");
@@ -XXX,XX +XXX,XX @@ static struct sock *mptcp_nl_find_ssk(struct mptcp_sock *msk,
 
 int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	struct nlattr *raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
-	struct nlattr *laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	struct mptcp_pm_addr_entry addr_l;
 	struct mptcp_addr_info addr_r;
+	struct nlattr *raddr, *laddr;
 	struct mptcp_sock *msk;
 	struct sock *sk, *ssk;
 	int err = -EINVAL;
 
-	if (!laddr || !raddr) {
-		GENL_SET_ERR_MSG(info, "missing required address(es)");
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR) ||
+	    GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE))
 		return err;
-	}
 
 	msk = mptcp_userspace_pm_get_sock(info);
 	if (!msk)
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 
 	sk = (struct sock *)msk;
 
+	laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	err = mptcp_pm_parse_entry(laddr, info, true, &addr_l);
 	if (err < 0) {
 		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr");
 		goto destroy_err;
 	}
 
+	raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
 	err = mptcp_pm_parse_addr(raddr, info, &addr_r);
 	if (err < 0) {
 		NL_SET_ERR_MSG_ATTR(info->extack, raddr, "error parsing remote addr");

-- 
2.47.1

mptcp_pm_parse_entry() and mptcp_pm_parse_addr() will already set a
error message in case of parsing issue.

Then, no need to override this error message with another less precise
one: "error parsing address".

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 
 	addr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	err = mptcp_pm_parse_entry(addr, info, true, &addr_val);
-	if (err < 0) {
-		GENL_SET_ERR_MSG(info, "error parsing local address");
+	if (err < 0)
 		goto announce_err;
-	}
 
 	if (addr_val.addr.id == 0) {
 		GENL_SET_ERR_MSG(info, "invalid addr id");
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 
 	laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	err = mptcp_pm_parse_entry(laddr, info, true, &entry);
-	if (err < 0) {
-		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr");
+	if (err < 0)
 		goto create_err;
-	}
 
 	if (entry.flags & MPTCP_PM_ADDR_FLAG_SIGNAL) {
 		GENL_SET_ERR_MSG(info, "invalid addr flags");
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 
 	raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
 	err = mptcp_pm_parse_addr(raddr, info, &addr_r);
-	if (err < 0) {
-		NL_SET_ERR_MSG_ATTR(info->extack, raddr, "error parsing remote addr");
+	if (err < 0)
 		goto create_err;
-	}
 
 	if (!mptcp_pm_addr_families_match(sk, &entry.addr, &addr_r)) {
 		GENL_SET_ERR_MSG(info, "families mismatch");
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 
 	laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	err = mptcp_pm_parse_entry(laddr, info, true, &addr_l);
-	if (err < 0) {
-		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr");
+	if (err < 0)
 		goto destroy_err;
-	}
 
 	raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
 	err = mptcp_pm_parse_addr(raddr, info, &addr_r);
-	if (err < 0) {
-		NL_SET_ERR_MSG_ATTR(info->extack, raddr, "error parsing remote addr");
+	if (err < 0)
 		goto destroy_err;
-	}
 
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
 	if (addr_l.addr.family == AF_INET && ipv6_addr_v4mapped(&addr_r.addr6)) {

-- 
2.47.1

mptcp_pm_parse_entry() will check if the given attribute is defined. If
not, it will return a generic error: "missing address info".

It might then not be clear for the userspace developer which attribute
is missing, especially when the command takes multiple addresses.

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -XXX,XX +XXX,XX @@ static bool mptcp_pm_has_addr_attr_id(const struct nlattr *attr,
 
 int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
 	struct mptcp_pm_addr_entry addr, *entry;
+	struct nlattr *attr;
 	int ret;
 
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
+		return -EINVAL;
+
+	attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	ret = mptcp_pm_parse_entry(attr, info, true, &addr);
 	if (ret < 0)
 		return ret;
@@ -XXX,XX +XXX,XX @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
 
 int mptcp_pm_nl_del_addr_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
 	struct mptcp_pm_addr_entry addr, *entry;
 	unsigned int addr_max;
+	struct nlattr *attr;
 	int ret;
 
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
+		return -EINVAL;
+
+	attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	ret = mptcp_pm_parse_entry(attr, info, false, &addr);
 	if (ret < 0)
 		return ret;
@@ -XXX,XX +XXX,XX @@ int mptcp_nl_fill_addr(struct sk_buff *skb,
 
 int mptcp_pm_nl_get_addr(struct sk_buff *skb, struct genl_info *info)
 {
-	struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
 	struct mptcp_pm_addr_entry addr, *entry;
 	struct sk_buff *msg;
+	struct nlattr *attr;
 	void *reply;
 	int ret;
 
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
+		return -EINVAL;
+
+	attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	ret = mptcp_pm_parse_entry(attr, info, false, &addr);
 	if (ret < 0)
 		return ret;
@@ -XXX,XX +XXX,XX @@ static int mptcp_nl_set_flags(struct net *net,
 int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info)
 {
 	struct mptcp_pm_addr_entry addr = { .addr = { .family = AF_UNSPEC }, };
-	struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP |
 			   MPTCP_PM_ADDR_FLAG_FULLMESH;
 	struct net *net = sock_net(skb->sk);
 	struct mptcp_pm_addr_entry *entry;
 	struct pm_nl_pernet *pernet;
+	struct nlattr *attr;
 	u8 lookup_by_id = 0;
 	u8 bkup = 0;
 	int ret;
 
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR))
+		return -EINVAL;
+
 	pernet = pm_nl_get_pernet(net);
 
+	attr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	ret = mptcp_pm_parse_entry(attr, info, false, &addr);
 	if (ret < 0)
 		return ret;
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 {
 	struct mptcp_pm_addr_entry loc = { .addr = { .family = AF_UNSPEC }, };
 	struct mptcp_pm_addr_entry rem = { .addr = { .family = AF_UNSPEC }, };
-	struct nlattr *attr_rem = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
-	struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	struct mptcp_pm_addr_entry *entry;
+	struct nlattr *attr, *attr_rem;
 	struct mptcp_sock *msk;
 	int ret = -EINVAL;
 	struct sock *sk;
 	u8 bkup = 0;
 
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR) ||
+	    GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE))
+		return ret;
+
 	msk = mptcp_userspace_pm_get_sock(info);
 	if (!msk)
 		return ret;
 
 	sk = (struct sock *)msk;
 
+	attr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	ret = mptcp_pm_parse_entry(attr, info, false, &loc);
 	if (ret < 0)
 		goto set_flags_err;
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 		goto set_flags_err;
 	}
 
+	attr_rem = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
 	ret = mptcp_pm_parse_entry(attr_rem, info, false, &rem);
 	if (ret < 0)
 		goto set_flags_err;
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
 int mptcp_userspace_pm_get_addr(struct sk_buff *skb,
 				struct genl_info *info)
 {
-	struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	struct mptcp_pm_addr_entry addr, *entry;
 	struct mptcp_sock *msk;
 	struct sk_buff *msg;
+	struct nlattr *attr;
 	int ret = -EINVAL;
 	struct sock *sk;
 	void *reply;
 
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
+		return ret;
+
 	msk = mptcp_userspace_pm_get_sock(info);
 	if (!msk)
 		return ret;
 
 	sk = (struct sock *)msk;
 
+	attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	ret = mptcp_pm_parse_entry(attr, info, false, &addr);
 	if (ret < 0)
 		goto out;

-- 
2.47.1

Instead of only returning a text message with GENL_SET_ERR_MSG(),
NL_SET_ERR_MSG_ATTR() can help the userspace developers by also
reporting which attribute is faulty.

When the error is specific to an attribute, NL_SET_ERR_MSG_ATTR() is now
used. The error messages have not been modified in this commit.

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
 		return ret;
 
 	if (addr.addr.port && !address_use_port(&addr)) {
-		GENL_SET_ERR_MSG(info, "flags must have signal and not subflow when using port");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr,
+				    "flags must have signal and not subflow when using port");
 		return -EINVAL;
 	}
 
 	if (addr.flags & MPTCP_PM_ADDR_FLAG_SIGNAL &&
 	    addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH) {
-		GENL_SET_ERR_MSG(info, "flags mustn't have both signal and fullmesh");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr,
+				    "flags mustn't have both signal and fullmesh");
 		return -EINVAL;
 	}
 
 	if (addr.flags & MPTCP_PM_ADDR_FLAG_IMPLICIT) {
-		GENL_SET_ERR_MSG(info, "can't create IMPLICIT endpoint");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr,
+				    "can't create IMPLICIT endpoint");
 		return -EINVAL;
 	}
 
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_del_addr_doit(struct sk_buff *skb, struct genl_info *info)
 	spin_lock_bh(&pernet->lock);
 	entry = __lookup_addr_by_id(pernet, addr.addr.id);
 	if (!entry) {
-		GENL_SET_ERR_MSG(info, "address not found");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		spin_unlock_bh(&pernet->lock);
 		return -EINVAL;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_get_addr(struct sk_buff *skb, struct genl_info *info)
 	rcu_read_lock();
 	entry = __lookup_addr_by_id(pernet, addr.addr.id);
 	if (!entry) {
-		GENL_SET_ERR_MSG(info, "address not found");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		ret = -EINVAL;
 		goto unlock_fail;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info)
 	if (addr.addr.family == AF_UNSPEC) {
 		lookup_by_id = 1;
 		if (!addr.addr.id) {
-			GENL_SET_ERR_MSG(info, "missing address ID");
+			NL_SET_ERR_MSG_ATTR(info->extack, attr,
+					    "missing address ID");
 			return -EOPNOTSUPP;
 		}
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info)
 			       __lookup_addr(pernet, &addr.addr);
 	if (!entry) {
 		spin_unlock_bh(&pernet->lock);
-		GENL_SET_ERR_MSG(info, "address not found");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		return -EINVAL;
 	}
 	if ((addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH) &&
 	    (entry->flags & (MPTCP_PM_ADDR_FLAG_SIGNAL |
 			     MPTCP_PM_ADDR_FLAG_IMPLICIT))) {
 		spin_unlock_bh(&pernet->lock);
-		GENL_SET_ERR_MSG(info, "invalid addr flags");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr, "invalid addr flags");
 		return -EINVAL;
 	}
 
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *in
 	}
 
 	if (!mptcp_pm_is_userspace(msk)) {
-		GENL_SET_ERR_MSG(info, "userspace PM not selected");
+		NL_SET_ERR_MSG_ATTR(info->extack, token,
+				    "userspace PM not selected");
 		sock_put((struct sock *)msk);
 		return NULL;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 		goto announce_err;
 
 	if (addr_val.addr.id == 0) {
-		GENL_SET_ERR_MSG(info, "invalid addr id");
+		NL_SET_ERR_MSG_ATTR(info->extack, addr, "invalid addr id");
 		err = -EINVAL;
 		goto announce_err;
 	}
 
 	if (!(addr_val.flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
-		GENL_SET_ERR_MSG(info, "invalid addr flags");
+		NL_SET_ERR_MSG_ATTR(info->extack, addr, "invalid addr flags");
 		err = -EINVAL;
 		goto announce_err;
 	}
 
 	err = mptcp_userspace_pm_append_new_local_addr(msk, &addr_val, false);
 	if (err < 0) {
-		GENL_SET_ERR_MSG(info, "did not match address and id");
+		NL_SET_ERR_MSG_ATTR(info->extack, addr,
+				    "did not match address and id");
 		goto announce_err;
 	}
 
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 	err = 0;
 out:
 	if (err)
-		GENL_SET_ERR_MSG_FMT(info,
-				     "address with id %u not found",
-				     id_val);
+		NL_SET_ERR_MSG_ATTR_FMT(info->extack, id,
+					"address with id %u not found",
+					id_val);
 
 	sock_put(sk);
 	return err;
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 		goto create_err;
 
 	if (entry.flags & MPTCP_PM_ADDR_FLAG_SIGNAL) {
-		GENL_SET_ERR_MSG(info, "invalid addr flags");
+		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "invalid addr flags");
 		err = -EINVAL;
 		goto create_err;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 
 	err = mptcp_userspace_pm_append_new_local_addr(msk, &entry, false);
 	if (err < 0) {
-		GENL_SET_ERR_MSG(info, "did not match address and id");
+		NL_SET_ERR_MSG_ATTR(info->extack, laddr,
+				    "did not match address and id");
 		goto create_err;
 	}
 
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 	}
 
 	if (!addr_l.addr.port) {
-		GENL_SET_ERR_MSG(info, "missing local port");
+		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "missing local port");
 		err = -EINVAL;
 		goto destroy_err;
 	}
 
 	if (!addr_r.port) {
-		GENL_SET_ERR_MSG(info, "missing remote port");
+		NL_SET_ERR_MSG_ATTR(info->extack, raddr, "missing remote port");
 		err = -EINVAL;
 		goto destroy_err;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 		goto set_flags_err;
 
 	if (loc.addr.family == AF_UNSPEC) {
-		GENL_SET_ERR_MSG(info, "invalid local address family");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr,
+				    "invalid local address family");
 		ret = -EINVAL;
 		goto set_flags_err;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 		goto set_flags_err;
 
 	if (rem.addr.family == AF_UNSPEC) {
-		GENL_SET_ERR_MSG(info, "invalid remote address family");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr_rem,
+				    "invalid remote address family");
 		ret = -EINVAL;
 		goto set_flags_err;
 	}
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_get_addr(struct sk_buff *skb,
 	spin_lock_bh(&msk->pm.lock);
 	entry = mptcp_userspace_pm_lookup_addr_by_id(msk, addr.addr.id);
 	if (!entry) {
-		GENL_SET_ERR_MSG(info, "address not found");
+		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		ret = -EINVAL;
 		goto unlock_fail;
 	}

-- 
2.47.1

From: Geliang Tang <tanggeliang@kylinos.cn>

Three netlink functions:

mptcp_pm_nl_get_addr_doit()
	mptcp_pm_nl_get_addr_dumpit()
	mptcp_pm_nl_set_flags_doit()

are generic, implemented for each PM, in-kernel PM and userspace PM. It's
clearer to move them from pm_netlink.c to pm.c.

And the linked three path manager wrappers

mptcp_pm_get_addr()
	mptcp_pm_dump_addr()
	mptcp_pm_set_flags()

can be changed as static functions, no need to export them in protocol.h.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm.c         | 23 ++++++++++++++++++++---
 net/mptcp/pm_netlink.c | 16 ----------------
 net/mptcp/protocol.h   |  3 ---
 3 files changed, 20 insertions(+), 22 deletions(-)

-- 
2.47.1

From: Geliang Tang <tanggeliang@kylinos.cn>

The first parameters 'skb' of get_addr() interfaces are now useless
since mptcp_userspace_pm_get_sock() helper is used. This patch drops
these useless parameters of them.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm.c           | 8 ++++----
 net/mptcp/pm_netlink.c   | 2 +-
 net/mptcp/pm_userspace.c | 3 +--
 net/mptcp/protocol.h     | 5 ++---
 4 files changed, 8 insertions(+), 10 deletions(-)

-- 
2.47.1

From: Geliang Tang <tanggeliang@kylinos.cn>

The address id is parsed both in mptcp_pm_nl_get_addr() and
mptcp_userspace_pm_get_addr(), this makes the code somewhat repetitive.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
v2:
- Fix 'attr' no longer being set in mptcp_pm_nl_get_addr(), but still
  used in this patch (no longer in the next one). (Simon)
v3:
- Same fix, but in mptcp_userspace_pm_get_addr().
---
 net/mptcp/pm.c           | 20 ++++++++++++++++----
 net/mptcp/pm_netlink.c   | 16 ++++------------
 net/mptcp/pm_userspace.c | 16 ++++------------
 net/mptcp/protocol.h     |  4 ++--
 4 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -XXX,XX +XXX,XX @@ bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc)
 	return mptcp_pm_nl_is_backup(msk, &skc_local);
 }
 
-static int mptcp_pm_get_addr(struct genl_info *info)
+static int mptcp_pm_get_addr(u8 id, struct genl_info *info)
 {
 	if (info->attrs[MPTCP_PM_ATTR_TOKEN])
-		return mptcp_userspace_pm_get_addr(info);
-	return mptcp_pm_nl_get_addr(info);
+		return mptcp_userspace_pm_get_addr(id, info);
+	return mptcp_pm_nl_get_addr(id, info);
 }
 
 int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	return mptcp_pm_get_addr(info);
+	struct mptcp_pm_addr_entry addr;
+	struct nlattr *attr;
+	int ret;
+
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
+		return -EINVAL;
+
+	attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
+	ret = mptcp_pm_parse_entry(attr, info, false, &addr);
+	if (ret < 0)
+		return ret;
+
+	return mptcp_pm_get_addr(addr.addr.id, info);
 }
 
 static int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -XXX,XX +XXX,XX @@ int mptcp_nl_fill_addr(struct sk_buff *skb,
 	return -EMSGSIZE;
 }
 
-int mptcp_pm_nl_get_addr(struct genl_info *info)
+int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info)
 {
+	struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
-	struct mptcp_pm_addr_entry addr, *entry;
+	struct mptcp_pm_addr_entry *entry;
 	struct sk_buff *msg;
-	struct nlattr *attr;
 	void *reply;
 	int ret;
 
-	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
-		return -EINVAL;
-
-	attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
-	ret = mptcp_pm_parse_entry(attr, info, false, &addr);
-	if (ret < 0)
-		return ret;
-
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
 		return -ENOMEM;
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_get_addr(struct genl_info *info)
 	}
 
 	rcu_read_lock();
-	entry = __lookup_addr_by_id(pernet, addr.addr.id);
+	entry = __lookup_addr_by_id(pernet, id);
 	if (!entry) {
 		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		ret = -EINVAL;
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
 	return ret;
 }
 
-int mptcp_userspace_pm_get_addr(struct genl_info *info)
+int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info)
 {
-	struct mptcp_pm_addr_entry addr, *entry;
+	struct nlattr *attr = attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
+	struct mptcp_pm_addr_entry *entry;
 	struct mptcp_sock *msk;
 	struct sk_buff *msg;
-	struct nlattr *attr;
 	int ret = -EINVAL;
 	struct sock *sk;
 	void *reply;
 
-	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
-		return ret;
-
 	msk = mptcp_userspace_pm_get_sock(info);
 	if (!msk)
 		return ret;
 
 	sk = (struct sock *)msk;
 
-	attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
-	ret = mptcp_pm_parse_entry(attr, info, false, &addr);
-	if (ret < 0)
-		goto out;
-
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg) {
 		ret = -ENOMEM;
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_get_addr(struct genl_info *info)
 
 	lock_sock(sk);
 	spin_lock_bh(&msk->pm.lock);
-	entry = mptcp_userspace_pm_lookup_addr_by_id(msk, addr.addr.id);
+	entry = mptcp_userspace_pm_lookup_addr_by_id(msk, id);
 	if (!entry) {
 		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		ret = -EINVAL;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_dump_addr(struct sk_buff *msg,
 			  struct netlink_callback *cb);
 int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
 				 struct netlink_callback *cb);
-int mptcp_pm_nl_get_addr(struct genl_info *info);
-int mptcp_userspace_pm_get_addr(struct genl_info *info);
+int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info);
+int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info);
 
 static inline u8 subflow_get_local_id(const struct mptcp_subflow_context *subflow)
 {

-- 
2.47.1

From: Geliang Tang <tanggeliang@kylinos.cn>

The current get_addr() flow looks like this:

lock();
	entry = get_entry();
	send_nlmsg(entry);
	unlock();

After holding the lock, get the entry from the list, send the entry, and
finally release the lock.

This patch changes the process by getting the entry while holding the lock,
then making a copy of the entry so that the lock can be released. Finally,
the copy of the entry is sent without locking:

lock();
	entry = get_entry();
	*copy = *entry;
	unlock();

send_nlmsg(copy);

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm.c           | 39 +++++++++++++++++++++++++++++++++++----
 net/mptcp/pm_netlink.c   | 40 ++++++----------------------------------
 net/mptcp/pm_userspace.c | 42 +++++-------------------------------------
 net/mptcp/protocol.h     |  6 ++++--
 4 files changed, 50 insertions(+), 77 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -XXX,XX +XXX,XX @@ bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc)
 	return mptcp_pm_nl_is_backup(msk, &skc_local);
 }
 
-static int mptcp_pm_get_addr(u8 id, struct genl_info *info)
+static int mptcp_pm_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
+			     struct genl_info *info)
 {
 	if (info->attrs[MPTCP_PM_ATTR_TOKEN])
-		return mptcp_userspace_pm_get_addr(id, info);
-	return mptcp_pm_nl_get_addr(id, info);
+		return mptcp_userspace_pm_get_addr(id, addr, info);
+	return mptcp_pm_nl_get_addr(id, addr, info);
 }
 
 int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info)
 {
 	struct mptcp_pm_addr_entry addr;
 	struct nlattr *attr;
+	struct sk_buff *msg;
+	void *reply;
 	int ret;
 
 	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ENDPOINT_ADDR))
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_get_addr_doit(struct sk_buff *skb, struct genl_info *info)
 	if (ret < 0)
 		return ret;
 
-	return mptcp_pm_get_addr(addr.addr.id, info);
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	reply = genlmsg_put_reply(msg, info, &mptcp_genl_family, 0,
+				  info->genlhdr->cmd);
+	if (!reply) {
+		GENL_SET_ERR_MSG(info, "not enough space in Netlink message");
+		ret = -EMSGSIZE;
+		goto fail;
+	}
+
+	ret = mptcp_pm_get_addr(addr.addr.id, &addr, info);
+	if (ret) {
+		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
+		goto fail;
+	}
+
+	ret = mptcp_nl_fill_addr(msg, &addr);
+	if (ret)
+		goto fail;
+
+	genlmsg_end(msg, reply);
+	ret = genlmsg_reply(msg, info);
+	return ret;
+
+fail:
+	nlmsg_free(msg);
+	return ret;
 }
 
 static int mptcp_pm_dump_addr(struct sk_buff *msg, struct netlink_callback *cb)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -XXX,XX +XXX,XX @@ int mptcp_nl_fill_addr(struct sk_buff *skb,
 	return -EMSGSIZE;
 }
 
-int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info)
+int mptcp_pm_nl_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
+			 struct genl_info *info)
 {
-	struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
 	struct mptcp_pm_addr_entry *entry;
-	struct sk_buff *msg;
-	void *reply;
-	int ret;
-
-	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
-	if (!msg)
-		return -ENOMEM;
-
-	reply = genlmsg_put_reply(msg, info, &mptcp_genl_family, 0,
-				  info->genlhdr->cmd);
-	if (!reply) {
-		GENL_SET_ERR_MSG(info, "not enough space in Netlink message");
-		ret = -EMSGSIZE;
-		goto fail;
-	}
+	int ret = -EINVAL;
 
 	rcu_read_lock();
 	entry = __lookup_addr_by_id(pernet, id);
-	if (!entry) {
-		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
-		ret = -EINVAL;
-		goto unlock_fail;
+	if (entry) {
+		*addr = *entry;
+		ret = 0;
 	}
-
-	ret = mptcp_nl_fill_addr(msg, entry);
-	if (ret)
-		goto unlock_fail;
-
-	genlmsg_end(msg, reply);
-	ret = genlmsg_reply(msg, info);
-	rcu_read_unlock();
-	return ret;
-
-unlock_fail:
 	rcu_read_unlock();
 
-fail:
-	nlmsg_free(msg);
 	return ret;
 }
 
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
 	return ret;
 }
 
-int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info)
+int mptcp_userspace_pm_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
+				struct genl_info *info)
 {
-	struct nlattr *attr = attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
 	struct mptcp_pm_addr_entry *entry;
 	struct mptcp_sock *msk;
-	struct sk_buff *msg;
 	int ret = -EINVAL;
 	struct sock *sk;
-	void *reply;
 
 	msk = mptcp_userspace_pm_get_sock(info);
 	if (!msk)
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info)
 
 	sk = (struct sock *)msk;
 
-	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
-	if (!msg) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	reply = genlmsg_put_reply(msg, info, &mptcp_genl_family, 0,
-				  info->genlhdr->cmd);
-	if (!reply) {
-		GENL_SET_ERR_MSG(info, "not enough space in Netlink message");
-		ret = -EMSGSIZE;
-		goto fail;
-	}
-
 	lock_sock(sk);
 	spin_lock_bh(&msk->pm.lock);
 	entry = mptcp_userspace_pm_lookup_addr_by_id(msk, id);
-	if (!entry) {
-		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
-		ret = -EINVAL;
-		goto unlock_fail;
+	if (entry) {
+		*addr = *entry;
+		ret = 0;
 	}
-
-	ret = mptcp_nl_fill_addr(msg, entry);
-	if (ret)
-		goto unlock_fail;
-
-	genlmsg_end(msg, reply);
-	ret = genlmsg_reply(msg, info);
 	spin_unlock_bh(&msk->pm.lock);
 	release_sock(sk);
-	sock_put(sk);
-	return ret;
 
-unlock_fail:
-	spin_unlock_bh(&msk->pm.lock);
-	release_sock(sk);
-fail:
-	nlmsg_free(msg);
-out:
 	sock_put(sk);
 	return ret;
 }
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_dump_addr(struct sk_buff *msg,
 			  struct netlink_callback *cb);
 int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
 				 struct netlink_callback *cb);
-int mptcp_pm_nl_get_addr(u8 id, struct genl_info *info);
-int mptcp_userspace_pm_get_addr(u8 id, struct genl_info *info);
+int mptcp_pm_nl_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
+			 struct genl_info *info);
+int mptcp_userspace_pm_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
+				struct genl_info *info);
 
 static inline u8 subflow_get_local_id(const struct mptcp_subflow_context *subflow)
 {

-- 
2.47.1

From: Geliang Tang <tanggeliang@kylinos.cn>

The first parameter 'skb' in mptcp_pm_nl_set_flags() is only used to
obtained the network namespace, which can also be obtained through the
second parameters 'info' by using genl_info_net() helper.

This patch drops these useless parameters 'skb' in all three set_flags()
interfaces.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm.c           | 8 ++++----
 net/mptcp/pm_netlink.c   | 4 ++--
 net/mptcp/pm_userspace.c | 2 +-
 net/mptcp/protocol.h     | 4 ++--
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_get_addr_dumpit(struct sk_buff *msg,
 	return mptcp_pm_dump_addr(msg, cb);
 }
 
-static int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
+static int mptcp_pm_set_flags(struct genl_info *info)
 {
 	if (info->attrs[MPTCP_PM_ATTR_TOKEN])
-		return mptcp_userspace_pm_set_flags(skb, info);
-	return mptcp_pm_nl_set_flags(skb, info);
+		return mptcp_userspace_pm_set_flags(info);
+	return mptcp_pm_nl_set_flags(info);
 }
 
 int mptcp_pm_nl_set_flags_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	return mptcp_pm_set_flags(skb, info);
+	return mptcp_pm_set_flags(info);
 }
 
 void mptcp_pm_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -XXX,XX +XXX,XX @@ static int mptcp_nl_set_flags(struct net *net,
 	return ret;
 }
 
-int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info)
+int mptcp_pm_nl_set_flags(struct genl_info *info)
 {
 	struct mptcp_pm_addr_entry addr = { .addr = { .family = AF_UNSPEC }, };
 	u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP |
 			   MPTCP_PM_ADDR_FLAG_FULLMESH;
-	struct net *net = sock_net(skb->sk);
+	struct net *net = genl_info_net(info);
 	struct mptcp_pm_addr_entry *entry;
 	struct pm_nl_pernet *pernet;
 	struct nlattr *attr;
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 	return err;
 }
 
-int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
+int mptcp_userspace_pm_set_flags(struct genl_info *info)
 {
 	struct mptcp_pm_addr_entry loc = { .addr = { .family = AF_UNSPEC }, };
 	struct mptcp_pm_addr_entry rem = { .addr = { .family = AF_UNSPEC }, };
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -XXX,XX +XXX,XX @@ bool mptcp_lookup_subflow_by_saddr(const struct list_head *list,
 				   const struct mptcp_addr_info *saddr);
 bool mptcp_remove_anno_list_by_saddr(struct mptcp_sock *msk,
 				     const struct mptcp_addr_info *addr);
-int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info);
-int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info);
+int mptcp_pm_nl_set_flags(struct genl_info *info);
+int mptcp_userspace_pm_set_flags(struct genl_info *info);
 int mptcp_pm_announce_addr(struct mptcp_sock *msk,
 			   const struct mptcp_addr_info *addr,
 			   bool echo);

-- 
2.47.1

From: Geliang Tang <tanggeliang@kylinos.cn>

Generally, in the path manager interfaces, the local address is defined
as an mptcp_pm_addr_entry type address, while the remote address is
defined as an mptcp_addr_info type one:

(struct mptcp_pm_addr_entry *local, struct mptcp_addr_info *remote)

But the set_flags() interface uses two mptcp_pm_addr_entry type
parameters.

This patch changes the second one to mptcp_addr_info type and use helper
mptcp_pm_parse_addr() to parse it instead of using mptcp_pm_parse_entry().

-- 
2.47.1

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch updates the interfaces set_flags to reduce repetitive
code, adds a new parameter 'local' for them.

The local address is parsed in public helper mptcp_pm_nl_set_flags_doit(),
then pass it to mptcp_pm_nl_set_flags() and mptcp_userspace_pm_set_flags().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm.c           | 16 ++++++++++++++--
 net/mptcp/pm_netlink.c   | 35 +++++++++++++----------------------
 net/mptcp/pm_userspace.c | 19 +++++++------------
 net/mptcp/protocol.h     |  6 ++++--
 4 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_get_addr_dumpit(struct sk_buff *msg,
 
 static int mptcp_pm_set_flags(struct genl_info *info)
 {
+	struct mptcp_pm_addr_entry loc = { .addr = { .family = AF_UNSPEC }, };
+	struct nlattr *attr_loc;
+	int ret = -EINVAL;
+
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR))
+		return ret;
+
+	attr_loc = info->attrs[MPTCP_PM_ATTR_ADDR];
+	ret = mptcp_pm_parse_entry(attr_loc, info, false, &loc);
+	if (ret < 0)
+		return ret;
+
 	if (info->attrs[MPTCP_PM_ATTR_TOKEN])
-		return mptcp_userspace_pm_set_flags(info);
-	return mptcp_pm_nl_set_flags(info);
+		return mptcp_userspace_pm_set_flags(&loc, info);
+	return mptcp_pm_nl_set_flags(&loc, info);
 }
 
 int mptcp_pm_nl_set_flags_doit(struct sk_buff *skb, struct genl_info *info)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -XXX,XX +XXX,XX @@ static int mptcp_nl_set_flags(struct net *net,
 	return ret;
 }
 
-int mptcp_pm_nl_set_flags(struct genl_info *info)
+int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
+			  struct genl_info *info)
 {
-	struct mptcp_pm_addr_entry addr = { .addr = { .family = AF_UNSPEC }, };
+	struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP |
 			   MPTCP_PM_ADDR_FLAG_FULLMESH;
 	struct net *net = genl_info_net(info);
 	struct mptcp_pm_addr_entry *entry;
 	struct pm_nl_pernet *pernet;
-	struct nlattr *attr;
 	u8 lookup_by_id = 0;
 	u8 bkup = 0;
-	int ret;
-
-	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR))
-		return -EINVAL;
 
 	pernet = pm_nl_get_pernet(net);
 
-	attr = info->attrs[MPTCP_PM_ATTR_ADDR];
-	ret = mptcp_pm_parse_entry(attr, info, false, &addr);
-	if (ret < 0)
-		return ret;
-
-	if (addr.addr.family == AF_UNSPEC) {
+	if (local->addr.family == AF_UNSPEC) {
 		lookup_by_id = 1;
-		if (!addr.addr.id) {
+		if (!local->addr.id) {
 			NL_SET_ERR_MSG_ATTR(info->extack, attr,
 					    "missing address ID");
 			return -EOPNOTSUPP;
 		}
 	}
 
-	if (addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
+	if (local->flags & MPTCP_PM_ADDR_FLAG_BACKUP)
 		bkup = 1;
 
 	spin_lock_bh(&pernet->lock);
-	entry = lookup_by_id ? __lookup_addr_by_id(pernet, addr.addr.id) :
-			       __lookup_addr(pernet, &addr.addr);
+	entry = lookup_by_id ? __lookup_addr_by_id(pernet, local->addr.id) :
+			       __lookup_addr(pernet, &local->addr);
 	if (!entry) {
 		spin_unlock_bh(&pernet->lock);
 		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
 		return -EINVAL;
 	}
-	if ((addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH) &&
+	if ((local->flags & MPTCP_PM_ADDR_FLAG_FULLMESH) &&
 	    (entry->flags & (MPTCP_PM_ADDR_FLAG_SIGNAL |
 			     MPTCP_PM_ADDR_FLAG_IMPLICIT))) {
 		spin_unlock_bh(&pernet->lock);
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_set_flags(struct genl_info *info)
 		return -EINVAL;
 	}
 
-	changed = (addr.flags ^ entry->flags) & mask;
-	entry->flags = (entry->flags & ~mask) | (addr.flags & mask);
-	addr = *entry;
+	changed = (local->flags ^ entry->flags) & mask;
+	entry->flags = (entry->flags & ~mask) | (local->flags & mask);
+	*local = *entry;
 	spin_unlock_bh(&pernet->lock);
 
-	mptcp_nl_set_flags(net, &addr.addr, bkup, changed);
+	mptcp_nl_set_flags(net, &local->addr, bkup, changed);
 	return 0;
 }
 
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -XXX,XX +XXX,XX @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 	return err;
 }
 
-int mptcp_userspace_pm_set_flags(struct genl_info *info)
+int mptcp_userspace_pm_set_flags(struct mptcp_pm_addr_entry *local,
+				 struct genl_info *info)
 {
-	struct mptcp_pm_addr_entry loc = { .addr = { .family = AF_UNSPEC }, };
 	struct mptcp_addr_info rem = { .family = AF_UNSPEC, };
 	struct mptcp_pm_addr_entry *entry;
 	struct nlattr *attr, *attr_rem;
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct genl_info *info)
 	struct sock *sk;
 	u8 bkup = 0;
 
-	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR) ||
-	    GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE))
+	if (GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_ADDR_REMOTE))
 		return ret;
 
 	msk = mptcp_userspace_pm_get_sock(info);
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct genl_info *info)
 	sk = (struct sock *)msk;
 
 	attr = info->attrs[MPTCP_PM_ATTR_ADDR];
-	ret = mptcp_pm_parse_entry(attr, info, false, &loc);
-	if (ret < 0)
-		goto set_flags_err;
-
-	if (loc.addr.family == AF_UNSPEC) {
+	if (local->addr.family == AF_UNSPEC) {
 		NL_SET_ERR_MSG_ATTR(info->extack, attr,
 				    "invalid local address family");
 		ret = -EINVAL;
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct genl_info *info)
 		goto set_flags_err;
 	}
 
-	if (loc.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
+	if (local->flags & MPTCP_PM_ADDR_FLAG_BACKUP)
 		bkup = 1;
 
 	spin_lock_bh(&msk->pm.lock);
-	entry = mptcp_userspace_pm_lookup_addr(msk, &loc.addr);
+	entry = mptcp_userspace_pm_lookup_addr(msk, &local->addr);
 	if (entry) {
 		if (bkup)
 			entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP;
@@ -XXX,XX +XXX,XX @@ int mptcp_userspace_pm_set_flags(struct genl_info *info)
 	spin_unlock_bh(&msk->pm.lock);
 
 	lock_sock(sk);
-	ret = mptcp_pm_nl_mp_prio_send_ack(msk, &loc.addr, &rem, bkup);
+	ret = mptcp_pm_nl_mp_prio_send_ack(msk, &local->addr, &rem, bkup);
 	release_sock(sk);
 
 	/* mptcp_pm_nl_mp_prio_send_ack() only fails in one case */
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index XXXXXXX..XXXXXXX 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -XXX,XX +XXX,XX @@ bool mptcp_lookup_subflow_by_saddr(const struct list_head *list,
 				   const struct mptcp_addr_info *saddr);
 bool mptcp_remove_anno_list_by_saddr(struct mptcp_sock *msk,
 				     const struct mptcp_addr_info *addr);
-int mptcp_pm_nl_set_flags(struct genl_info *info);
-int mptcp_userspace_pm_set_flags(struct genl_info *info);
+int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
+			  struct genl_info *info);
+int mptcp_userspace_pm_set_flags(struct mptcp_pm_addr_entry *local,
+				 struct genl_info *info);
 int mptcp_pm_announce_addr(struct mptcp_sock *msk,
 			   const struct mptcp_addr_info *addr,
 			   bool echo);

-- 
2.47.1