[PATCH 6/10][CTNETLINK] dump counters iif connection ended or counters filled up

[PATCH 6/10][CTNETLINK] dump counters iif connection ended or counters filled up

[Pablo Neira Ayuso]

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

This patch makes ctnetlink to dump counters iif connection reaches the
destroy state or altenatively if counters filled up.

AFAICS counters on NEW and UPDATE events doesn't provide interesting
information, they just consume the limited netlink bandwidth.

Upcoming conntrackd release in statistics mode uses counters from
DESTROY events to keep the contability of traffic that the firewall has
processed.

I think that this patch should also reset counters upon fill up event,
comments?

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

-- 
The dawn of the fourth age of Linux firewalling is coming; a time of
great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris


[-- Attachment #2: 06counters.patch --]
[-- Type: text/plain, Size: 1995 bytes --]

[CTNETLINK] dump counters iif connection ended or counters filled up

This patch makes ctnetlink to dump counters iif connection reaches the
destroy state or altenatively if counters filled up.

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

Index: net-2.6/net/ipv4/netfilter/ip_conntrack_netlink.c
===================================================================
--- net-2.6.orig/net/ipv4/netfilter/ip_conntrack_netlink.c	2006-07-06 19:52:54.000000000 +0200
+++ net-2.6/net/ipv4/netfilter/ip_conntrack_netlink.c	2006-07-06 19:54:00.000000000 +0200
@@ -381,8 +381,10 @@ static int ctnetlink_conntrack_event(str
 	    && ctnetlink_dump_helpinfo(skb, ct) < 0)
 		goto nfattr_failure;
 
-	if (ctnetlink_dump_counters(skb, ct, IP_CT_DIR_ORIGINAL) < 0 ||
-	    ctnetlink_dump_counters(skb, ct, IP_CT_DIR_REPLY) < 0)
+	/* this connection has died or counters wrapped around */
+	if ((events & IPCT_DESTROY || events & IPCT_COUNTER_FILLING)
+	    && (ctnetlink_dump_counters(skb, ct, IP_CT_DIR_ORIGINAL) < 0 ||
+	        ctnetlink_dump_counters(skb, ct, IP_CT_DIR_REPLY) < 0))
 		goto nfattr_failure;
 
 	if (events & IPCT_MARK
Index: net-2.6/net/netfilter/nf_conntrack_netlink.c
===================================================================
--- net-2.6.orig/net/netfilter/nf_conntrack_netlink.c	2006-07-06 19:54:02.000000000 +0200
+++ net-2.6/net/netfilter/nf_conntrack_netlink.c	2006-07-06 19:54:35.000000000 +0200
@@ -391,8 +391,10 @@ static int ctnetlink_conntrack_event(str
 	    && ctnetlink_dump_helpinfo(skb, ct) < 0)
 		goto nfattr_failure;
 
-	if (ctnetlink_dump_counters(skb, ct, IP_CT_DIR_ORIGINAL) < 0 ||
-	    ctnetlink_dump_counters(skb, ct, IP_CT_DIR_REPLY) < 0)
+	/* this connection has died or counters wrapped around */
+	if ((events & IPCT_DESTROY || events & IPCT_COUNTER_FILLING)
+	     && (ctnetlink_dump_counters(skb, ct, IP_CT_DIR_ORIGINAL) < 0 ||
+		 ctnetlink_dump_counters(skb, ct, IP_CT_DIR_REPLY) < 0))
 		goto nfattr_failure;
 
 	if (events & IPCT_MARK

Re: [PATCH 6/10][CTNETLINK] dump counters iif connection ended or counters filled up

[Patrick McHardy]

Pablo Neira Ayuso wrote:
> This patch makes ctnetlink to dump counters iif connection reaches the
> destroy state or altenatively if counters filled up.
> 
> AFAICS counters on NEW and UPDATE events doesn't provide interesting
> information, they just consume the limited netlink bandwidth.
> 
> Upcoming conntrackd release in statistics mode uses counters from
> DESTROY events to keep the contability of traffic that the firewall has
> processed.
> 
> I think that this patch should also reset counters upon fill up event,
> comments?

Not sure, do you know any users of the counters besides conntrackd?
I would like to look at how they're used.

Re: [PATCH 6/10][CTNETLINK] dump counters iif connection ended or counters filled up

[Amin Azez]

Patrick McHardy wrote:
> Pablo Neira Ayuso wrote:
>> This patch makes ctnetlink to dump counters iif connection reaches the
>> destroy state or altenatively if counters filled up.
>>
>> AFAICS counters on NEW and UPDATE events doesn't provide interesting
>> information, they just consume the limited netlink bandwidth.
>>
>> Upcoming conntrackd release in statistics mode uses counters from
>> DESTROY events to keep the contability of traffic that the firewall has
>> processed.
>>
>> I think that this patch should also reset counters upon fill up event,
>> comments?
> 
> Not sure, do you know any users of the counters besides conntrackd?
> I would like to look at how they're used.

I have a client daemon that reads the output of conntrackd, I would
prefer overflow rather than zero-ing. A main advantage is that "missing"
and event (netlink packet dropped) would not loose any information as
the overflow is visible by the fact that the counters are less than the
previous logged value and presuming they didn't overflow twice, its
obvious how much data passed in the mean time.

We could effectively increase the counter size by making the
counter-filled mask be the top few bits instead of just the top bit.
Then to actually wrap sooner (nearest thing to resetting that I like) we
just clear these top few bits.

Sam

Re: [PATCH 6/10][CTNETLINK] dump counters iif connection ended or counters filled up

[Pablo Neira Ayuso]

Patrick McHardy wrote:
> Pablo Neira Ayuso wrote:
> 
>>This patch makes ctnetlink to dump counters iif connection reaches the
>>destroy state or altenatively if counters filled up.
>>
>>AFAICS counters on NEW and UPDATE events doesn't provide interesting
>>information, they just consume the limited netlink bandwidth.
>>
>>Upcoming conntrackd release in statistics mode uses counters from
>>DESTROY events to keep the contability of traffic that the firewall has
>>processed.
>>
>>I think that this patch should also reset counters upon fill up event,
>>comments?
> 
> Not sure, do you know any users of the counters besides conntrackd?

I don't know any ctnetlink user of the counters. Thinking it well this 
"counters fill up" issue is tricky. Since netlink is unreliable, what if 
the fill up event gets lost? we could reset counters and nobody would 
apparently notice. I think that we need an overflow bit in the conntrack 
that must be set whenever and overflow happens and unset such bit once 
the overflow event has been caught.

-- 
The dawn of the fourth age of Linux firewalling is coming; a time of 
great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris

Re: [PATCH 6/10][CTNETLINK] dump counters iif connection ended or counters filled up

[Amin Azez]

Pablo Neira Ayuso wrote:
> Patrick McHardy wrote:
>> Pablo Neira Ayuso wrote:
>>
>>> This patch makes ctnetlink to dump counters iif connection reaches the
>>> destroy state or altenatively if counters filled up.
>>>
>>> AFAICS counters on NEW and UPDATE events doesn't provide interesting
>>> information, they just consume the limited netlink bandwidth.
>>>
>>> Upcoming conntrackd release in statistics mode uses counters from
>>> DESTROY events to keep the contability of traffic that the firewall has
>>> processed.
>>>
>>> I think that this patch should also reset counters upon fill up event,
>>> comments?
>>
>> Not sure, do you know any users of the counters besides conntrackd?
> 
> I don't know any ctnetlink user of the counters. Thinking it well this
> "counters fill up" issue is tricky. Since netlink is unreliable, what if
> the fill up event gets lost? we could reset counters and nobody would
> apparently notice. I think that we need an overflow bit in the conntrack
> that must be set whenever and overflow happens and unset such bit once
> the overflow event has been caught.
> 

What does it mean to "catch" the overflow even, esp as there may be
multiple clients each catching it (or not) for themselves.

A straight overflow is more simple. For clients that don't want to track
all counters, they can watch when the counter gets high, and then only
track counters that get so high, to count the number of wraps.

Sam

Re: [PATCH 6/10][CTNETLINK] dump counters iif connection ended or counters filled up

[Patrick McHardy]

Pablo Neira Ayuso wrote:
> Patrick McHardy wrote:
> 
>> Pablo Neira Ayuso wrote:
>>
>>> I think that this patch should also reset counters upon fill up event,
>>> comments?
>>
>>
>> Not sure, do you know any users of the counters besides conntrackd?
> 
> 
> I don't know any ctnetlink user of the counters. Thinking it well this
> "counters fill up" issue is tricky. Since netlink is unreliable, what if
> the fill up event gets lost? we could reset counters and nobody would
> apparently notice. I think that we need an overflow bit in the conntrack
> that must be set whenever and overflow happens and unset such bit once
> the overflow event has been caught.


If a message is lost userspace simply _must_ resync if it wants
reliability. If we reset the counter its impossible for userspace
to reconstruct the value at which it overflowed, so I don't think
we should do that. But you raise a good point, how should userspace
react to lost DESTROY messages that contain new information (like
counter values)? I don't see how it could reliable handle that case.

Re: [PATCH 6/10][CTNETLINK] dump counters iif connection ended or counters filled up

[Pablo Neira Ayuso]

Patrick McHardy wrote:
> Pablo Neira Ayuso wrote:
> 
>>Patrick McHardy wrote:
>>
>>
>>>Pablo Neira Ayuso wrote:
>>>
>>>
>>>>I think that this patch should also reset counters upon fill up event,
>>>>comments?
>>>
>>>
>>>Not sure, do you know any users of the counters besides conntrackd?
>>
>>
>>I don't know any ctnetlink user of the counters. Thinking it well this
>>"counters fill up" issue is tricky. Since netlink is unreliable, what if
>>the fill up event gets lost? we could reset counters and nobody would
>>apparently notice. I think that we need an overflow bit in the conntrack
>>that must be set whenever and overflow happens and unset such bit once
>>the overflow event has been caught.
> 
> If a message is lost userspace simply _must_ resync if it wants
> reliability. If we reset the counter its impossible for userspace
> to reconstruct the value at which it overflowed, so I don't think
> we should do that. But you raise a good point, how should userspace
> react to lost DESTROY messages that contain new information (like
> counter values)? I don't see how it could reliable handle that case.

I can't see any either at the moment :(

BTW, something related to the overflow issue: what I'm currently doing 
in conntrackd is the following thing, if an overflow is detected, the 
size of socket buffer is duplicated via RCVBUFFFORCE and then it 
resyncs. I'll add a clause to limit the socket buffer maximum growth. 
Can you see any problem with this?

-- 
The dawn of the fourth age of Linux firewalling is coming; a time of 
great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris

Re: [PATCH 6/10][CTNETLINK] dump counters iif connection ended or counters filled up

[Patrick McHardy]

On Thu, 13 Jul 2006, Pablo Neira Ayuso wrote:

> Patrick McHardy wrote:
>> If a message is lost userspace simply _must_ resync if it wants
>> reliability. If we reset the counter its impossible for userspace
>> to reconstruct the value at which it overflowed, so I don't think
>> we should do that. But you raise a good point, how should userspace
>> react to lost DESTROY messages that contain new information (like
>> counter values)? I don't see how it could reliable handle that case.
>
> I can't see any either at the moment :(
>
> BTW, something related to the overflow issue: what I'm currently doing in 
> conntrackd is the following thing, if an overflow is detected, the size of 
> socket buffer is duplicated via RCVBUFFFORCE and then it resyncs. I'll add a 
> clause to limit the socket buffer maximum growth. Can you see any problem 
> with this?

No, that seems reasonable as long as you place an upper limit on the
size.

Re: [PATCH 6/10][CTNETLINK] dump counters iif connection ended or counters filled up

[Amin Azez]

Pablo Neira Ayuso wrote:
> This patch makes ctnetlink to dump counters iif connection reaches the
> destroy state or altenatively if counters filled up.
> 
> AFAICS counters on NEW and UPDATE events doesn't provide interesting
> information, they just consume the limited netlink bandwidth.
> 
> Upcoming conntrackd release in statistics mode uses counters from
> DESTROY events to keep the contability of traffic that the firewall has
> processed.
> 
> I think that this patch should also reset counters upon fill up event,
> comments?

Wouldn't it be better to let them just overflow?

By overflowing instead of resetting the fill-up event is just as
observable but throws away less information

Sam