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

[Jakub Kicinski <kuba@kernel.org>]

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;
> +}