Re: conntrackd [ERROR] commit: Invalid argument

Re: conntrackd [ERROR] commit: Invalid argument

[Pablo Neira Ayuso]

[-- Attachment #1: Type: text/plain, Size: 1476 bytes --]

Hi Marco,

Marco Barbero wrote:
> conntrack-tools-0.9.7
> libnetfilter_conntrack-0.0.94
> libnfnetlink-0.0.38
> 
> kernel 2.6.25.5
> Mode ALARM
> 
> conntrackd -c from node master:
> 
> looking logs:
> 
> a lot of  [ERROR] commit: Invalid argument
> Mon Jun  9 15:01:26 2008        tcp      6 180 TIME_WAIT
> src=192.168.200.14 dst=62.149.195.137 sport=47144 dport=80 src=x.x.x.x
> dst=192.168.200.14 sport=80 dport=47144 [ASSURED] mark=0
> 
> and at the end:
> 
> [Mon Jun  9 15:01:26 2008] (pid=13176) [notice] Committed 1172 new entries
> [Mon Jun  9 15:01:26 2008] (pid=13176) [notice] 3294 entries can't be committed
> 
> Any hints?

Are your scripts committing the entries twice (ie. invoking conntrackd 
-c several times)? The only way to reproduce this that I have found is 
to double insert an existing conntrack with some NAT handling. In the 
upcoming 2.6.26 you'll get a EBUSY instead of EINVAL which sounds more 
reasonable.

Anyhow, does the patch attached fix this behaviour? The idea behind it 
is to check if there is a conntrack present in kernel, if so, just 
update the attributes of the conntrack object that are changeable to 
avoid the error. Would you mind testing it?

> [...]
> solved kernel panic issues but still I got 'entries can't be committed'
> [ERROR] commit: Invalid argument

Patrick posted a patch to netfilter-devel to fix the kernel panics. He 
has also passed it to -stable.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1253 bytes --]

