netdev - Re: [PATCH net-next v5 1/4] ethtool: provide customized dim profile management

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: <20240409184400.4e5444f3@kernel.org>
Date: Tue, 9 Apr 2024 18:44:00 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Heng Qi <hengqi@...ux.alibaba.com>
Cc: netdev@...r.kernel.org, virtualization@...ts.linux.dev, "David S.
 Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Paolo
 Abeni <pabeni@...hat.com>, Jason Wang <jasowang@...hat.com>, "Michael S.
 Tsirkin" <mst@...hat.com>, Ratheesh Kannoth <rkannoth@...vell.com>,
 Alexander Lobakin <aleksander.lobakin@...el.com>, Xuan Zhuo
 <xuanzhuo@...ux.alibaba.com>
Subject: Re: [PATCH net-next v5 1/4] ethtool: provide customized dim profile
 management

On Tue,  9 Apr 2024 20:03:21 +0800 Heng Qi wrote:
> +/**
> + * coalesce_put_profile - fill reply with a nla nest with four child nla nests.
> + * @skb: socket buffer the message is stored in
> + * @attr_type: nest attr type ETHTOOL_A_COALESCE_*X_*QE_PROFILE
> + * @profile: data passed to userspace
> + * @supported_params: modifiable parameters supported by the driver
> + *
> + * Put a dim profile nest attribute. Refer to ETHTOOL_A_MODERATIONS_MODERATION.

unfortunately kdoc got more picky and it also wants us to document
return values now, you gotta add something like

 * Returns: true if ..

actually this functions seems to return negative error codes as bool..

> +static bool coalesce_put_profile(struct sk_buff *skb, u16 attr_type,
> +				 const struct dim_cq_moder *profile,
> +				 u32 supported_params)
> +{
> +	struct nlattr *profile_attr, *moder_attr;
> +	bool valid = false;
> +	int i;
> +
> +	for (i = 0; i < NET_DIM_PARAMS_NUM_PROFILES; i++) {
> +		if (profile[i].usec || profile[i].pkts || profile[i].comps) {
> +			valid = true;
> +			break;
> +		}
> +	}
> +
> +	if (!valid || !(supported_params & attr_to_mask(attr_type)))
> +		return false;
> +
> +	profile_attr = nla_nest_start(skb, attr_type);
> +	if (!profile_attr)
> +		return -EMSGSIZE;
> +
> +	for (i = 0; i < NET_DIM_PARAMS_NUM_PROFILES; i++) {
> +		moder_attr = nla_nest_start(skb, ETHTOOL_A_MODERATIONS_MODERATION);
> +		if (!moder_attr)
> +			goto nla_cancel_profile;
> +
> +		if (nla_put_u16(skb, ETHTOOL_A_MODERATION_USEC, profile[i].usec) ||
> +		    nla_put_u16(skb, ETHTOOL_A_MODERATION_PKTS, profile[i].pkts) ||
> +		    nla_put_u16(skb, ETHTOOL_A_MODERATION_COMPS, profile[i].comps))

u16 in netlink is almost always the wrong choice, sizes are rounded 
up to 4B, anyway, let's use u32 at netlink level.