Re: [PATCH nf-next,v2] netfilter: nft_counter: convert it to use per-cpu counters

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Fri, Jun 19, 2015 at 11:39:37AM +0200, Patrick McHardy wrote:
> On 19.06, Pablo Neira Ayuso wrote:
> >  static void nft_counter_eval(const struct nft_expr *expr,
> >  			     struct nft_regs *regs,
> >  			     const struct nft_pktinfo *pkt)
> >  {
> > -	struct nft_counter *priv = nft_expr_priv(expr);
> > +	struct nft_counter_percpu_priv *priv = nft_expr_priv(expr);
> > +	struct nft_counter_percpu *this_cpu;
> >  
> > -	write_seqlock_bh(&priv->lock);
> > -	priv->bytes += pkt->skb->len;
> > -	priv->packets++;
> > -	write_sequnlock_bh(&priv->lock);
> > +	this_cpu = this_cpu_ptr(priv->counter);
> > +	this_cpu->counter.bytes += pkt->skb->len;
> > +	this_cpu->counter.packets++;
> 
> u64_state_update_begin()/end()?

Right, will fix that, thanks!

> > @@ -67,23 +83,46 @@ static int nft_counter_init(const struct nft_ctx *ctx,
> >  			    const struct nft_expr *expr,
> >  			    const struct nlattr * const tb[])
> >  {
> > -	struct nft_counter *priv = nft_expr_priv(expr);
> > -
> > -	if (tb[NFTA_COUNTER_PACKETS])
> > -	        priv->packets = be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_PACKETS]));
> > -	if (tb[NFTA_COUNTER_BYTES])
> > -		priv->bytes = be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_BYTES]));
> > +	struct nft_counter_percpu_priv *priv = nft_expr_priv(expr);
> > +	struct nft_counter_percpu __percpu *cpu_stats;
> > +	struct nft_counter_percpu *this_cpu;
> > +
> > +	cpu_stats = netdev_alloc_pcpu_stats(struct nft_counter_percpu);
> > +	if (cpu_stats == NULL)
> > +		return ENOMEM;
> > +
> > +	preempt_disable();
> 
> Seems unnecessary, the stats are not active durign ->init().

I know, I need to find a better way to silent a lockdep preempt splat
that I'm getting here because of illegal use of this_cpu_ptr() with
preemption enabled.

> > +	this_cpu = this_cpu_ptr(cpu_stats);
> > +	if (tb[NFTA_COUNTER_PACKETS]) {
> > +	        this_cpu->counter.packets =
> > +			be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_PACKETS]));
> > +	}
> > +	if (tb[NFTA_COUNTER_BYTES]) {
> > +		this_cpu->counter.bytes =
> > +			be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_BYTES]));
> > +	}
> > +	preempt_enable();
> > +
> > +	priv->counter = cpu_stats;
> >  
> > -	seqlock_init(&priv->lock);
> >  	return 0;
> >  }