add TCP protocol state event groups

[CTNETLINK] add TCP protocol state event groups

This patch adds per-protocol state event groups, so one can only listen to a certain TCP state change such as ESTABLISHED. Although such per-state message filtering could be done in userspace, we save CPU cycles since the kernel does not need to build and delivery messages that will be later discarded in userspace. This patch is particularly useful for conntrackd.

Signed-off-by Pablo Neira Ayuso <[email protected]>

net/netfilter/nf_conntrack_netlink.c +7 -2

@@ -317,7 +331,8 @@ static int ctnetlink_conntrack_event(str
 	struct sk_buff *skb;
 	unsigned int type;
 	sk_buff_data_t b;
-	unsigned int flags = 0, group;
+	unsigned int flags = 0, group, proto_group;
+	bool proto_group_has_listener = false;
 
 	/* ignore our fake conntrack entry */
 	if (ct == &nf_conntrack_untracked)

@@ -336,7 +351,11 @@ static int ctnetlink_conntrack_event(str
 	} else
 		return NOTIFY_DONE;
 
-	if (!nfnetlink_has_listeners(group))
+	proto_group = proto_event_group(ct);
+	if (proto_group != NFNLGRP_NONE && nfnetlink_has_listeners(proto_group))
+		proto_group_has_listener = true;
+
+	if (!proto_group_has_listener && !nfnetlink_has_listeners(group))
 		return NOTIFY_DONE;
 
 	skb = alloc_skb(NLMSG_GOODSIZE, GFP_ATOMIC);

Quoted 7 lines · Pablo Neira Ayuso

[CTNETLINK] add TCP protocol state event groups

This patch adds per-protocol state event groups, so one can only listen to a certain TCP state change such as ESTABLISHED. Although such per-state message filtering could be done in userspace, we save CPU cycles since the kernel does not need to build and delivery messages that will be later discarded in userspace. This patch is particularly useful for conntrackd.

I can see that this is useful, but one group per protocol state sounds rather excessive, I would expect that we could group them more logically, maybe "connection setup, teardown and updates"? Which states is conntrackd particulary interested in?

I would also like to hear from Holger whether his conntrack daemon could make use of a mechnism like this too and if the filtering capabilities you propose will do.

Quoted 31 lines

Signed-off-by Pablo Neira Ayuso <[email protected]>

net/netfilter/nf_conntrack_netlink.c +7 -2

@@ -317,7 +331,8 @@ static int ctnetlink_conntrack_event(str
 	struct sk_buff *skb;
 	unsigned int type;
 	sk_buff_data_t b;
-	unsigned int flags = 0, group;
+	unsigned int flags = 0, group, proto_group;
+	bool proto_group_has_listener = false;
 
 	/* ignore our fake conntrack entry */
 	if (ct == &nf_conntrack_untracked)

@@ -336,7 +351,11 @@ static int ctnetlink_conntrack_event(str
 	} else
 		return NOTIFY_DONE;
 
-	if (!nfnetlink_has_listeners(group))
+	proto_group = proto_event_group(ct);
+	if (proto_group != NFNLGRP_NONE && nfnetlink_has_listeners(proto_group))
+		proto_group_has_listener = true;
+
+	if (!proto_group_has_listener && !nfnetlink_has_listeners(group))
 		return NOTIFY_DONE;
 
 	skb = alloc_skb(NLMSG_GOODSIZE, GFP_ATOMIC);

@@ -396,7 +415,11 @@ static int ctnetlink_conntrack_event(str
 	}
 

net/netfilter/nf_conntrack_netlink.c static int ctnetlink_conntrack_event(str · line 420

Quoted 4 lines

net/netfilter/nf_conntrack_netlink.c +2 -0

 	nlh->nlmsg_len = skb->tail - b;
+	if (proto_group_has_listener)
+		atomic_inc(&skb->users);
 	nfnetlink_send(skb, 0, group, 0);

This will always send to the main group even if only the proto group has listeners.

Quoted 10 lines · Patrick McHardy

Quoted 5 lines

This patch adds per-protocol state event groups, so one can only listen to a certain TCP state change such as ESTABLISHED. Although such per-state message filtering could be done in userspace, we save CPU cycles since the kernel does not need to build and delivery messages that will be later discarded in userspace. This patch is particularly useful for conntrackd.

I can see that this is useful, but one group per protocol state sounds rather excessive, I would expect that we could group them more logically, maybe "connection setup, teardown and updates"? Which states is conntrackd particulary interested in?

Well, why just save a couple of groups if we've got 2^32 event groups? Moreover, per protocol state seems to me the most fine-grain and flexible solution. Depending on the replication schema I might be interested in different states.

Quoted 3 lines

I would also like to hear from Holger whether his conntrack daemon could make use of a mechnism like this too and if the filtering capabilities you propose will do.

I'm sure he will benefit of it. Currently there are two main CPU cycle
consumers: event delivery and network transmission, and it is linked to the number of messages generated. Not surprisingly, if we reduce the number of messages generated, we reduce CPU consumption. Sysadmins may enable this tradeoff. BTW, where's Holger's code? :)

I have a paper here on conntrackd that I can't release yet. Would you be interested in reviewing it? In return, you'll see all the work that I've currently done. Do you have some minor spare cycle in your busy agenda? :)

Quoted 33 lines

Quoted 33 lines

Signed-off-by Pablo Neira Ayuso <[email protected]>

net/netfilter/nf_conntrack_netlink.c +7 -2

@@ -317,7 +331,8 @@ static int ctnetlink_conntrack_event(str
 	struct sk_buff *skb;
 	unsigned int type;
 	sk_buff_data_t b;
-	unsigned int flags = 0, group;
+	unsigned int flags = 0, group, proto_group;
+	bool proto_group_has_listener = false;
 
 	/* ignore our fake conntrack entry */
 	if (ct == &nf_conntrack_untracked)

@@ -336,7 +351,11 @@ static int ctnetlink_conntrack_event(str
 	} else
 		return NOTIFY_DONE;
 
-	if (!nfnetlink_has_listeners(group))
+	proto_group = proto_event_group(ct);
+	if (proto_group != NFNLGRP_NONE && nfnetlink_has_listeners(proto_group))
+		proto_group_has_listener = true;
+
+	if (!proto_group_has_listener && !nfnetlink_has_listeners(group))
 		return NOTIFY_DONE;
 
 	skb = alloc_skb(NLMSG_GOODSIZE, GFP_ATOMIC);

@@ -396,7 +415,11 @@ static int ctnetlink_conntrack_event(str
 	}
 

Quoted 7 lines

net/netfilter/nf_conntrack_netlink.c static int ctnetlink_conntrack_event(str · line 420

Quoted 4 lines

net/netfilter/nf_conntrack_netlink.c +2 -0

 	nlh->nlmsg_len = skb->tail - b;
+	if (proto_group_has_listener)
+		atomic_inc(&skb->users);
 	nfnetlink_send(skb, 0, group, 0);

This will always send to the main group even if only the proto group has listeners.

I can improve that. Anyway, AFAIK the main cost here is the message allocation and setup. Since we have already do it for the protocol group, netlink will just notice itself that there's no listeners for that event just a bit later.

Quoted 10 lines · Pablo Neira Ayuso

Quoted 7 lines · Patrick McHardy

Quoted 3 lines

This patch adds per-protocol state event groups, so one can only listen to a certain TCP state change such as ESTABLISHED. Although such per-state message filtering could be done in userspace, we save CPU cycles since the kernel does
not need to build and delivery messages that will be later discarded in userspace. This patch is particularly useful for conntrackd.

I can see that this is useful, but one group per protocol state sounds rather excessive, I would expect that we could group them more logically, maybe "connection setup, teardown and updates"? Which states is conntrackd particulary interested in?

Quoted 4 lines

Well, why just save a couple of groups if we've got 2^32 event groups? Moreover, per protocol state seems to me the most fine-grain and flexible solution. Depending on the replication schema I might be interested in different states.

Its not only about saving groups. A scheme like this only makes sense if you introduce groups for every tiny bit, otherwise you need to subscribe to the "global" group anyway to get the remaining "unclassified" events you're interested in. And that not only uses a lot of groups, it also requires dispatching the same event to potentially many groups. I'm interested, do you already use this feature in conntrackd? If yes, how do you deal with UDP etc. that you didn't introduce new groups for?

Quoted 6 lines · Patrick McHardy

Quoted 4 lines

Well, why just save a couple of groups if we've got 2^32 event groups? Moreover, per protocol state seems to me the most fine-grain and flexible solution. Depending on the replication schema I might be interested in different states.

Quoted 8 lines

Its not only about saving groups. A scheme like this only makes sense if you introduce groups for every tiny bit, otherwise you need to subscribe to the "global" group anyway to get the remaining "unclassified" events you're interested in. And that not only uses a lot of groups, it also requires dispatching the same event to potentially many groups. I'm interested, do you already use this feature in conntrackd? If yes, how do you deal with UDP etc. that you didn't introduce new groups for?

The patch is incomplete but you can guess the schema. Consider a patch to add the following groups:

NFNLGRP_CONNTRACK_PROTO_UDP_NEW,
NFNLGRP_CONNTRACK_PROTO_UDP_UPDATE,
NFNLGRP_CONNTRACK_PROTO_UDP_DESTROY,
NFNLGRP_CONNTRACK_PROTO_ICMP_NEW,
NFNLGRP_CONNTRACK_PROTO_ICMP_UPDATE,
NFNLGRP_CONNTRACK_PROTO_ICMP_DESTROY,
NFNLGRP_CONNTRACK_PROTO_SCTP_...,
NFNLGRP_CONNTRACK_PROTO_UNCLASSIFIED_NEW,
NFNLGRP_CONNTRACK_PROTO_UNCLASSIFIED_UPDATE,
NFNLGRP_CONNTRACK_PROTO_UNCLASSIFIED_DESTROY

At a glance I see the function netlink_update_listeners that is O(n), that could be the bottleneck if we have a lot of groups. Still objections with this approach?

Quoted 7 lines · Pablo Neira Ayuso

Quoted 5 lines · Patrick McHardy

Quoted 4 lines

Well, why just save a couple of groups if we've got 2^32 event groups? Moreover, per protocol state seems to me the most fine-grain and flexible solution. Depending on the replication schema I might be interested in different states.

Quoted 8 lines

Quoted 8 lines

Its not only about saving groups. A scheme like this only makes sense if you introduce groups for every tiny bit, otherwise you need to subscribe to the "global" group anyway to get the remaining "unclassified" events you're interested in. And that not only uses a lot of groups, it also requires dispatching the same event to potentially many groups. I'm interested, do you already use this feature in conntrackd? If yes, how do you deal with UDP etc. that you didn't introduce new groups for?

You skipped my question.

Quoted 14 lines

The patch is incomplete but you can guess the schema. Consider a patch to add the following groups:

NFNLGRP_CONNTRACK_PROTO_UDP_NEW,
NFNLGRP_CONNTRACK_PROTO_UDP_UPDATE,
NFNLGRP_CONNTRACK_PROTO_UDP_DESTROY,
NFNLGRP_CONNTRACK_PROTO_ICMP_NEW,
NFNLGRP_CONNTRACK_PROTO_ICMP_UPDATE,
NFNLGRP_CONNTRACK_PROTO_ICMP_DESTROY,
NFNLGRP_CONNTRACK_PROTO_SCTP_...,
NFNLGRP_CONNTRACK_PROTO_UNCLASSIFIED_NEW,
NFNLGRP_CONNTRACK_PROTO_UNCLASSIFIED_UPDATE,
NFNLGRP_CONNTRACK_PROTO_UNCLASSIFIED_DESTROY

Quoted 3 lines

At a glance I see the function netlink_update_listeners that is O(n), that could be the bottleneck if we have a lot of groups. Still objections with this approach?

So far I presume conntrackd still listens to the global group. So I would like to know how many groups we'll end up with until this scheme is really fine-grained enough that it allows conntrackd to avoid listening to the global group and what types of events it would ignore (so I can understand the performance improvement this might yield). The above still only contains protocol specific groups.

Quoted 7 lines · Patrick McHardy

So far I presume conntrackd still listens to the global group. So I would like to know how many groups we'll end up with until this scheme is really fine-grained enough that it allows conntrackd to avoid listening to the global group and what types of events it would ignore (so I can understand the performance improvement this might yield). The above still only contains protocol specific groups.

OK, let me develop the issue a bit more. In my initial experiments, I used the patch to replicate only TCP connections, so conntrackd was not listening the global group but just TCP state events. I currently have a clause that looks like:

Replicate ESTABLISHED TIME_WAIT for TCP

Thus, only established and time_wait states are replicated. My plan is to add similar clauses for other protocols so:

Replicate ESTABLISHED for STCP

Replicate NEW DESTROY for UDP

Replica NEW DESTROY for UNCLASSIFIED

Quoted 5 lines · Pablo Neira Ayuso

OK, let me develop the issue a bit more. In my initial experiments, I used the patch to replicate only TCP connections, so conntrackd was not listening the global group but just TCP state events. I currently have a clause that looks like:

Quoted 4 lines

Replicate ESTABLISHED TIME_WAIT for TCP

Thus, only established and time_wait states are replicated. My plan is to add similar clauses for other protocols so:

Is that set of states useful in real-life?

How are connections destroyed?

Quoted 1 line

Replicate ESTABLISHED for STCP

What does this exactly mean? Replicate messages that put the connection in established state, or replicate all messages that happen while the connection is in established state?

Quoted 1 line

Replicate NEW DESTROY for UDP

There aren't any other events for UDP anyway except STATUS for the assured bit, and I guess you do want to replicate that as well.

Quoted 1 line

Replica NEW DESTROY for UNCLASSIFIED

Same here, assuming UNCLASSIFIED will be proto generic in the end.

Again, what I would like to understand before we add something we have to live with forever is what it will look like in the end and what the gain of it is, so I would like to see a realistic example. Since the vast majority of updates is in TCP ESTABLISHED state and basically everyone is going to synchronize that I have a hard time believing that this is really going to give a notable performance improvement.

I also only see new groups related to protocol states, what about helpers, timeouts, ...?

Quoted 6 lines · Patrick McHardy

Quoted 4 lines

Replicate ESTABLISHED TIME_WAIT for TCP

Thus, only established and time_wait states are replicated. My plan is to add similar clauses for other protocols so:

Is that set of states useful in real-life?

I think so. In general, syn states have a very short lifetime. Thus, they are quickly superseded by established, making the replication effort barely useful. Moreover, I assume that clients usually reissue connections that fail in (early) syn states. I'm assuming that we are focusing on the recovery of connection established.

Quoted 1 line

How are connections destroyed?

Once we received the time_wait, we tell other replicas to destroy the connection. We miss previous teardown states that have also short lifetime. This is a tradeoff, we reduce the "copy-equivalence" degree among replicas but reduce the number of messages generated in return.

Quoted 7 lines

Again, what I would like to understand before we add something we have to live with forever is what it will look like in the end and what the gain of it is, so I would like to see a realistic example. Since the vast majority of updates is in TCP ESTABLISHED state and basically everyone is going to synchronize that I have a hard time believing that this is really going to give a notable performance improvement.

Well, since there is usually just one messages per state change in the case of TCP connections, we would just generate two, one to notify the established state and one to destroy it. Thus, in the particular case of a HTTP connection, we would need just two messages instead of seven.

Quoted 2 lines

I also only see new groups related to protocol states, what about helpers, timeouts, ...?

Same thing as above, the protocol events groups are just a way to group events. Thus, if the conntrack mark changes while in established state, an event message to the established group is delivered.

Quoted 3 lines

This patch adds per-protocol state event groups, so one can only listen to a certain TCP state change such as ESTABLISHED. Although such per-state message filtering could be done in userspace, we save CPU cycles since the kernel does
not need to build and delivery messages that will be later discarded in userspace. This patch is particularly useful for conntrackd.

I can see that this is useful, but one group per protocol state sounds rather excessive, I would expect that we could group them more logically, maybe "connection setup, teardown and updates"? Which states is conntrackd particulary interested in?

I would also like to hear from Holger whether his conntrack daemon could make use of a mechnism like this too and if the filtering capabilities you propose will do.

Quoted 9 lines · Patrick McHardy

I can see that this is useful, but one group per protocol state sounds rather excessive, I would expect that we could group them more logically, maybe "connection setup, teardown and updates"? Which states is conntrackd particulary interested in?

I would also like to hear from Holger whether his conntrack daemon could make use of a mechnism like this too and if the filtering capabilities you propose will do.

ctsyncd currently does quite some per-protocol filtering, e.g. TCP connections currently get synced only after being in the ESTABLISHED state and deleted on the slave side if the teardown event is received. This greatly reduces the number of events which are send to the ctsyncd peer.

Although ctsyncd does not have a performance problem at all even if I run it against our Spirent performance system I generally like the idea of doing some of the filtering in kernelspace. Though I would leave the userspace filtering code as a fallback solution in place.

As of July I have a new colleague which hopefully takes over some of the workloads I currently have. My plan therefore for ctsyncd is to finally improve filtering capability of ctsyncd and make if configurable, as currently most of the filtering code is still hardcoded. My plan is also to improve the partnership with keepalived et all, basically making this part more configurable too. This version will then be my 1.0 release, as it currently runs fine for serveral months now on our ASG products.

So hopefully I am able to release ctsyncd 1.0 into the public within the next few weeks, especially with Netfilter Workshop in mind.