Re: [PATCH] fix iptables on systems with discontiguous processor ids

[seb@frankengul.org]

On Wed, Oct 12, 2005 at 03:54:19PM -0700, David S. Miller wrote:
> From: Harald Welte <laforge@netfilter.org>
> Date: Tue, 11 Oct 2005 19:54:57 +0200
> 
> I think I found the bug in your patch Harald.
> 
> > @@ -716,8 +716,8 @@ static int translate_table(const char *n
> >  	}
> >  
> >  	/* And one copy for every other CPU */
> > -	for (i = 1; i < num_possible_cpus(); i++) {
> > -		memcpy(newinfo->entries + SMP_ALIGN(newinfo->size)*i,
> > +	for_each_cpu_mask(i, cpu_possible_map) {
> > +		memcpy(newinfo->entries + SMP_ALIGN(newinfo->size)*(1+i),
> >  		       newinfo->entries,
> >  		       SMP_ALIGN(newinfo->size));
> >  	}
> 
> This isn't correct I think.  It assumes cpu numbers are
> linear :-)
> 
> In the two cpu case of cpu "0" and cpu "2" what this loop
> will do is:
> 
> 1) copy entry 0 to entry 1
> 2) copy entry 0 to entry 3
> 
> which fails to initialize entry 2 and that's how we crash.
> 
> Maybe the loop is better constructed like this (btw, for_each_cpu()
> walks over cpu_possible_map, no need to expand that by hand):
> 
> 	for_each_cpu(i) {
> 		if (i == 0)
> 			continue;
> 
> 		memcpy(newinfo->entries + SMP_ALIGN(newinfo->size) * i,
> 		       newinfo->entries,
> 		       SMP_ALIGN(newinfo->size));
> 	}
> 
> Here is an updated patch, and it takes care of all of the platforms
> that don't export the necessary cpumask symbols.
> 
> Sebastian can you test this one out please?
> 
> Thanks a lot.

You're welcome :).

I'm pleased to announce that the mail i'm writing is on the machine with
your patch applied, and nothing bad happenned in the 5 first minutes.

So I can reasonnably assert that the patch is correct and fix the
problem.

Just one little point: Should the cpu_possible_map be an exported symbol
for all smp arch or not ?

	Seb