Re: [RFT 3/3] iptables: lock free counters

[Stephen Hemminger <shemminger@vyatta.com>]

On Mon, 09 Feb 2009 16:52:59 +0100
Patrick McHardy <kaber@trash.net> wrote:

> Eric Dumazet wrote:
> > Stephen Hemminger a écrit :
> >> @@ -939,14 +973,30 @@ static struct xt_counters * alloc_counte
> >>  	counters = vmalloc_node(countersize, numa_node_id());
> >>  
> >>  	if (counters == NULL)
> >> -		return ERR_PTR(-ENOMEM);
> >> +		goto nomem;
> >> +
> >> +	tmp = xt_alloc_table_info(private->size);
> >> +	if (!tmp)
> >> +		goto free_counters;
> >> +
> > 
> >> +	xt_zero_table_entries(tmp);
> > This is not correct. We must copy rules and zero counters on the copied stuff.
> 
> Indeed.
It is in next version.

> >>  static int
> >>  do_add_counters(struct net *net, void __user *user, unsigned int len, int compat)
> >> @@ -1393,13 +1422,14 @@ do_add_counters(struct net *net, void __
> >>  		goto free;
> >>  	}
> >>  
> >> -	write_lock_bh(&t->lock);
> >> +	mutex_lock(&t->lock);
> >>  	private = t->private;
> >>  	if (private->number != num_counters) {
> >>  		ret = -EINVAL;
> >>  		goto unlock_up_free;
> >>  	}
> >>  
> >> +	preempt_disable();
> >>  	i = 0;
> >>  	/* Choose the copy that is on our node */
> 
> This isn't actually necessary, its merely an optimization. Since this
> can take quite a while, it might be nicer not to disable preempt.

Need to stay on same cpu, to avoid race of preempt and two cpu's updating
same counter entry (and 64 bit counter update is not atomic)