Transmit mark during connection destruction event

Transmit mark during connection destruction event

[Eric Leblond]

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

Hi,

The following feature was submitted some months ago. It forces the dump
of mark during the connection destruction event. The induced load is
quiet small and the patch is usefull to provide an easy way to filter
event on user side without having to keep an hash in userspace.

This new version is against 2.6.24 git tree.

BR,
-- 
Eric Leblond <eric@inl.fr>
NuFW, Now User Filtering Works : http://www.nufw.org

[-- Attachment #2: 0001-Transmit-mark-during-all-events.patch --]
[-- Type: text/x-diff, Size: 1371 bytes --]

From a222a44fc10d6bd0a6fdb4b0f086d83f6347d529 Mon Sep 17 00:00:00 2001
From: Eric Leblond <eric@inl.fr>
Date: Tue, 29 Jan 2008 00:00:46 +0100
Subject: [PATCH] Transmit mark during all events.


Signed-off-by: Eric Leblond <eric@inl.fr>
---
 net/netfilter/nf_conntrack_netlink.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 7d23124..d6ade4d 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -414,18 +414,18 @@ static int ctnetlink_conntrack_event(struct notifier_block *this,
 		    && ctnetlink_dump_helpinfo(skb, ct) < 0)
 			goto nla_put_failure;
 
-#ifdef CONFIG_NF_CONNTRACK_MARK
-		if ((events & IPCT_MARK || ct->mark)
-		    && ctnetlink_dump_mark(skb, ct) < 0)
-			goto nla_put_failure;
-#endif
-
 		if (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 nla_put_failure;
 	}
 
+#ifdef CONFIG_NF_CONNTRACK_MARK
+	if ((events & IPCT_MARK || ct->mark)
+			&& ctnetlink_dump_mark(skb, ct) < 0)
+		goto nla_put_failure;
+#endif
+
 	nlh->nlmsg_len = skb->tail - b;
 	nfnetlink_send(skb, 0, group, 0);
 	return NOTIFY_DONE;
-- 
1.5.2.5

Re: Transmit mark during connection destruction event

[Patrick McHardy]

Eric Leblond wrote:
> Hi,
> 
> The following feature was submitted some months ago. It forces the dump
> of mark during the connection destruction event. The induced load is
> quiet small and the patch is usefull to provide an easy way to filter
> event on user side without having to keep an hash in userspace.
> 
> This new version is against 2.6.24 git tree.


It clashed with some changes I had queued locally, but I fixed it
up and applied it. Thanks Eric.

Re: Transmit mark during connection destruction event

[Pablo Neira Ayuso]

Patrick McHardy wrote:
> Eric Leblond wrote:
>> The following feature was submitted some months ago. It forces the dump
>> of mark during the connection destruction event. The induced load is
>> quiet small and the patch is usefull to provide an easy way to filter
>> event on user side without having to keep an hash in userspace.
>>
>> This new version is against 2.6.24 git tree.
> 
> It clashed with some changes I had queued locally, but I fixed it
> up and applied it. Thanks Eric.

Please, hold it on. I don't see the point of consuming 8 extra byte in
every extra destroy message. You have tons of resources in userspace to
implement whatever performance structure to store the conntrackd but we
do have limited bandwidth in netlink. Instead we may dump the id but I
don't support this option either.

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

Re: Transmit mark during connection destruction event

[Patrick McHardy]

Pablo Neira Ayuso wrote:
> Patrick McHardy wrote:
>> Eric Leblond wrote:
>>> The following feature was submitted some months ago. It forces the dump
>>> of mark during the connection destruction event. The induced load is
>>> quiet small and the patch is usefull to provide an easy way to filter
>>> event on user side without having to keep an hash in userspace.
>>>
>>> This new version is against 2.6.24 git tree.
>> It clashed with some changes I had queued locally, but I fixed it
>> up and applied it. Thanks Eric.
> 
> Please, hold it on. I don't see the point of consuming 8 extra byte in
> every extra destroy message. You have tons of resources in userspace to
> implement whatever performance structure to store the conntrackd but we
> do have limited bandwidth in netlink. Instead we may dump the id but I
> don't support this option either.

I agree with Eric, its a useful option for avoiding overhead in
userspace, and what counts in the end is the accumulated overhead
of both kernel and userspace. If userspace can avoid dealing with
tuples and complicated bookkeeping it can read messages faster,
thus avoiding recv-queue overflows.

Re: Transmit mark during connection destruction event

[Pablo Neira Ayuso]

Patrick McHardy wrote:
> Pablo Neira Ayuso wrote:
>> Patrick McHardy wrote:
>>> Eric Leblond wrote:
>>>> The following feature was submitted some months ago. It forces the dump
>>>> of mark during the connection destruction event. The induced load is
>>>> quiet small and the patch is usefull to provide an easy way to filter
>>>> event on user side without having to keep an hash in userspace.
>>>>
>>>> This new version is against 2.6.24 git tree.
>>> It clashed with some changes I had queued locally, but I fixed it
>>> up and applied it. Thanks Eric.
>>
>> Please, hold it on. I don't see the point of consuming 8 extra byte in
>> every extra destroy message. You have tons of resources in userspace to
>> implement whatever performance structure to store the conntrackd but we
>> do have limited bandwidth in netlink. Instead we may dump the id but I
>> don't support this option either.
> 
> I agree with Eric, its a useful option for avoiding overhead in
> userspace, and what counts in the end is the accumulated overhead
> of both kernel and userspace. If userspace can avoid dealing with
> tuples and complicated bookkeeping it can read messages faster,
> thus avoiding recv-queue overflows.

Then, dump the id but not the mark if he wants to identify a conntrack.

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

Re: Transmit mark during connection destruction event

[Pablo Neira Ayuso]

Pablo Neira Ayuso wrote:
> Patrick McHardy wrote:
>> Pablo Neira Ayuso wrote:
>>> Patrick McHardy wrote:
>>>> Eric Leblond wrote:
>>>>> The following feature was submitted some months ago. It forces the dump
>>>>> of mark during the connection destruction event. The induced load is
>>>>> quiet small and the patch is usefull to provide an easy way to filter
>>>>> event on user side without having to keep an hash in userspace.
>>>>>
>>>>> This new version is against 2.6.24 git tree.
>>>> It clashed with some changes I had queued locally, but I fixed it
>>>> up and applied it. Thanks Eric.
>>> Please, hold it on. I don't see the point of consuming 8 extra byte in
>>> every extra destroy message. You have tons of resources in userspace to
>>> implement whatever performance structure to store the conntrackd but we
>>> do have limited bandwidth in netlink. Instead we may dump the id but I
>>> don't support this option either.
>> I agree with Eric, its a useful option for avoiding overhead in
>> userspace, and what counts in the end is the accumulated overhead
>> of both kernel and userspace. If userspace can avoid dealing with
>> tuples and complicated bookkeeping it can read messages faster,
>> thus avoiding recv-queue overflows.
> 
> Then, dump the id but not the mark if he wants to identify a conntrack.

BTW, why just dump the id/mark in the destroy message? One may want to
identify the conntrack in new and update messages as well. IMO, this
patch also introduces an inconsistency.

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

Re: Transmit mark during connection destruction event

[Patrick McHardy]

Pablo Neira Ayuso wrote:
> Pablo Neira Ayuso wrote:
>> Patrick McHardy wrote:
>>> Pablo Neira Ayuso wrote:
>>>> Patrick McHardy wrote:
>>>>> Eric Leblond wrote:
>>>>>> The following feature was submitted some months ago. It forces the dump
>>>>>> of mark during the connection destruction event. The induced load is
>>>>>> quiet small and the patch is usefull to provide an easy way to filter
>>>>>> event on user side without having to keep an hash in userspace.
>>>>>>
>>>>>> This new version is against 2.6.24 git tree.
>>>>> It clashed with some changes I had queued locally, but I fixed it
>>>>> up and applied it. Thanks Eric.
>>>> Please, hold it on. I don't see the point of consuming 8 extra byte in
>>>> every extra destroy message. You have tons of resources in userspace to
>>>> implement whatever performance structure to store the conntrackd but we
>>>> do have limited bandwidth in netlink. Instead we may dump the id but I
>>>> don't support this option either.
>>> I agree with Eric, its a useful option for avoiding overhead in
>>> userspace, and what counts in the end is the accumulated overhead
>>> of both kernel and userspace. If userspace can avoid dealing with
>>> tuples and complicated bookkeeping it can read messages faster,
>>> thus avoiding recv-queue overflows.
>> Then, dump the id but not the mark if he wants to identify a conntrack.
> 
> BTW, why just dump the id/mark in the destroy message? One may want to
> identify the conntrack in new and update messages as well. IMO, this
> patch also introduces an inconsistency.


Its sent in all messages now if its non-zero.

Re: Transmit mark during connection destruction event

[Pablo Neira Ayuso]

Pablo Neira Ayuso wrote:
> Pablo Neira Ayuso wrote:
>> Patrick McHardy wrote:
>>> Pablo Neira Ayuso wrote:
>>>> Patrick McHardy wrote:
>>>>> Eric Leblond wrote:
>>>>>> The following feature was submitted some months ago. It forces the dump
>>>>>> of mark during the connection destruction event. The induced load is
>>>>>> quiet small and the patch is usefull to provide an easy way to filter
>>>>>> event on user side without having to keep an hash in userspace.
>>>>>>
>>>>>> This new version is against 2.6.24 git tree.
>>>>> It clashed with some changes I had queued locally, but I fixed it
>>>>> up and applied it. Thanks Eric.
>>>> Please, hold it on. I don't see the point of consuming 8 extra byte in
>>>> every extra destroy message. You have tons of resources in userspace to
>>>> implement whatever performance structure to store the conntrackd but we
>>>> do have limited bandwidth in netlink. Instead we may dump the id but I
>>>> don't support this option either.
>>> I agree with Eric, its a useful option for avoiding overhead in
>>> userspace, and what counts in the end is the accumulated overhead
>>> of both kernel and userspace. If userspace can avoid dealing with
>>> tuples and complicated bookkeeping it can read messages faster,
>>> thus avoiding recv-queue overflows.
>> Then, dump the id but not the mark if he wants to identify a conntrack.
> 
> BTW, why just dump the id/mark in the destroy message? One may want to
> identify the conntrack in new and update messages as well. IMO, this
> patch also introduces an inconsistency.

Never mind. I was confused because of the patch description. It is
supposed to dump mark always.

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

Re: Transmit mark during connection destruction event

[Patrick McHardy]

Pablo Neira Ayuso wrote:
> Patrick McHardy wrote:
>
>> I agree with Eric, its a useful option for avoiding overhead in
>> userspace, and what counts in the end is the accumulated overhead
>> of both kernel and userspace. If userspace can avoid dealing with
>> tuples and complicated bookkeeping it can read messages faster,
>> thus avoiding recv-queue overflows.
> 
> Then, dump the id but not the mark if he wants to identify a conntrack.


That probably won't help since the ID is chosen arbitarily, while the
mark allows you to encode information. I don't see the big problem here,
it only increases the message size if marks are actually used.

I'm also sure you could decrease overhead far more by choosing a proper
allocation size without affecting functionality thats apparently useful
for some people.

Re: Transmit mark during connection destruction event

[Pablo Neira Ayuso]

Patrick McHardy wrote:
> Pablo Neira Ayuso wrote:
>> Patrick McHardy wrote:
>>
>>> I agree with Eric, its a useful option for avoiding overhead in
>>> userspace, and what counts in the end is the accumulated overhead
>>> of both kernel and userspace. If userspace can avoid dealing with
>>> tuples and complicated bookkeeping it can read messages faster,
>>> thus avoiding recv-queue overflows.
>>
>> Then, dump the id but not the mark if he wants to identify a conntrack.
> 
> That probably won't help since the ID is chosen arbitarily, while the
> mark allows you to encode information. I don't see the big problem here,
> it only increases the message size if marks are actually used.

Make sense.

> I'm also sure you could decrease overhead far more by choosing a proper
> allocation size without affecting functionality thats apparently useful
> for some people.

Please, elaborate this a bit.

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

Re: Transmit mark during connection destruction event

[Patrick McHardy]

Pablo Neira Ayuso wrote:
> Patrick McHardy wrote:
>
>> I'm also sure you could decrease overhead far more by choosing a proper
>> allocation size without affecting functionality thats apparently useful
>> for some people.
> 
> Please, elaborate this a bit.


ctnetlink currently allocates skb of NLMSG_GOODSIZE size, which is
4k on most architectures, but the actual messages are far smaller.
When more than 50% of the skb are unused, af_netlink reallocates
the skb, copying the entire data.

Re: Transmit mark during connection destruction event

[Pablo Neira Ayuso]

Eric Leblond wrote:
> The following feature was submitted some months ago. It forces the dump
> of mark during the connection destruction event. The induced load is
> quiet small and the patch is usefull to provide an easy way to filter
> event on user side without having to keep an hash in userspace.

I think that I told you that I'm against this patch. You may use the id
for this, although I'm not a big fun of it. I prefer having the hash
table or whatever structure in userspace, why don't you consider this?
If you keep the conntrack entries in a list and you have tons of them
your performance would be harmed anyway.

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