On Thu, 15 Nov 2007 13:41:54 +0100 Patrick McHardy <[EMAIL PROTECTED]> wrote:
> Eric Dumazet wrote: > > [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) > > > I fixed a compilation error with CONFIG_COMPAT and applied it, thanks > Eric. One question though: > > > +#define XT_TABLE_INFO_SZ (offsetof(struct xt_table_info, entries) \ > > + + nr_cpu_ids * sizeof(char *)) > > > > /* overflow check */ > > - if (tmp.size >= (INT_MAX - sizeof(struct xt_table_info)) / NR_CPUS - > > - SMP_CACHE_BYTES) > > + if (tmp.size >= INT_MAX / num_possible_cpus()) > > return -ENOMEM; > > We need to make sure offsetof(struct xt_table_info, entries) + > nr_cpu_ids * sizeof(char *) doesn't overflow, so why doesn't it > use nr_cpu_ids here as well? > nr_cpu_ids is <= NR_CPUS, so XT_TABLE_INFO_SZ cannot overflow The 'overflow check' we do here is in fact not very usefull now that we dont need to multiply tmp.size by NR_CPUS and potentially overflow the result. We can delete the test, because kmalloc()/vmalloc() will probably fail gracefully if we ask too much memory. We could imagine a dual Opteron machine, with a total of 32GB of ram, and it could be possible to load a 3GB iptable (that would consume 2*3GB of ram), but the 'overflow check' test actually forbids such a scenario. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
