From mboxrd@z Thu Jan  1 00:00:00 1970
From: Eric Dumazet <eric.dumazet@gmail.com>
Subject: Re: [PATCH net-next 2/2] net/sched: act_police: don't use spinlock in
 the data path
Date: Wed, 14 Nov 2018 22:46:26 -0800
Message-ID: <45d88e35-43f8-d9ae-4da4-de61c3591a62@gmail.com>
References: <cover.1536852493.git.dcaratti@redhat.com>
 <6d287f706fdcff76eb1e9a1c85ea6ce188db11d8.1536852493.git.dcaratti@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: netdev@vger.kernel.org
To: Davide Caratti <dcaratti@redhat.com>,
        Jamal Hadi Salim <jhs@mojatatu.com>,
        Cong Wang <xiyou.wangcong@gmail.com>,
        Jiri Pirko <jiri@resnulli.us>,
        "David S. Miller" <davem@davemloft.net>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-pf1-f196.google.com ([209.85.210.196]:36897 "EHLO
        mail-pf1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1726574AbeKOQxC (ORCPT
        <rfc822;netdev@vger.kernel.org>); Thu, 15 Nov 2018 11:53:02 -0500
Received: by mail-pf1-f196.google.com with SMTP id u3-v6so6523927pfm.4
        for <netdev@vger.kernel.org>; Wed, 14 Nov 2018 22:46:29 -0800 (PST)
In-Reply-To: <6d287f706fdcff76eb1e9a1c85ea6ce188db11d8.1536852493.git.dcaratti@redhat.com>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>



On 09/13/2018 10:29 AM, Davide Caratti wrote:
> use RCU instead of spinlocks, to protect concurrent read/write on
> act_police configuration. This reduces the effects of contention in the
> data path, in case multiple readers are present.
> 
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
> ---
>  net/sched/act_police.c | 156 ++++++++++++++++++++++++-----------------
>  1 file changed, 92 insertions(+), 64 deletions(-)
> 

I must be missing something obvious with this patch.

How can the following piece of code in tcf_police_act() can possibly be run
without a spinlock or something preventing multiple cpus messing badly with the state variables  ?


		now = ktime_get_ns();
		toks = min_t(s64, now - p->tcfp_t_c, p->tcfp_burst);
		if (p->peak_present) {
			ptoks = toks + p->tcfp_ptoks;
			if (ptoks > p->tcfp_mtu_ptoks)
				ptoks = p->tcfp_mtu_ptoks;
			ptoks -= (s64)psched_l2t_ns(&p->peak,
						    qdisc_pkt_len(skb));
		}
		toks += p->tcfp_toks;
		if (toks > p->tcfp_burst)
			toks = p->tcfp_burst;
		toks -= (s64)psched_l2t_ns(&p->rate, qdisc_pkt_len(skb));
		if ((toks|ptoks) >= 0) {
			p->tcfp_t_c = now;
			p->tcfp_toks = toks;
			p->tcfp_ptoks = ptoks;
			ret = p->tcfp_result;
			goto inc_drops;
		}