diff --git a/src/cache_iterators.c b/src/cache_iterators.c
index c26d349..2fe7278 100644
--- a/src/cache_iterators.c
+++ b/src/cache_iterators.c
@@ -91,20 +91,29 @@ static int do_commit(void *data1, void *
 	 */
 	nfct_set_attr_u32(ct, ATTR_TIMEOUT, CONFIG(commit_timeout));
 
-	ret = nl_create_conntrack(ct);
-	if (ret == -1) {
-		switch(errno) {
-			case EEXIST:
-				c->commit_exist++;
-				break;
-			default:
-				dlog(LOG_ERR, "commit: %s", strerror(errno));
-				dlog_ct(STATE(log), u->ct, NFCT_O_PLAIN);
-				c->commit_fail++;
-				break;
-		}
-	} else {
-		c->commit_ok++;
+	ret = nl_exist_conntrack(ct);
+	switch (ret) {
+	case -1:
+		dlog(LOG_ERR, "commit-exist: %s", strerror(errno));
+		dlog_ct(STATE(log), ct, NFCT_O_PLAIN);
+		break;
+	case 0:
+		if (nl_create_conntrack(ct) == -1) {
+			dlog(LOG_ERR, "commit-create: %s", strerror(errno));
+			dlog_ct(STATE(log), ct, NFCT_O_PLAIN);
+			c->commit_fail++;
+		} else
+			c->commit_ok++;
+		break;
+	case 1:
+		c->commit_exist++;
+		if (nl_update_conntrack(ct) == -1) {
+			dlog(LOG_ERR, "commit-update: %s", strerror(errno));
+			dlog_ct(STATE(log), ct, NFCT_O_PLAIN);
+			c->commit_fail++;
+		} else
+			c->commit_ok++;
+		break;
 	}
 
 	/* keep iterating even if we have found errors */

Re: conntrackd [ERROR] commit: Invalid argument

[Rainer Sabelka]

On Wednesday 11 June 2008 15:25, Pablo Neira Ayuso wrote:
> Hi Marco,
> Marco Barbero wrote:
> > conntrack-tools-0.9.7
> > libnetfilter_conntrack-0.0.94
> > libnfnetlink-0.0.38
> >
> > kernel 2.6.25.5
> > Mode ALARM
> >
> > conntrackd -c from node master:
> >
> > looking logs:
> >
> > a lot of  [ERROR] commit: Invalid argument
> > Mon Jun  9 15:01:26 2008        tcp      6 180 TIME_WAIT
> > src=192.168.200.14 dst=62.149.195.137 sport=47144 dport=80 src=x.x.x.x
> > dst=192.168.200.14 sport=80 dport=47144 [ASSURED] mark=0
> >
> > and at the end:
> >
> > [Mon Jun  9 15:01:26 2008] (pid=13176) [notice] Committed 1172 new
> > entries [Mon Jun  9 15:01:26 2008] (pid=13176) [notice] 3294 entries
> > can't be committed
> >
> > Any hints?
>
> Are your scripts committing the entries twice (ie. invoking conntrackd
> -c several times)? 

In my case - yes I did.

> The only way to reproduce this that I have found is 
> to double insert an existing conntrack with some NAT handling. In the
> upcoming 2.6.26 you'll get a EBUSY instead of EINVAL which sounds more
> reasonable.
>
> Anyhow, does the patch attached fix this behaviour? The idea behind it
> is to check if there is a conntrack present in kernel, if so, just
> update the attributes of the conntrack object that are changeable to
> avoid the error. Would you mind testing it?

Thanks for the patch!
Now I see no more "commit: Invalid argument" in the logs. Instead I get 
something like this, which looks much fiendlier:

Jun 11 15:36:48 fw1b conntrack-tools[13273]: committing external cache
Jun 11 15:36:48 fw1b conntrack-tools[13273]: Committed 69 new entries
Jun 11 15:36:48 fw1b conntrack-tools[13273]: 53 entries ignored, already exist

But in rare cases I can see "commit-create: Cannot allocate memory".
I also noticed this a few times before applying this patch. Is this something 
I should worry about?

Jun 11 15:40:07 fw1b conntrack-tools[13383]: committing external cache
Jun 11 15:40:07 fw1b conntrack-tools[13383]: commit-create: Cannot allocate 
memory
Jun 11 15:40:07 fw1b conntrack-tools[13383]: Committed 33 new entries
Jun 11 15:40:07 fw1b conntrack-tools[13383]: 25 entries ignored, already exist
Jun 11 15:40:07 fw1b conntrack-tools[13383]: 1 entries can't be committed

Thanks,
-Rainer

Re: conntrackd [ERROR] commit: Invalid argument

[Pablo Neira Ayuso]

Rainer Sabelka wrote:
> On Wednesday 11 June 2008 15:25, Pablo Neira Ayuso wrote:
>> Are your scripts committing the entries twice (ie. invoking conntrackd
>> -c several times)? 
> 
> In my case - yes I did.
> 
>> The only way to reproduce this that I have found is 
>> to double insert an existing conntrack with some NAT handling. In the
>> upcoming 2.6.26 you'll get a EBUSY instead of EINVAL which sounds more
>> reasonable.
>>
>> Anyhow, does the patch attached fix this behaviour? The idea behind it
>> is to check if there is a conntrack present in kernel, if so, just
>> update the attributes of the conntrack object that are changeable to
>> avoid the error. Would you mind testing it?
> 
> Thanks for the patch!
> Now I see no more "commit: Invalid argument" in the logs. Instead I get 
> something like this, which looks much fiendlier:
> 
> Jun 11 15:36:48 fw1b conntrack-tools[13273]: committing external cache
> Jun 11 15:36:48 fw1b conntrack-tools[13273]: Committed 69 new entries
> Jun 11 15:36:48 fw1b conntrack-tools[13273]: 53 entries ignored, already exist
                                                             ^^^
I'll also fix the message (those entries are not ignored but updated). 
Then I'll commit the patch asap.

> But in rare cases I can see "commit-create: Cannot allocate memory".
> I also noticed this a few times before applying this patch. Is this something 
> I should worry about?

Yes, very strange. ctnetlink is hitting ENOMEM, ie. it cannot create the 
conntrack because there's no memory available. As for now ctnetlink is 
allocating conntracks via nf_conntrack_alloc() which uses GFP_ATOMIC. 
I'll send you a patch to relax this to check if that is the problem, 
otherwise I think that the error is bogus.

> Jun 11 15:40:07 fw1b conntrack-tools[13383]: committing external cache
> Jun 11 15:40:07 fw1b conntrack-tools[13383]: commit-create: Cannot allocate 
> memory
> Jun 11 15:40:07 fw1b conntrack-tools[13383]: Committed 33 new entries
> Jun 11 15:40:07 fw1b conntrack-tools[13383]: 25 entries ignored, already exist
> Jun 11 15:40:07 fw1b conntrack-tools[13383]: 1 entries can't be committed

What it is really annoying is the fact that you hit error with that less 
entries. I cannot reproduce that in my testbed with ~20000 entries. Are 
you using some kind of embedded device? Architecture?

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: conntrackd [ERROR] commit: Invalid argument

[Rainer Sabelka]

On Wednesday 11 June 2008 18:26, Pablo Neira Ayuso wrote:
> Rainer Sabelka wrote:
> > On Wednesday 11 June 2008 15:25, Pablo Neira Ayuso wrote:
> >> Are your scripts committing the entries twice (ie. invoking conntrackd
> >> -c several times)?
> >
> > In my case - yes I did.
> >
> >> The only way to reproduce this that I have found is
> >> to double insert an existing conntrack with some NAT handling. In the
> >> upcoming 2.6.26 you'll get a EBUSY instead of EINVAL which sounds more
> >> reasonable.
> >>
> >> Anyhow, does the patch attached fix this behaviour? The idea behind it
> >> is to check if there is a conntrack present in kernel, if so, just
> >> update the attributes of the conntrack object that are changeable to
> >> avoid the error. Would you mind testing it?
> >
> > Thanks for the patch!
> > Now I see no more "commit: Invalid argument" in the logs. Instead I get
> > something like this, which looks much fiendlier:
> >
> > Jun 11 15:36:48 fw1b conntrack-tools[13273]: committing external cache
> > Jun 11 15:36:48 fw1b conntrack-tools[13273]: Committed 69 new entries
> > Jun 11 15:36:48 fw1b conntrack-tools[13273]: 53 entries ignored, already
> > exist
>                                                              ^^^
> I'll also fix the message (those entries are not ignored but updated).
> Then I'll commit the patch asap.
>
> > But in rare cases I can see "commit-create: Cannot allocate memory".
> > I also noticed this a few times before applying this patch. Is this
> > something I should worry about?
>
> Yes, very strange. ctnetlink is hitting ENOMEM, ie. it cannot create the
> conntrack because there's no memory available. As for now ctnetlink is
> allocating conntracks via nf_conntrack_alloc() which uses GFP_ATOMIC.
> I'll send you a patch to relax this to check if that is the problem,
> otherwise I think that the error is bogus.
>
> > Jun 11 15:40:07 fw1b conntrack-tools[13383]: committing external cache
> > Jun 11 15:40:07 fw1b conntrack-tools[13383]: commit-create: Cannot
> > allocate memory
> > Jun 11 15:40:07 fw1b conntrack-tools[13383]: Committed 33 new entries
> > Jun 11 15:40:07 fw1b conntrack-tools[13383]: 25 entries ignored, already
> > exist Jun 11 15:40:07 fw1b conntrack-tools[13383]: 1 entries can't be
> > committed
>
> What it is really annoying is the fact that you hit error with that less
> entries. I cannot reproduce that in my testbed with ~20000 entries. Are
> you using some kind of embedded device? Architecture?

This is a vitual machine (i686) which 128 MByte of memory running under VMware 
server 1.0.4.

-Rainer

Re: conntrackd [ERROR] commit: Invalid argument

[Pablo Neira Ayuso]

[-- Attachment #1: Type: text/plain, Size: 2705 bytes --]

Rainer Sabelka wrote:
> On Wednesday 11 June 2008 18:26, Pablo Neira Ayuso wrote:
>> Rainer Sabelka wrote:
>>> On Wednesday 11 June 2008 15:25, Pablo Neira Ayuso wrote:
>>>> Are your scripts committing the entries twice (ie. invoking conntrackd
>>>> -c several times)?
>>> In my case - yes I did.
>>>
>>>> The only way to reproduce this that I have found is
>>>> to double insert an existing conntrack with some NAT handling. In the
>>>> upcoming 2.6.26 you'll get a EBUSY instead of EINVAL which sounds more
>>>> reasonable.
>>>>
>>>> Anyhow, does the patch attached fix this behaviour? The idea behind it
>>>> is to check if there is a conntrack present in kernel, if so, just
>>>> update the attributes of the conntrack object that are changeable to
>>>> avoid the error. Would you mind testing it?
>>> Thanks for the patch!
>>> Now I see no more "commit: Invalid argument" in the logs. Instead I get
>>> something like this, which looks much fiendlier:
>>>
>>> Jun 11 15:36:48 fw1b conntrack-tools[13273]: committing external cache
>>> Jun 11 15:36:48 fw1b conntrack-tools[13273]: Committed 69 new entries
>>> Jun 11 15:36:48 fw1b conntrack-tools[13273]: 53 entries ignored, already
>>> exist
>>                                                              ^^^
>> I'll also fix the message (those entries are not ignored but updated).
>> Then I'll commit the patch asap.
>>
>>> But in rare cases I can see "commit-create: Cannot allocate memory".
>>> I also noticed this a few times before applying this patch. Is this
>>> something I should worry about?
>> Yes, very strange. ctnetlink is hitting ENOMEM, ie. it cannot create the
>> conntrack because there's no memory available. As for now ctnetlink is
>> allocating conntracks via nf_conntrack_alloc() which uses GFP_ATOMIC.
>> I'll send you a patch to relax this to check if that is the problem,
>> otherwise I think that the error is bogus.
>>
>>> Jun 11 15:40:07 fw1b conntrack-tools[13383]: committing external cache
>>> Jun 11 15:40:07 fw1b conntrack-tools[13383]: commit-create: Cannot
>>> allocate memory
>>> Jun 11 15:40:07 fw1b conntrack-tools[13383]: Committed 33 new entries
>>> Jun 11 15:40:07 fw1b conntrack-tools[13383]: 25 entries ignored, already
>>> exist Jun 11 15:40:07 fw1b conntrack-tools[13383]: 1 entries can't be
>>> committed
>> What it is really annoying is the fact that you hit error with that less
>> entries. I cannot reproduce that in my testbed with ~20000 entries. Are
>> you using some kind of embedded device? Architecture?
> 
> This is a vitual machine (i686) which 128 MByte of memory running under VMware 
> server 1.0.4.

Would you try this patch?

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 2530 bytes --]

[PATCH] add allocation flag to nf_conntrack_alloc

ctnetlink does not need to allocate the conntrack entries with GFP_ATOMIC
as its code is executed in user context.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

Index: net-2.6/include/net/netfilter/nf_conntrack.h
===================================================================
--- net-2.6.orig/include/net/netfilter/nf_conntrack.h	2008-06-12 14:43:41.000000000 +0200
+++ net-2.6/include/net/netfilter/nf_conntrack.h	2008-06-12 14:43:59.000000000 +0200
@@ -239,7 +239,8 @@
 extern void nf_conntrack_free(struct nf_conn *ct);
 extern struct nf_conn *
 nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
-		   const struct nf_conntrack_tuple *repl);
+		   const struct nf_conntrack_tuple *repl,
+		   gfp_t allocation);
 
 /* It's confirmed if it is, or has been in the hash table. */
 static inline int nf_ct_is_confirmed(struct nf_conn *ct)
