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] [day] [month] [year] [list]

Date:   Wed, 25 Mar 2020 09:45: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 Wed, 25 Mar 2020 12:51:21 +0200 Ido Schimmel wrote:
> 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.

SGTM, thanks!