netdev - Re: [PATCH RFC net-next] net/sched: act_police: add support for packet-per-second policing

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: <20210126183812.180d0d61@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Tue, 26 Jan 2021 18:38:12 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Simon Horman <simon.horman@...ronome.com>
Cc:     Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jiri Pirko <jiri@...lanox.com>, netdev@...r.kernel.org,
        oss-drivers@...ronome.com,
        Baowen Zheng <baowen.zheng@...igine.com>,
        Louis Peens <louis.peens@...ronome.com>
Subject: Re: [PATCH RFC net-next] net/sched: act_police: add support for
 packet-per-second policing

On Mon, 25 Jan 2021 16:18:19 +0100 Simon Horman wrote:
> From: Baowen Zheng <baowen.zheng@...igine.com>
> 
> Allow a policer action to enforce a rate-limit based on packets-per-second,
> configurable using a packet-per-second rate and burst parameters. This may
> be used in conjunction with existing byte-per-second rate limiting in the
> same policer action.
> 
> e.g.
> tc filter add dev tap1 parent ffff: u32 match \
>               u32 0 0 police pkts_rate 3000 pkts_burst 1000
> 
> Testing was unable to uncover a performance impact of this change on
> existing features.
> 
> Signed-off-by: Baowen Zheng <baowen.zheng@...igine.com>
> Signed-off-by: Simon Horman <simon.horman@...ronome.com>
> Signed-off-by: Louis Peens <louis.peens@...ronome.com>

> diff --git a/net/sched/act_police.c b/net/sched/act_police.c
> index 8d8452b1cdd4..d700b2105535 100644
> --- a/net/sched/act_police.c
> +++ b/net/sched/act_police.c
> @@ -42,6 +42,8 @@ static const struct nla_policy police_policy[TCA_POLICE_MAX + 1] = {
>  	[TCA_POLICE_RESULT]	= { .type = NLA_U32 },
>  	[TCA_POLICE_RATE64]     = { .type = NLA_U64 },
>  	[TCA_POLICE_PEAKRATE64] = { .type = NLA_U64 },
> +	[TCA_POLICE_PKTRATE64]  = { .type = NLA_U64 },
> +	[TCA_POLICE_PKTBURST64] = { .type = NLA_U64 },

Should we set the policy so that .min = 1?

>  };
>  
>  static int tcf_police_init(struct net *net, struct nlattr *nla,
> @@ -61,6 +63,7 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
>  	bool exists = false;
>  	u32 index;
>  	u64 rate64, prate64;
> +	u64 pps, ppsburst;
>  
>  	if (nla == NULL)
>  		return -EINVAL;
> @@ -183,6 +186,16 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
>  	if (tb[TCA_POLICE_AVRATE])
>  		new->tcfp_ewma_rate = nla_get_u32(tb[TCA_POLICE_AVRATE]);
>  
> +	if (tb[TCA_POLICE_PKTRATE64] && tb[TCA_POLICE_PKTBURST64]) {

Should we reject if only one is present?

> +		pps = nla_get_u64(tb[TCA_POLICE_PKTRATE64]);
> +		ppsburst = nla_get_u64(tb[TCA_POLICE_PKTBURST64]);
> +		if (pps) {
> +			new->pps_present = true;
> +			new->tcfp_pkt_burst = PSCHED_TICKS2NS(ppsburst);
> +			psched_ppscfg_precompute(&new->ppsrate, pps);
> +		}
> +	}
> +
>  	spin_lock_bh(&police->tcf_lock);
>  	spin_lock_bh(&police->tcfp_lock);
>  	police->tcfp_t_c = ktime_get_ns();

> +void psched_ppscfg_precompute(struct psched_pktrate *r,
> +			      u64 pktrate64)
> +{
> +	memset(r, 0, sizeof(*r));
> +	r->rate_pkts_ps = pktrate64;
> +	r->mult = 1;
> +	/* The deal here is to replace a divide by a reciprocal one
> +	 * in fast path (a reciprocal divide is a multiply and a shift)
> +	 *
> +	 * Normal formula would be :
> +	 *  time_in_ns = (NSEC_PER_SEC * pkt_num) / pktrate64
> +	 *
> +	 * We compute mult/shift to use instead :
> +	 *  time_in_ns = (len * mult) >> shift;
> +	 *
> +	 * We try to get the highest possible mult value for accuracy,
> +	 * but have to make sure no overflows will ever happen.
> +	 */
> +	if (r->rate_pkts_ps > 0) {
> +		u64 factor = NSEC_PER_SEC;
> +
> +		for (;;) {
> +			r->mult = div64_u64(factor, r->rate_pkts_ps);
> +			if (r->mult & (1U << 31) || factor & (1ULL << 63))
> +				break;
> +			factor <<= 1;
> +			r->shift++;

Aren't there helpers somewhere for the reciprocal divide
pre-calculation?

> +		}
> +	}
> +}
> +EXPORT_SYMBOL(psched_ppscfg_precompute);