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: <20200325105121.GD1332836@splinter>
Date:   Wed, 25 Mar 2020 12:51:21 +0200
From:   Ido Schimmel <idosch@...sch.org>
To:     Jakub Kicinski <kuba@...nel.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, Mar 24, 2020 at 08:33:49PM -0700, Jakub Kicinski wrote:
> 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?

Yea, it's a good idea. We can also expose these values to user space,
but I think it's not really necessary and I prefer not to extend uAPI
unless we really have to.

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