netdev - Re: [PATCH] netfilter : struct xt_table

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 14 Nov 2007 18:55:44 +0100
From:	Eric Dumazet <dada1@...mosbay.com>
To:	Patrick McHardy <kaber@...sh.net>
Cc:	David Miller <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Netfilter Development Mailinglist 
	<netfilter-devel@...r.kernel.org>
Subject: Re: [PATCH] netfilter : struct xt_table_info diet

On Wed, 14 Nov 2007 18:19:41 +0100
Patrick McHardy <kaber@...sh.net> wrote:

> [netfilter-devel CCed]
> 
> Eric Dumazet wrote:
> > Hi David & Patrick
> > 
> > Please find a patch against net-2.6.25
> > 
> > Thank you
> > 
> > 
> > [PATCH] netfilter : struct xt_table_info diet
> > 
> > Instead of using a big array of NR_CPUS entries, we can compute the size needed at runtime, using nr_cpu_ids
> > 
> > This should save some ram (especially on David's machines where NR_CPUS=4096 : 32 KB can be saved per table, and 64KB for dynamically allocated ones (because of slab/slub alignements) )
> > 
> > In particular, the 'bootstrap' tables are not any more static (in data section) but on stack as their
> > size is now very small.
> > 
> > This also should reduce the size used on stack in compat functions 
> > (get_info() declares an automatic variable, that could be bigger than kernel stack size for big NR_CPUS)
> 
> 
> Thanks, this looks good. One question:
> 
> > diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
> > index 2909c92..ed3bd0b 100644
> > --- a/net/ipv4/netfilter/arp_tables.c
> > +++ b/net/ipv4/netfilter/arp_tables.c
> > @@ -811,7 +811,7 @@ static int do_replace(void __user *user, unsigned int len)
> >  		return -ENOPROTOOPT;
> >  
> >  	/* overflow check */
> > -	if (tmp.size >= (INT_MAX - sizeof(struct xt_table_info)) / NR_CPUS -
> > +	if (tmp.size >= (INT_MAX - XT_TABLE_INFO_SZ) / NR_CPUS -
> >  			SMP_CACHE_BYTES)
> 
> 
> Shouldn't NR_CPUs be replaced by nr_cpu_ids here? I'm wondering
> why we still include NR_CPUs in the calculation at all though,
> unlike in 2.4, we don't allocate one huge area of memory anymore
> but do one allocation per CPU. IIRC it even was you who changed
> that.
> 

Yes, doing an allocation per possible cpu was better than one giant 
allocation (memory savings and NUMA aware)

Well, technically speaking you are right, we may also replace these 
divides per NR_CPUS by nr_cpu_ids (or even better : num_possible_cpus())

Because with NR_CPUS=4096, we actually limit tmp.size to about 524000,
 what a shame ! :)

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html