[ULOG] Fix memory allocation bug - take two

[ULOG] Fix memory allocation bug - take two

[Holger Eitzenberger]

Hi,

this is resend of my previous patch which fixes a bug in the way 
ULOG does allocate memory.  It uses NLMSG_GOODSIZE to portably
determine the size of 'nlbufsiz'.  Thanks Patrick!

Please apply.

Thanks.  /holger

[PATCH] [ULOG] Use better value for 'nlbufsiz'

[Holger Eitzenberger]

Performance tests showed that ULOG may fail on heavy loaded systems
because of failed order-N allocations (N >= 1).

The default value of 4096 is not optimal in the sense that it actually
allocates _two_ contigous physical pages.  Reasoning: ULOG uses
alloc_skb(), which adds another ~300 bytes for skb_shared_info.

This patch sets the default value to NLMSG_GOODSIZE and adds some
documentation at the top.

Signed-off-by: Holger Eitzenberger <heitzenberger@astaro.com>

---

 net/ipv4/netfilter/ipt_ULOG.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

d07d41421fb31a3759f66e41ebaea204c5d4146c
diff --git a/net/ipv4/netfilter/ipt_ULOG.c b/net/ipv4/netfilter/ipt_ULOG.c
index 641dbc4..2fe6413 100644
--- a/net/ipv4/netfilter/ipt_ULOG.c
+++ b/net/ipv4/netfilter/ipt_ULOG.c
@@ -35,6 +35,10 @@
  * each nlgroup you are using, so the total kernel memory usage increases
  * by that factor.
  *
+ * Actually you should use nlbufsiz a bit smaller than PAGE_SIZE, since
+ * nlbufsiz is used with alloc_skb, which adds another
+ * sizeof(struct skb_shared_info).  Use NLMSG_GOODSIZE instead.
+ *
  * flushtimeout:
  *   Specify, after how many hundredths of a second the queue should be
  *   flushed even if it is not full yet.
@@ -76,7 +80,7 @@ MODULE_ALIAS_NET_PF_PROTO(PF_NETLINK, NE
 
 #define PRINTR(format, args...) do { if (net_ratelimit()) printk(format , ## args); } while (0)
 
-static unsigned int nlbufsiz = 4096;
+static unsigned int nlbufsiz = NLMSG_GOODSIZE;
 module_param(nlbufsiz, uint, 0400);
 MODULE_PARM_DESC(nlbufsiz, "netlink buffer size");
 
-- 
1.0.GIT

Re: [PATCH] [ULOG] Use better value for 'nlbufsiz'

[Patrick McHardy]

Holger Eitzenberger wrote:
> Performance tests showed that ULOG may fail on heavy loaded systems
> because of failed order-N allocations (N >= 1).
> 
> The default value of 4096 is not optimal in the sense that it actually
> allocates _two_ contigous physical pages.  Reasoning: ULOG uses
> alloc_skb(), which adds another ~300 bytes for skb_shared_info.
> 
> This patch sets the default value to NLMSG_GOODSIZE and adds some
> documentation at the top.

Applied, thanks Holger.

[ULOG] Use better value for nlbufsiz

[Holger Eitzenberger]

Hi,

attached you will find a patch for a bug in ULOG which appeard during
our latest performance tests.

Please apply.

Thanks.  /holger

[PATCH] ULOG] Use better value for 'nlbufsiz'

[Holger Eitzenberger]

Performance tests showed that ULOG may fail on heavy loaded systems
because of failed order-N allocations (N >= 1).

The default value of 4096 is not optimal in the sense that it actually
allocates _two_ contigous physical pages.  Reasoning: ULOG uses
alloc_skb(), which adds another ~300 bytes for skb_shared_info.

This patch sets the default value to 3700 and adds some documentation
at the top.

Signed-off-by: Holger Eitzenberger <heitzenberger@astaro.com>

---

 net/ipv4/netfilter/ipt_ULOG.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

4109f782faca0719825e39633d71b5e8b5dc74df
diff --git a/net/ipv4/netfilter/ipt_ULOG.c b/net/ipv4/netfilter/ipt_ULOG.c
index 641dbc4..7a6adaf 100644
--- a/net/ipv4/netfilter/ipt_ULOG.c
+++ b/net/ipv4/netfilter/ipt_ULOG.c
@@ -35,6 +35,11 @@
  * each nlgroup you are using, so the total kernel memory usage increases
  * by that factor.
  *
+ * Actually you should use nlbufsiz a bit smaller than PAGE_SIZE, since
+ * nlbufsiz is used with alloc_skb, which adds another
+ * sizeof(struct skb_shared_info).  This is ~300 bytes on i386, so use a
+ * value of ~3700 instead.
+ *
  * flushtimeout:
  *   Specify, after how many hundredths of a second the queue should be
  *   flushed even if it is not full yet.
@@ -76,7 +81,7 @@ MODULE_ALIAS_NET_PF_PROTO(PF_NETLINK, NE
 
 #define PRINTR(format, args...) do { if (net_ratelimit()) printk(format , ## args); } while (0)
 
-static unsigned int nlbufsiz = 4096;
+static unsigned int nlbufsiz = 3700;
 module_param(nlbufsiz, uint, 0400);
 MODULE_PARM_DESC(nlbufsiz, "netlink buffer size");
 
-- 
1.0.GIT

Re: [PATCH] ULOG] Use better value for 'nlbufsiz'

[Patrick McHardy]

Holger Eitzenberger wrote:
> Performance tests showed that ULOG may fail on heavy loaded systems
> because of failed order-N allocations (N >= 1).
> 
> The default value of 4096 is not optimal in the sense that it actually
> allocates _two_ contigous physical pages.  Reasoning: ULOG uses
> alloc_skb(), which adds another ~300 bytes for skb_shared_info.
> 
> This patch sets the default value to 3700 and adds some documentation
> at the top.

I think we should just use NLMSG_GOODSIZE, which will use the maximum
amount to fit one page on any architecture.

Re: [PATCH] ULOG] Use better value for 'nlbufsiz'

[Holger Eitzenberger]

On Wed, Jan 18, 2006 at 05:51:06PM +0100, Patrick McHardy wrote:

> > This patch sets the default value to 3700 and adds some documentation
> > at the top.
> 
> I think we should just use NLMSG_GOODSIZE, which will use the maximum
> amount to fit one page on any architecture.

This sounds reasonable.  I will resend.

/holger