netdev - Re: [PATCH net-next v2 08/13] genetlink: inline genl_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <986fc7c6-af01-20a0-e862-8f77b1ccae76@intel.com>
Date:   Thu, 3 Nov 2022 10:04:32 -0700
From:   Jacob Keller <jacob.e.keller@...el.com>
To:     Jakub Kicinski <kuba@...nel.org>, <davem@...emloft.net>
CC:     <netdev@...r.kernel.org>, <edumazet@...gle.com>,
        <pabeni@...hat.com>, <jiri@...nulli.us>, <razor@...ckwall.org>,
        <nicolas.dichtel@...nd.com>, <gnault@...hat.com>, <fw@...len.de>
Subject: Re: [PATCH net-next v2 08/13] genetlink: inline genl_get_cmd()



On 11/2/2022 2:33 PM, Jakub Kicinski wrote:
> All callers go via genl_get_cmd_split() now,
> so merge genl_get_cmd() into it.

In some sense this is merging genl_get_cmd_split into genl_get_cmd since 
thats the name we end up with, but practically/code-wise its merging the 
other way then renaming back to genl_get_cmd.

Still looks good.
> 
> Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
>   net/netlink/genetlink.c | 30 ++++++++++++------------------
>   1 file changed, 12 insertions(+), 18 deletions(-)
> 
> diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
> index 93e33e20a0e8..ec32b6063a3f 100644
> --- a/net/netlink/genetlink.c
> +++ b/net/netlink/genetlink.c
> @@ -181,14 +181,6 @@ static int genl_get_cmd_small(u32 cmd, const struct genl_family *family,
>   	return -ENOENT;
>   }
>   
> -static int genl_get_cmd(u32 cmd, const struct genl_family *family,
> -			struct genl_ops *op)
> -{
> -	if (!genl_get_cmd_full(cmd, family, op))
> -		return 0;
> -	return genl_get_cmd_small(cmd, family, op);
> -}
> -
>   static int
>   genl_cmd_full_to_split(struct genl_split_ops *op,
>   		       const struct genl_family *family,
> @@ -231,13 +223,15 @@ genl_cmd_full_to_split(struct genl_split_ops *op,
>   }
>   
>   static int
> -genl_get_cmd_split(u32 cmd, u8 flags, const struct genl_family *family,
> -		   struct genl_split_ops *op)
> +genl_get_cmd(u32 cmd, u8 flags, const struct genl_family *family,
> +	     struct genl_split_ops *op)
>   {
>   	struct genl_ops full;
>   	int err;
>   
> -	err = genl_get_cmd(cmd, family, &full);
> +	err = genl_get_cmd_full(cmd, family, &full);
> +	if (err == -ENOENT)
> +		err = genl_get_cmd_small(cmd, family, &full);
>   	if (err) {
>   		memset(op, 0, sizeof(*op));
>   		return err;
> @@ -867,7 +861,7 @@ static int genl_family_rcv_msg(const struct genl_family *family,
>   
>   	flags = (nlh->nlmsg_flags & NLM_F_DUMP) == NLM_F_DUMP ?
>   		GENL_CMD_CAP_DUMP : GENL_CMD_CAP_DO;
> -	if (genl_get_cmd_split(hdr->cmd, flags, family, &op))
> +	if (genl_get_cmd(hdr->cmd, flags, family, &op))
>   		return -EOPNOTSUPP;
>   
>   	if ((op.flags & GENL_ADMIN_PERM) &&
> @@ -1265,8 +1259,8 @@ static int ctrl_dumppolicy_start(struct netlink_callback *cb)
>   		ctx->single_op = true;
>   		ctx->op = nla_get_u32(tb[CTRL_ATTR_OP]);
>   
> -		if (genl_get_cmd_split(ctx->op, GENL_CMD_CAP_DO, rt, &doit) &&
> -		    genl_get_cmd_split(ctx->op, GENL_CMD_CAP_DUMP, rt, &dump)) {
> +		if (genl_get_cmd(ctx->op, GENL_CMD_CAP_DO, rt, &doit) &&
> +		    genl_get_cmd(ctx->op, GENL_CMD_CAP_DUMP, rt, &dump)) {
>   			NL_SET_BAD_ATTR(cb->extack, tb[CTRL_ATTR_OP]);
>   			return -ENOENT;
>   		}
> @@ -1406,10 +1400,10 @@ static int ctrl_dumppolicy(struct sk_buff *skb, struct netlink_callback *cb)
>   		struct genl_ops op;
>   
>   		if (ctx->single_op) {
> -			if (genl_get_cmd_split(ctx->op, GENL_CMD_CAP_DO,
> -					       ctx->rt, &doit) &&
> -			    genl_get_cmd_split(ctx->op, GENL_CMD_CAP_DUMP,
> -					       ctx->rt, &dumpit)) {
> +			if (genl_get_cmd(ctx->op, GENL_CMD_CAP_DO,
> +					 ctx->rt, &doit) &&
> +			    genl_get_cmd(ctx->op, GENL_CMD_CAP_DUMP,
> +					 ctx->rt, &dumpit)) {
>   				WARN_ON(1);
>   				return -ENOENT;
>   			}