* [PATCH, take2] netfilter : struct xt_table_info diet
@ 2007-11-14 21:50 Eric Dumazet
2007-11-15 12:41 ` Patrick McHardy
2007-11-20 6:32 ` Simon Horman
0 siblings, 2 replies; 8+ messages in thread
From: Eric Dumazet @ 2007-11-14 21:50 UTC (permalink / raw)
To: Patrick McHardy, David S. Miller
Cc: Linux Netdev List, Netfilter Development Mailinglist
[-- Attachment #1: Type: text/plain, Size: 902 bytes --]
Hi David & Patrick
Please find an updated version of this patch, against net-2.6.25
(relax the check against sizes of loaded tables, using a divide by
num_possible_cpus() in place of NR_CPUS)
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)
Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
[-- Attachment #2: xt_table_info_diet.patch --]
[-- Type: text/plain, Size: 5732 bytes --]
diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index 9657c4e..e305f2d 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -269,9 +269,12 @@ struct xt_table_info
unsigned int underflow[NF_INET_NUMHOOKS];
/* ipt_entry tables: one per CPU */
- char *entries[NR_CPUS];
+ /* Note : this field MUST be the last one, see XT_TABLE_INFO_SZ */
+ char *entries[1];
};
+#define XT_TABLE_INFO_SZ (offsetof(struct xt_table_info, entries) \
+ + nr_cpu_ids * sizeof(char *))
extern int xt_register_target(struct xt_target *target);
extern void xt_unregister_target(struct xt_target *target);
extern int xt_register_targets(struct xt_target *target, unsigned int n);
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 2909c92..a21722d 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -811,8 +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 -
- SMP_CACHE_BYTES)
+ if (tmp.size >= INT_MAX / num_possible_cpus())
return -ENOMEM;
if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters))
return -ENOMEM;
@@ -1090,7 +1089,7 @@ int arpt_register_table(struct arpt_table *table,
{
int ret;
struct xt_table_info *newinfo;
- static struct xt_table_info bootstrap
+ struct xt_table_info bootstrap
= { 0, 0, 0, { 0 }, { 0 }, { } };
void *loc_cpu_entry;
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index a99fe89..0afef0f 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1090,7 +1090,8 @@ compat_calc_match(struct ipt_entry_match *m, int * size)
return 0;
}
-static int compat_calc_entry(struct ipt_entry *e, struct xt_table_info *info,
+static int compat_calc_entry(struct ipt_entry *e,
+ const struct xt_table_info *info,
void *base, struct xt_table_info *newinfo)
{
struct ipt_entry_target *t;
@@ -1118,7 +1119,7 @@ static int compat_calc_entry(struct ipt_entry *e, struct xt_table_info *info,
return 0;
}
-static int compat_table_info(struct xt_table_info *info,
+static int compat_table_info(const struct xt_table_info *info,
struct xt_table_info *newinfo)
{
void *loc_cpu_entry;
@@ -1127,13 +1128,9 @@ static int compat_table_info(struct xt_table_info *info,
if (!newinfo || !info)
return -EINVAL;
- memset(newinfo, 0, sizeof(struct xt_table_info));
- newinfo->size = info->size;
- newinfo->number = info->number;
- for (i = 0; i < NF_INET_NUMHOOKS; i++) {
- newinfo->hook_entry[i] = info->hook_entry[i];
- newinfo->underflow[i] = info->underflow[i];
- }
+ /* we dont care about newinfo->entries[] */
+ memcpy(newinfo, info, offsetof(struct xt_table_info, entries));
+ newinfo->initial_entries = 0;
loc_cpu_entry = info->entries[raw_smp_processor_id()];
return IPT_ENTRY_ITERATE(loc_cpu_entry, info->size,
compat_calc_entry, info, loc_cpu_entry, newinfo);
@@ -1327,8 +1324,7 @@ do_replace(void __user *user, unsigned int len)
return -ENOPROTOOPT;
/* 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;
if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters))
return -ENOMEM;
@@ -1861,8 +1857,7 @@ compat_do_replace(void __user *user, unsigned int len)
return -ENOPROTOOPT;
/* 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;
if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters))
return -ENOMEM;
@@ -2036,7 +2031,7 @@ compat_get_entries(struct compat_ipt_get_entries __user *uptr, int *len)
duprintf("t->private->number = %u\n",
private->number);
ret = compat_table_info(private, &info);
- if (!ret && get.size == info.size) {
+ if (!ret && get.size == info->size) {
ret = compat_copy_entries_to_user(private->size,
t, uptr->entrytable);
} else if (!ret) {
@@ -2159,7 +2154,7 @@ int ipt_register_table(struct xt_table *table, const struct ipt_replace *repl)
{
int ret;
struct xt_table_info *newinfo;
- static struct xt_table_info bootstrap
+ struct xt_table_info bootstrap
= { 0, 0, 0, { 0 }, { 0 }, { } };
void *loc_cpu_entry;
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index e1e87ef..e60c1b4 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -1042,8 +1042,7 @@ do_replace(void __user *user, unsigned int len)
return -EFAULT;
/* 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;
if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters))
return -ENOMEM;
@@ -1339,7 +1338,7 @@ int ip6t_register_table(struct xt_table *table,
{
int ret;
struct xt_table_info *newinfo;
- static struct xt_table_info bootstrap
+ struct xt_table_info bootstrap
= { 0, 0, 0, { 0 }, { 0 }, { } };
void *loc_cpu_entry;
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index d9a3bde..862b27d 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -495,7 +495,7 @@ struct xt_table_info *xt_alloc_table_info(unsigned int size)
if ((SMP_ALIGN(size) >> PAGE_SHIFT) + 2 > num_physpages)
return NULL;
- newinfo = kzalloc(sizeof(struct xt_table_info), GFP_KERNEL);
+ newinfo = kzalloc(XT_TABLE_INFO_SZ, GFP_KERNEL);
if (!newinfo)
return NULL;
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH, take2] netfilter : struct xt_table_info diet
2007-11-14 21:50 [PATCH, take2] netfilter : struct xt_table_info diet Eric Dumazet
@ 2007-11-15 12:41 ` Patrick McHardy
2007-11-15 15:45 ` Eric Dumazet
2007-11-20 6:32 ` Simon Horman
1 sibling, 1 reply; 8+ messages in thread
From: Patrick McHardy @ 2007-11-15 12:41 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S. Miller, Linux Netdev List,
Netfilter Development Mailinglist
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?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH, take2] netfilter : struct xt_table_info diet
2007-11-15 12:41 ` Patrick McHardy
@ 2007-11-15 15:45 ` Eric Dumazet
2007-11-15 15:58 ` Patrick McHardy
0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2007-11-15 15:45 UTC (permalink / raw)
To: Patrick McHardy
Cc: David S. Miller, Linux Netdev List,
Netfilter Development Mailinglist
On Thu, 15 Nov 2007 13:41:54 +0100
Patrick McHardy <kaber@trash.net> 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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH, take2] netfilter : struct xt_table_info diet
2007-11-15 15:45 ` Eric Dumazet
@ 2007-11-15 15:58 ` Patrick McHardy
0 siblings, 0 replies; 8+ messages in thread
From: Patrick McHardy @ 2007-11-15 15:58 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S. Miller, Linux Netdev List,
Netfilter Development Mailinglist
Eric Dumazet wrote:
> On Thu, 15 Nov 2007 13:41:54 +0100
> Patrick McHardy <kaber@trash.net> wrote:
>
>>> +#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
Yes, but nr_cpu_ids is >= num_possible_cpus, which is what we're
using with your patch.
> 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.
You're right, I'll remove it. Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH, take2] netfilter : struct xt_table_info diet
2007-11-14 21:50 [PATCH, take2] netfilter : struct xt_table_info diet Eric Dumazet
2007-11-15 12:41 ` Patrick McHardy
@ 2007-11-20 6:32 ` Simon Horman
2007-11-20 7:02 ` Herbert Xu
2007-11-20 7:54 ` Patrick McHardy
1 sibling, 2 replies; 8+ messages in thread
From: Simon Horman @ 2007-11-20 6:32 UTC (permalink / raw)
To: Eric Dumazet
Cc: Patrick McHardy, David S. Miller, Linux Netdev List,
Netfilter Development Mailinglist
From: Eric Dumazet <dada1@cosmosbay.com>
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)
Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
Tue, 20 Nov 2007 15:23:33 +0900, Simon Horman <horms@verge.net.au>
Up-port to net-2.6.25 6d20d53c3d54d80211247dbe5c9cf67fda083a88
diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index 9657c4e..e305f2d 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -269,9 +269,12 @@ struct xt_table_info
unsigned int underflow[NF_INET_NUMHOOKS];
/* ipt_entry tables: one per CPU */
- char *entries[NR_CPUS];
+ /* Note : this field MUST be the last one, see XT_TABLE_INFO_SZ */
+ char *entries[1];
};
+#define XT_TABLE_INFO_SZ (offsetof(struct xt_table_info, entries) \
+ + nr_cpu_ids * sizeof(char *))
extern int xt_register_target(struct xt_target *target);
extern void xt_unregister_target(struct xt_target *target);
extern int xt_register_targets(struct xt_target *target, unsigned int n);
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 2909c92..a21722d 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -811,8 +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 -
- SMP_CACHE_BYTES)
+ if (tmp.size >= INT_MAX / num_possible_cpus())
return -ENOMEM;
if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters))
return -ENOMEM;
@@ -1090,7 +1089,7 @@ int arpt_register_table(struct arpt_table *table,
{
int ret;
struct xt_table_info *newinfo;
- static struct xt_table_info bootstrap
+ struct xt_table_info bootstrap
= { 0, 0, 0, { 0 }, { 0 }, { } };
void *loc_cpu_entry;
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index a99fe89..0afef0f 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1090,7 +1090,8 @@ compat_calc_match(struct ipt_entry_match *m, int * size)
return 0;
}
-static int compat_calc_entry(struct ipt_entry *e, struct xt_table_info *info,
+static int compat_calc_entry(struct ipt_entry *e,
+ const struct xt_table_info *info,
void *base, struct xt_table_info *newinfo)
{
struct ipt_entry_target *t;
@@ -1118,7 +1119,7 @@ static int compat_calc_entry(struct ipt_entry *e, struct xt_table_info *info,
return 0;
}
-static int compat_table_info(struct xt_table_info *info,
+static int compat_table_info(const struct xt_table_info *info,
struct xt_table_info *newinfo)
{
void *loc_cpu_entry;
@@ -1127,13 +1128,9 @@ static int compat_table_info(struct xt_table_info *info,
if (!newinfo || !info)
return -EINVAL;
- memset(newinfo, 0, sizeof(struct xt_table_info));
- newinfo->size = info->size;
- newinfo->number = info->number;
- for (i = 0; i < NF_INET_NUMHOOKS; i++) {
- newinfo->hook_entry[i] = info->hook_entry[i];
- newinfo->underflow[i] = info->underflow[i];
- }
+ /* we dont care about newinfo->entries[] */
+ memcpy(newinfo, info, offsetof(struct xt_table_info, entries));
+ newinfo->initial_entries = 0;
loc_cpu_entry = info->entries[raw_smp_processor_id()];
return IPT_ENTRY_ITERATE(loc_cpu_entry, info->size,
compat_calc_entry, info, loc_cpu_entry, newinfo);
@@ -1327,8 +1324,7 @@ do_replace(void __user *user, unsigned int len)
return -ENOPROTOOPT;
/* 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;
if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters))
return -ENOMEM;
@@ -1861,8 +1857,7 @@ compat_do_replace(void __user *user, unsigned int len)
return -ENOPROTOOPT;
/* 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;
if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters))
return -ENOMEM;
@@ -2036,7 +2031,7 @@ compat_get_entries(struct compat_ipt_get_entries __user *uptr, int *len)
duprintf("t->private->number = %u\n",
private->number);
ret = compat_table_info(private, &info);
- if (!ret && get.size == info.size) {
+ if (!ret && get.size == info->size) {
ret = compat_copy_entries_to_user(private->size,
t, uptr->entrytable);
} else if (!ret) {
@@ -2159,7 +2154,7 @@ int ipt_register_table(struct xt_table *table, const struct ipt_replace *repl)
{
int ret;
struct xt_table_info *newinfo;
- static struct xt_table_info bootstrap
+ struct xt_table_info bootstrap
= { 0, 0, 0, { 0 }, { 0 }, { } };
void *loc_cpu_entry;
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index e1e87ef..e60c1b4 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -1042,8 +1042,7 @@ do_replace(void __user *user, unsigned int len)
return -EFAULT;
/* 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;
if (tmp.num_counters >= INT_MAX / sizeof(struct xt_counters))
return -ENOMEM;
@@ -1339,7 +1338,7 @@ int ip6t_register_table(struct xt_table *table,
{
int ret;
struct xt_table_info *newinfo;
- static struct xt_table_info bootstrap
+ struct xt_table_info bootstrap
= { 0, 0, 0, { 0 }, { 0 }, { } };
void *loc_cpu_entry;
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index d9a3bde..862b27d 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -495,7 +495,7 @@ struct xt_table_info *xt_alloc_table_info(unsigned int size)
if ((SMP_ALIGN(size) >> PAGE_SHIFT) + 2 > num_physpages)
return NULL;
- newinfo = kzalloc(sizeof(struct xt_table_info), GFP_KERNEL);
+ newinfo = kzalloc(XT_TABLE_INFO_SZ, GFP_KERNEL);
if (!newinfo)
return NULL;
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH, take2] netfilter : struct xt_table_info diet
2007-11-20 6:32 ` Simon Horman
@ 2007-11-20 7:02 ` Herbert Xu
2007-11-20 7:50 ` Patrick McHardy
2007-11-20 7:54 ` Patrick McHardy
1 sibling, 1 reply; 8+ messages in thread
From: Herbert Xu @ 2007-11-20 7:02 UTC (permalink / raw)
To: Simon Horman; +Cc: dada1, kaber, davem, netdev, netfilter-devel
Simon Horman <horms@verge.net.au> wrote:
>
> @@ -269,9 +269,12 @@ struct xt_table_info
> unsigned int underflow[NF_INET_NUMHOOKS];
>
> /* ipt_entry tables: one per CPU */
> - char *entries[NR_CPUS];
> + /* Note : this field MUST be the last one, see XT_TABLE_INFO_SZ */
> + char *entries[1];
Just do
char *entries[];
Not only is it ISO C99, but the compiler will die if it's not at
the end.
Actually I haven't read the rest of the patch, so scratch this
comment if you really need to have one entry there for some reason :)
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH, take2] netfilter : struct xt_table_info diet
2007-11-20 7:02 ` Herbert Xu
@ 2007-11-20 7:50 ` Patrick McHardy
0 siblings, 0 replies; 8+ messages in thread
From: Patrick McHardy @ 2007-11-20 7:50 UTC (permalink / raw)
To: Herbert Xu; +Cc: Simon Horman, dada1, davem, netdev, netfilter-devel
Herbert Xu wrote:
> Simon Horman <horms@verge.net.au> wrote:
>
>> @@ -269,9 +269,12 @@ struct xt_table_info
>> unsigned int underflow[NF_INET_NUMHOOKS];
>>
>> /* ipt_entry tables: one per CPU */
>> - char *entries[NR_CPUS];
>> + /* Note : this field MUST be the last one, see XT_TABLE_INFO_SZ */
>> + char *entries[1];
>>
>
> Just do
>
> char *entries[];
>
> Not only is it ISO C99, but the compiler will die if it's not at
> the end.
>
> Actually I haven't read the rest of the patch, so scratch this
> comment if you really need to have one entry there for some reason :)
>
I thought this too, but its needed for the bootstrap tables, which are
declared on the stack :)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH, take2] netfilter : struct xt_table_info diet
2007-11-20 6:32 ` Simon Horman
2007-11-20 7:02 ` Herbert Xu
@ 2007-11-20 7:54 ` Patrick McHardy
1 sibling, 0 replies; 8+ messages in thread
From: Patrick McHardy @ 2007-11-20 7:54 UTC (permalink / raw)
To: Simon Horman
Cc: Eric Dumazet, David S. Miller, Linux Netdev List,
Netfilter Development Mailinglist
Simon Horman wrote:
> From: Eric Dumazet <dada1@cosmosbay.com>
>
> 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)
This seems to be identical to the patch I already got queued
apart from reintroducing the NR_CPUS check. Am I missing
something?
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2007-11-20 7:55 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-11-14 21:50 [PATCH, take2] netfilter : struct xt_table_info diet Eric Dumazet
2007-11-15 12:41 ` Patrick McHardy
2007-11-15 15:45 ` Eric Dumazet
2007-11-15 15:58 ` Patrick McHardy
2007-11-20 6:32 ` Simon Horman
2007-11-20 7:02 ` Herbert Xu
2007-11-20 7:50 ` Patrick McHardy
2007-11-20 7:54 ` Patrick McHardy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).