netdev - Re: [PATCH net-next 11/15] mlxsw: spectrum_trap: Add devlink-trap policer support

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: <20200324203349.6a76e581@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Tue, 24 Mar 2020 20:33:49 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Ido Schimmel <idosch@...sch.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, jiri@...lanox.com,
        andrew@...n.ch, f.fainelli@...il.com, vivien.didelot@...il.com,
        roopa@...ulusnetworks.com, nikolay@...ulusnetworks.com,
        mlxsw@...lanox.com, Ido Schimmel <idosch@...lanox.com>
Subject: Re: [PATCH net-next 11/15] mlxsw: spectrum_trap: Add devlink-trap
 policer support

On Tue, 24 Mar 2020 21:32:46 +0200 Ido Schimmel wrote:
> +static int mlxsw_sp_trap_policer_params_check(u64 rate, u64 burst,
> +					      u8 *p_burst_size,
> +					      struct netlink_ext_ack *extack)
> +{
> +	int bs = fls64(burst);
> +
> +	if (rate < MLXSW_REG_QPCR_LOWEST_CIR) {
> +		NL_SET_ERR_MSG_MOD(extack, "Policer rate lower than limit");
> +		return -EINVAL;
> +	}
> +
> +	if (rate > MLXSW_REG_QPCR_HIGHEST_CIR) {
> +		NL_SET_ERR_MSG_MOD(extack, "Policer rate higher than limit");
> +		return -EINVAL;
> +	}
> +
> +	if (!bs) {
> +		NL_SET_ERR_MSG_MOD(extack, "Policer burst size lower than limit");
> +		return -EINVAL;
> +	}
> +
> +	--bs;
> +
> +	if (burst != (1 << bs)) {
> +		NL_SET_ERR_MSG_MOD(extack, "Policer burst size is not power of two");
> +		return -EINVAL;
> +	}
> +
> +	if (bs < MLXSW_REG_QPCR_LOWEST_CBS) {
> +		NL_SET_ERR_MSG_MOD(extack, "Policer burst size lower than limit");
> +		return -EINVAL;
> +	}
> +
> +	if (bs > MLXSW_REG_QPCR_HIGHEST_CBS) {
> +		NL_SET_ERR_MSG_MOD(extack, "Policer burst size higher than limit");
> +		return -EINVAL;
> +	}

Any chance we could make the min/max values part of the policer itself?
Are they dynamic? Seems like most drivers will have to repeat this
checking against constants while maybe core could have done it?

> +
> +	*p_burst_size = bs;
> +
> +	return 0;
> +}