From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Miller Subject: Re: [PATCH net v2] net/sched: act_police: fix race condition on state variables Date: Tue, 20 Nov 2018 15:01:22 -0800 (PST) Message-ID: <20181120.150122.966909501495254169.davem@davemloft.net> References: <541a3b91af953326fa8f8fe046f0c284274377ff.1542748323.git.dcaratti@redhat.com> Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: edumazet@google.com, jhs@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us, netdev@vger.kernel.org, ivecera@redhat.com To: dcaratti@redhat.com Return-path: Received: from shards.monkeyblade.net ([23.128.96.9]:51104 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725900AbeKUKAO (ORCPT ); Wed, 21 Nov 2018 05:00:14 -0500 In-Reply-To: <541a3b91af953326fa8f8fe046f0c284274377ff.1542748323.git.dcaratti@redhat.com> Sender: netdev-owner@vger.kernel.org List-ID: From: Davide Caratti Date: Tue, 20 Nov 2018 22:18:44 +0100 > after 'police' configuration parameters were converted to use RCU instead > of spinlock, the state variables used to compute the traffic rate (namely > 'tcfp_toks', 'tcfp_ptoks' and 'tcfp_t_c') are erroneously read/updated in > the traffic path without any protection. > > Use a dedicated spinlock to avoid race conditions on these variables, and > ensure proper cache-line alignment. In this way, 'police' is still faster > than what we observed when 'tcf_lock' was used in the traffic path _ i.e. > reverting commit 2d550dbad83c ("net/sched: act_police: don't use spinlock > in the data path"). Moreover, we preserve the throughput improvement that > was obtained after 'police' started using per-cpu counters, when 'avrate' > is used instead of 'rate'. > > Changes since v1 (thanks to Eric Dumazet): > - call ktime_get_ns() before acquiring the lock in the traffic path > - use a dedicated spinlock instead of tcf_lock > - improve cache-line usage > > Fixes: 2d550dbad83c ("net/sched: act_police: don't use spinlock in the data path") > Reported-and-suggested-by: Eric Dumazet > Signed-off-by: Davide Caratti Applied.