Re: [PATCH mptcp-next v3] mptcp: use GENL_REQ_ATTR_CHECK for token

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 16/12/2024 09:31, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> A more general way to check if MPTCP_PM_ATTR_TOKEN exists in 'info'
> is to use GENL_REQ_ATTR_CHECK(info, MPTCP_PM_ATTR_TOKEN) instead of
> directly reading info->attrs[MPTCP_PM_ATTR_TOKEN] and then checking
> if it's NULL.
> 
> So this patch uses GENL_REQ_ATTR_CHECK() for 'token' in 'info' in
> mptcp_userspace_pm_get_sock().
> 
> 'Suggested-by: Jakub Kicinski <kuba@kernel.org>'
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> v3:
>  - use GENL_REQ_ATTR_CHECK in mptcp_userspace_pm_get_sock only
>  - drop GENL_SET_ERR_MSG as Matt suggested (thanks)

Thank you for the v3. While at it, do you want to do a similar change
everywhere we do:

  struct nlattr *X = info->attrs[MPTCP_PM_ATTR_Y]:
  if (!X) {
      GENL_SET_ERR_MSG(info, "missing ...");
      return Z;
  }

e.g. in mptcp_pm_nl_announce_doit() (addr), mptcp_pm_nl_remove_doit()
(id), mptcp_pm_nl_subflow_create_doit() (raddr, laddr),
mptcp_pm_nl_subflow_destroy_doit() (raddr, laddr).
(and maybe mptcp_pm_parse_pm_addr_attr(), but it needs more changes
around to pass the attribute ID, probably best not to change that)

Also, do you know if 'pm_nl_ctl' will continue to indicate a helpful
message if such attribute is missing (that's a message more for
userspace app dev)? Or does it need to be modified to display the
missing argument set by GENL_REQ_ATTR_CHECK()?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.