Index: net-2.6/net/netfilter/nf_conntrack_core.c
===================================================================
--- net-2.6.orig/net/netfilter/nf_conntrack_core.c	2008-06-12 14:42:05.000000000 +0200
+++ net-2.6/net/netfilter/nf_conntrack_core.c	2008-06-12 14:45:31.000000000 +0200
@@ -466,7 +466,8 @@
 }
 
 struct nf_conn *nf_conntrack_alloc(const struct nf_conntrack_tuple *orig,
-				   const struct nf_conntrack_tuple *repl)
+				   const struct nf_conntrack_tuple *repl,
+				   gfp_t allocation)
 {
 	struct nf_conn *ct = NULL;
 
@@ -491,7 +492,7 @@
 		}
 	}
 
-	ct = kmem_cache_zalloc(nf_conntrack_cachep, GFP_ATOMIC);
+	ct = kmem_cache_zalloc(nf_conntrack_cachep, allocation);
 	if (ct == NULL) {
 		pr_debug("nf_conntrack_alloc: Can't alloc conntrack.\n");
 		atomic_dec(&nf_conntrack_count);
@@ -543,7 +544,7 @@
 		return NULL;
 	}
 
-	ct = nf_conntrack_alloc(tuple, &repl_tuple);
+	ct = nf_conntrack_alloc(tuple, &repl_tuple, GFP_ATOMIC);
 	if (ct == NULL || IS_ERR(ct)) {
 		pr_debug("Can't allocate conntrack.\n");
 		return (struct nf_conntrack_tuple_hash *)ct;
Index: net-2.6/net/netfilter/nf_conntrack_netlink.c
===================================================================
--- net-2.6.orig/net/netfilter/nf_conntrack_netlink.c	2008-06-12 14:45:42.000000000 +0200
+++ net-2.6/net/netfilter/nf_conntrack_netlink.c	2008-06-12 14:45:50.000000000 +0200
@@ -1130,7 +1130,7 @@
 	struct nf_conn_help *help;
 	struct nf_conntrack_helper *helper;
 
-	ct = nf_conntrack_alloc(otuple, rtuple);
+	ct = nf_conntrack_alloc(otuple, rtuple, GFP_KERNEL);
 	if (ct == NULL || IS_ERR(ct))
 		return -ENOMEM;
 

Re: conntrackd [ERROR] commit: Invalid argument

[Patrick McHardy]

Pablo Neira Ayuso wrote:
> Rainer Sabelka wrote:
>>
>> This is a vitual machine (i686) which 128 MByte of memory running 
>> under VMware server 1.0.4.
> 
> Would you try this patch?
> 
 > [PATCH] add allocation flag to nf_conntrack_alloc

Even if it doesn't help in this case, it makes sense, so if you
don't mind I'll queue it for 2.6.27.

Re: conntrackd [ERROR] commit: Invalid argument

[Rainer Sabelka]

On Thursday 12 June 2008 17:01:04 Pablo Neira Ayuso wrote:
> Would you try this patch?

Hello Pablo,

thanks for your patch. I'just tried it, but I'm still getting the "Cannot 
allocate memory" message.

-Rainer

# conntrackd -c

Jun 12 23:34:30 fw1b conntrack-tools[5604]: committing external cache
Jun 12 23:34:30 fw1b conntrack-tools[5604]: commit-create: Cannot allocate 
memory
Jun 12 23:34:30 fw1b conntrack-tools[5604]: Committed 140 new entries
Jun 12 23:34:30 fw1b conntrack-tools[5604]: 1 entries can't be committed

Re: conntrackd [ERROR] commit: Invalid argument

[Rainer Sabelka]

On Thursday 12 June 2008 17:01:04 Pablo Neira Ayuso wrote:
> >> Yes, very strange. ctnetlink is hitting ENOMEM, ie. it cannot create the
> >> conntrack because there's no memory available. As for now ctnetlink is
> >> allocating conntracks via nf_conntrack_alloc() which uses GFP_ATOMIC.
> >> I'll send you a patch to relax this to check if that is the problem,
> >> otherwise I think that the error is bogus.
> >>
> >>> Jun 11 15:40:07 fw1b conntrack-tools[13383]: committing external cache
> >>> Jun 11 15:40:07 fw1b conntrack-tools[13383]: commit-create: Cannot
> >>> allocate memory
> >>> Jun 11 15:40:07 fw1b conntrack-tools[13383]: Committed 33 new entries
> >>> Jun 11 15:40:07 fw1b conntrack-tools[13383]: 25 entries ignored,
> >>> already exist Jun 11 15:40:07 fw1b conntrack-tools[13383]: 1 entries
> >>> can't be committed
> >>
> >> What it is really annoying is the fact that you hit error with that less
> >> entries. I cannot reproduce that in my testbed with ~20000 entries. Are
> >> you using some kind of embedded device? Architecture?
> >
> > This is a vitual machine (i686) which 128 MByte of memory running under
> > VMware server 1.0.4.
>
> Would you try this patch?

Pablo,

I tried to debug this a bit and added some printk()s in the 
ctnetlink_create_conntrack() function to find out where the ENOMEM is coming 
from:
So, now I see that nf_conntrack_alloc() is not returning this error, but it is 
coming from a couple of lines below in the same function:

        helper = nf_ct_helper_find_get(rtuple);
        if (helper) {
                help = nf_ct_helper_ext_add(ct, GFP_KERNEL);
                if (help == NULL) {
                        nf_ct_helper_put(helper);
                        err = -ENOMEM;
                        goto err;
                }

There, nf_ct_helper_ext_add() returns NULL, which causes ENOMEM to be 
returned.

I didn't debug this further because I'm rather lost in the code. But maybe 
this gives you some hint what's wrong.

-Rainer

Re: conntrackd [ERROR] commit: Invalid argument

[Pablo Neira Ayuso]

[-- Attachment #1: Type: text/plain, Size: 1691 bytes --]

Rainer Sabelka wrote:
> I tried to debug this a bit and added some printk()s in the 
> ctnetlink_create_conntrack() function to find out where the ENOMEM is coming 
> from:
> So, now I see that nf_conntrack_alloc() is not returning this error, but it is 
> coming from a couple of lines below in the same function:
> 
>         helper = nf_ct_helper_find_get(rtuple);
>         if (helper) {
>                 help = nf_ct_helper_ext_add(ct, GFP_KERNEL);
>                 if (help == NULL) {
>                         nf_ct_helper_put(helper);
>                         err = -ENOMEM;
>                         goto err;
>                 }
> 
> There, nf_ct_helper_ext_add() returns NULL, which causes ENOMEM to be 
> returned.
> 
> I didn't debug this further because I'm rather lost in the code. But maybe 
> this gives you some hint what's wrong.

I just noticed a bug that may be the reason for EINVAL while injecting
connections that have a helper. The messages that contained connections
with helpers were malformed (one attribute was missing). Attached a
patch to fix this problem in libnetfilter_conntrack (already applied to
git, so probably it is better if you check out a working copy). With
regards to ENOMEM, probably we're hitting it because of some malformed
message.

The other patch is not directly related but it reduces the size of the
messages that are sent to kernel space to check for the existence of a
conntrack.

I have put a lot effort on the synchronization protocols in this release
but it seems that the commit still need one spin. As always, any help
testing and reporting problems is appreciated.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

[-- Attachment #2: conntrack.patch --]
[-- Type: text/x-diff, Size: 1054 bytes --]

X-Git-Url: http://git.netfilter.org/cgi-bin/gitweb.cgi?p=conntrack-tools.git;a=blobdiff_plain;f=src%2Fnetlink.c;h=387062d5f29094733bc1e19760b82a877c15182b;hp=10c464360999fe6d1b4c39f85daa6997194ff7da;hb=807f1e477baf2eb7a642e65017ede0a079ebeb4d;hpb=40598325d5ff7a6b928640e456a377001aeae285

diff --git a/src/netlink.c b/src/netlink.c
index 10c4643..387062d 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -23,6 +23,7 @@
 #include "log.h"
 #include "debug.h"
 
+#include <string.h>
 #include <errno.h>
 
 int ignore_conntrack(struct nf_conntrack *ct)
@@ -219,8 +220,15 @@ int nl_overrun_request_resync(void)
 int nl_exist_conntrack(struct nf_conntrack *ct)
 {
 	int ret;
+	char __tmp[nfct_maxsize()];
+	struct nf_conntrack *tmp = (struct nf_conntrack *) (void *)__tmp;
 
-	ret = nfct_query(STATE(dump), NFCT_Q_GET, ct);
+	memset(__tmp, 0, sizeof(__tmp));
+
+	/* use the original tuple to check if it is there */
+	nfct_copy(tmp, ct, NFCT_CP_ORIG);
+
+	ret = nfct_query(STATE(dump), NFCT_Q_GET, tmp);
 	if (ret == -1)
 		return errno == ENOENT ? 0 : -1;
 

[-- Attachment #3: libnetfilter_conntrack.patch --]
[-- Type: text/x-diff, Size: 1931 bytes --]

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Sun, 15 Jun 2008 23:58:41 +0000 (+0200)
Subject: fix wrong ATTR_*_L3PROTO handling in the message building
X-Git-Url: http://git.netfilter.org/cgi-bin/gitweb.cgi?p=libnetfilter_conntrack.git;a=commitdiff_plain;h=29ce47fc611015a64f66d1ec93c67a9d998f0592;hp=0ceaca69ad2517e156066203111e153084140a18

fix wrong ATTR_*_L3PROTO handling in the message building

- include missing ATTR_MASTER_L3PROTO attribute into messages
- include ATTR_[ORIG|REPL]_L3PROTO iff there is at least another layer 3
attribute
---

diff --git a/src/conntrack/parse.c b/src/conntrack/parse.c
index a18e3ad..7b6c0c5 100644
--- a/src/conntrack/parse.c
+++ b/src/conntrack/parse.c
@@ -379,23 +379,29 @@ void __parse_conntrack(const struct nlmsghdr *nlh,
 {
 	struct nfgenmsg *nfhdr = NLMSG_DATA(nlh);
 
-	ct->tuple[__DIR_ORIG].l3protonum = nfhdr->nfgen_family;
-	set_bit(ATTR_ORIG_L3PROTO, ct->set);
+	if (cda[CTA_TUPLE_ORIG-1]) {
+		ct->tuple[__DIR_ORIG].l3protonum = nfhdr->nfgen_family;
+		set_bit(ATTR_ORIG_L3PROTO, ct->set);
 
-	ct->tuple[__DIR_REPL].l3protonum = nfhdr->nfgen_family;
-	set_bit(ATTR_REPL_L3PROTO, ct->set);
-
-	if (cda[CTA_TUPLE_ORIG-1])
 		__parse_tuple(cda[CTA_TUPLE_ORIG-1], 
 			      &ct->tuple[__DIR_ORIG], __DIR_ORIG, ct->set);
+	}
+
+	if (cda[CTA_TUPLE_REPLY-1]) {
+		ct->tuple[__DIR_REPL].l3protonum = nfhdr->nfgen_family;
+		set_bit(ATTR_REPL_L3PROTO, ct->set);
 
-	if (cda[CTA_TUPLE_REPLY-1])
 		__parse_tuple(cda[CTA_TUPLE_REPLY-1], 
 			      &ct->tuple[__DIR_REPL], __DIR_REPL, ct->set);
+	}
+
+	if (cda[CTA_TUPLE_MASTER-1]) {
+		ct->tuple[__DIR_MASTER].l3protonum = nfhdr->nfgen_family;
+		set_bit(ATTR_MASTER_L3PROTO, ct->set);
 
-	if (cda[CTA_TUPLE_MASTER-1])
 		__parse_tuple(cda[CTA_TUPLE_MASTER-1], 
 			      &ct->tuple[__DIR_MASTER], __DIR_MASTER, ct->set);
+	}
 
 	if (cda[CTA_NAT_SEQ_ADJ_ORIG-1])
 		__parse_nat_seq(cda[CTA_NAT_SEQ_ADJ_ORIG-1], ct, __DIR_ORIG);

Re: conntrackd [ERROR] commit: Invalid argument

[Rainer Sabelka]

On Monday 16 June 2008 02:31:07 Pablo Neira Ayuso wrote:
> Rainer Sabelka wrote:
> > I tried to debug this a bit and added some printk()s in the
> > ctnetlink_create_conntrack() function to find out where the ENOMEM is
> > coming from:
> > So, now I see that nf_conntrack_alloc() is not returning this error, but
> > it is coming from a couple of lines below in the same function:
> >
> >         helper = nf_ct_helper_find_get(rtuple);
> >         if (helper) {
> >                 help = nf_ct_helper_ext_add(ct, GFP_KERNEL);
> >                 if (help == NULL) {
> >                         nf_ct_helper_put(helper);
> >                         err = -ENOMEM;
> >                         goto err;
> >                 }
> >
> > There, nf_ct_helper_ext_add() returns NULL, which causes ENOMEM to be
> > returned.
> >
> > I didn't debug this further because I'm rather lost in the code. But
> > maybe this gives you some hint what's wrong.
>
> I just noticed a bug that may be the reason for EINVAL while injecting
> connections that have a helper. The messages that contained connections
> with helpers were malformed (one attribute was missing). Attached a
> patch to fix this problem in libnetfilter_conntrack (already applied to
> git, so probably it is better if you check out a working copy). With
> regards to ENOMEM, probably we're hitting it because of some malformed
> message.
>
> The other patch is not directly related but it reduces the size of the
> messages that are sent to kernel space to check for the existence of a
> conntrack.
>
> I have put a lot effort on the synchronization protocols in this release
> but it seems that the commit still need one spin. As always, any help
> testing and reporting problems is appreciated.

Thanks Pablo. I've tested your patches but unfortunately I still get "Cannot 
allocate memory" sometimes.

Jun 16 22:24:45 fw1b conntrack-tools[5599]: committing external cache
Jun 16 22:24:45 fw1b conntrack-tools[5599]: commit-create: Cannot allocate 
memory
Jun 16 22:24:45 fw1b conntrack-tools[5599]: Committed 623 new entries
Jun 16 22:24:45 fw1b conntrack-tools[5599]: 1 entries can't be committed

-Rainer

Re: conntrackd [ERROR] commit: Invalid argument

[Pablo Neira Ayuso]

Hi Rainer,

Rainer Sabelka wrote:
> Thanks Pablo. I've tested your patches but unfortunately I still get "Cannot 
> allocate memory" sometimes.
> 
> Jun 16 22:24:45 fw1b conntrack-tools[5599]: committing external cache
> Jun 16 22:24:45 fw1b conntrack-tools[5599]: commit-create: Cannot allocate 
> memory
> Jun 16 22:24:45 fw1b conntrack-tools[5599]: Committed 623 new entries
> Jun 16 22:24:45 fw1b conntrack-tools[5599]: 1 entries can't be committed

Sorry for the late reply. The problem seems to be in the kernel side. It
was introduced in 2.6.23. I have posted a patch that fixes it [1]. It
applies to 2.6.27-rc but it should be easy to backport it.

[1] http://marc.info/?l=netfilter-devel&m=121812264402131&w=2

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: conntrackd [ERROR] commit: Invalid argument

[Pablo Neira Ayuso]

Marco Barbero wrote:
> Thanks Pablo,
> Now I'm getting:
> 
> [Wed Jun 11 20:04:07 2008] (pid=9974) [notice] Committed 1179 new entries
> [Wed Jun 11 20:04:07 2008] (pid=9974) [notice] 1179 entries ignored,
> already exist
     ^^^
I'll fix this, the patch that I have sent (yet uncommitted to netfilter 
git) says "ignored" but it should say "updated" instead.

> [Wed Jun 11 20:04:07 2008] (pid=9974) [notice] 8 entries can't be committed
> [Wed Jun 11 20:04:07 2008] (pid=3012) [notice] resync with master table
> 
> 
> So it seems issue is now only on 8 entries
> 
> 4 entries with:
> [ERROR] commit-create: Invalid argument

Could you post the complete log message to check if the entry that we 
want to inject is malformed?

> and 4 entries with:
> [ERROR] commit-create: Cannot allocate memory

Could you also give a try to the kernel patch that I have sent to Rainer?

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers