[PATCH 0/2] [RFC] Extended accounting infrastructure for iptables

[PATCH 0/2] [RFC] Extended accounting infrastructure for iptables

[pablo]

From: Pablo Neira Ayuso <pablo@netfilter.org>

Hi!

We currently have two ways to account traffic in netfilter:

- iptables chain and rule counters:

 # iptables -L -n -v
Chain INPUT (policy DROP 3 packets, 867 bytes)
 pkts bytes target     prot opt in     out     source               destinat
    8  1104 ACCEPT     all  --  lo     *       0.0.0.0/0            0.0.0.0/

- use flow-based accounting provided by ctnetlink:

 # conntrack -L
tcp      6 431999 ESTABLISHED src=192.168.1.130 dst=212.106.219.168 sport=58

While trying to display real-time accounting statistics, we require
to pool the kernel periodically to obtain this information. This is
OK if the number of flows is relatively low. However, in case that
the number of flows is huge, we can spend a considerable amount of
cycles to iterate over the list of flows that have been obtained.

Moreover, if we want to obtain the sum of the flow accounting results
that match some criteria, we have to iterate over the whole list of
existing flows, look for matchings and update the counters.

This patchset adds the extended accounting infrastructure in
kernel-space. It is composed of one nfnetlink interface that
allows you to create, to update and to retrieve accounting objects.
These objects can be used to account traffic with the flexibility
that iptables rules provide (by means of the new NFACCT target).

Quick example of use:

1) You create the accounting object:

libnetfilter_acct/examples# ./nfacct-add http-traffic

2) Add the iptables rules for traffic you want to account:

# iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name http-traffic
# iptables -I OUTPUT -p tcp --dport 80 -j NFACCT --nfacct-name http-traffic

3) Retrieve the counters.

libnetfilter_acct/examples# ./nfacct-get
http-traffic = { pkts = 000000000125,   bytes = 000000023162 };

You can also atomically retrieve and zero counters in case that you
want to collect information periodically.

This comes with the user-space libnetfilter_acct library:

http://1984.lsi.us.es/git/libnetfilter_acct/

[Note: The current library API is still quite preliminary. I plan
to heavily rework it]

And the NFACCT extension for user-space iptables is here:

http://1984.lsi.us.es/git/iptables/commit/?id=850104ff9d619a7e0bc561aa16fee838fcd1937c

Comments welcome!

Pablo Neira Ayuso (2):
  netfilter: add extended accounting infrastructure over nfnetlink
  netfilter: xtables: add NFACCT target to support extended accounting

 include/linux/netfilter/Kbuild           |    2 +
 include/linux/netfilter/nfnetlink.h      |    3 +-
 include/linux/netfilter/nfnetlink_acct.h |   34 +++
 include/linux/netfilter/xt_NFACCT.h      |   17 ++
 net/netfilter/Kconfig                    |   18 ++
 net/netfilter/Makefile                   |    2 +
 net/netfilter/nfnetlink_acct.c           |  352 ++++++++++++++++++++++++++++++
 net/netfilter/xt_NFACCT.c                |   78 +++++++
 8 files changed, 505 insertions(+), 1 deletions(-)
 create mode 100644 include/linux/netfilter/nfnetlink_acct.h
 create mode 100644 include/linux/netfilter/xt_NFACCT.h
 create mode 100644 net/netfilter/nfnetlink_acct.c
 create mode 100644 net/netfilter/xt_NFACCT.c

-- 
1.7.2.5

[PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[pablo]

From: Pablo Neira Ayuso <pablo@netfilter.org>

We currently have two ways to account traffic in netfilter:

- iptables chain and rule counters:

 # iptables -L -n -v
Chain INPUT (policy DROP 3 packets, 867 bytes)
 pkts bytes target     prot opt in     out     source               destination
    8  1104 ACCEPT     all  --  lo     *       0.0.0.0/0            0.0.0.0/0

- use flow-based accounting provided by ctnetlink:

 # conntrack -L
tcp      6 431999 ESTABLISHED src=192.168.1.130 dst=212.106.219.168 sport=58152 dport=80 packets=47 bytes=7654 src=212.106.219.168 dst=192.168.1.130 sport=80 dport=58152 packets=49 bytes=66340 [ASSURED] mark=0 use=1

While trying to display real-time accounting statistics, we require
to pool the kernel periodically to obtain this information. This is
OK if the number of flows is relatively low. However, in case that
the number of flows is huge, we can spend a considerable amount of
cycles to iterate over the list of flows that have been obtained.

Moreover, if we want to obtain the sum of the flow accounting results
that match some criteria, we have to iterate over the whole list of
existing flows, look for matchings and update the counters.

This patch adds the extended accounting infrastructure for
nfnetlink which aims to allow displaying real-time traffic accounting
without the need of complicated and resource-consuming implementation
in user-space. Basically, this new infrastructure allows you to create
accounting objects. One accounting object is composed of packet and
byte counters.

In order to manipulate create accounting objects, you require the
new libnetfilter_acct library. It contains several examples of use:

libnetfilter_acct/examples# ./nfacct-add http-traffic
libnetfilter_acct/examples# ./nfacct-get
http-traffic = { pkts = 000000000000,   bytes = 000000000000 };

Then, you can use one of this accounting objects in several iptables
rules using the new NFACCT target (which comes in a follow-up patch):

 # iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name http-traffic
 # iptables -I OUTPUT -p tcp --dport 80 -j NFACCT --nfacct-name http-traffic

The idea is simple: if one packet matches the rule, the NFACCT target
updates the counters.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/linux/netfilter/Kbuild           |    1 +
 include/linux/netfilter/nfnetlink.h      |    3 +-
 include/linux/netfilter/nfnetlink_acct.h |   34 +++
 net/netfilter/Kconfig                    |    8 +
 net/netfilter/Makefile                   |    1 +
 net/netfilter/nfnetlink_acct.c           |  352 ++++++++++++++++++++++++++++++
 6 files changed, 398 insertions(+), 1 deletions(-)
 create mode 100644 include/linux/netfilter/nfnetlink_acct.h
 create mode 100644 net/netfilter/nfnetlink_acct.c

diff --git a/include/linux/netfilter/Kbuild b/include/linux/netfilter/Kbuild
index a1b410c..8995867 100644
--- a/include/linux/netfilter/Kbuild
+++ b/include/linux/netfilter/Kbuild
@@ -6,6 +6,7 @@ header-y += nf_conntrack_sctp.h
 header-y += nf_conntrack_tcp.h
 header-y += nf_conntrack_tuple_common.h
 header-y += nfnetlink.h
+header-y += nfnetlink_acct.h
 header-y += nfnetlink_compat.h
 header-y += nfnetlink_conntrack.h
 header-y += nfnetlink_log.h
diff --git a/include/linux/netfilter/nfnetlink.h b/include/linux/netfilter/nfnetlink.h
index 74d3386..b64454c 100644
--- a/include/linux/netfilter/nfnetlink.h
+++ b/include/linux/netfilter/nfnetlink.h
@@ -48,7 +48,8 @@ struct nfgenmsg {
 #define NFNL_SUBSYS_ULOG		4
 #define NFNL_SUBSYS_OSF			5
 #define NFNL_SUBSYS_IPSET		6
-#define NFNL_SUBSYS_COUNT		7
+#define NFNL_SUBSYS_ACCT		7
+#define NFNL_SUBSYS_COUNT		8
 
 #ifdef __KERNEL__
 
diff --git a/include/linux/netfilter/nfnetlink_acct.h b/include/linux/netfilter/nfnetlink_acct.h
new file mode 100644
index 0000000..9a1a119
--- /dev/null
+++ b/include/linux/netfilter/nfnetlink_acct.h
@@ -0,0 +1,34 @@
+#ifndef _NFNL_ACCT_H_
+#define _NFNL_ACCT_H_
+#include <linux/netfilter/nfnetlink.h>
+
+#define NFACCT_NAME_MAX		64
+
+enum nfnl_acct_msg_types {
+	NFNL_MSG_ACCT_NEW,
+	NFNL_MSG_ACCT_GET,
+	NFNL_MSG_ACCT_GET_CTRZERO,
+	NFNL_MSG_ACCT_DEL,
+	NFNL_MSG_ACCT_MAX
+};
+
+enum nfnl_acct_type {
+	NFACCT_UNSPEC,
+	NFACCT_NAME,
+	NFACCT_PKTS,
+	NFACCT_BYTES,
+	__NFACCT_MAX
+};
+#define NFACCT_MAX (__NFACCT_MAX - 1)
+
+#ifdef __KERNEL__
+
+struct nf_acct;
+
+extern struct nf_acct *nfnl_acct_find_get(const char *filter_name);
+extern void nfnl_acct_put(struct nf_acct *acct);
+extern void nfnl_acct_update(const struct sk_buff *skb, struct nf_acct *nfacct);
+
+#endif /* __KERNEL__ */
+
+#endif /* _NFNL_ACCT_H */
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index d5597b7..77326ac 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -4,6 +4,14 @@ menu "Core Netfilter Configuration"
 config NETFILTER_NETLINK
 	tristate
 
+config NETFILTER_NETLINK_ACCT
+tristate "Netfilter NFACCT over NFNETLINK interface"
+	depends on NETFILTER_ADVANCED
+	select NETFILTER_NETLINK
+	help
+	  If this option is enabled, the kernel will include support
+	  for extended accounting via NFNETLINK.
+
 config NETFILTER_NETLINK_QUEUE
 	tristate "Netfilter NFQUEUE over NFNETLINK interface"
 	depends on NETFILTER_ADVANCED
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 1a02853..4da1c87 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -7,6 +7,7 @@ nf_conntrack-$(CONFIG_NF_CONNTRACK_EVENTS) += nf_conntrack_ecache.o
 obj-$(CONFIG_NETFILTER) = netfilter.o
 
 obj-$(CONFIG_NETFILTER_NETLINK) += nfnetlink.o
+obj-$(CONFIG_NETFILTER_NETLINK_ACCT) += nfnetlink_acct.o
 obj-$(CONFIG_NETFILTER_NETLINK_QUEUE) += nfnetlink_queue.o
 obj-$(CONFIG_NETFILTER_NETLINK_LOG) += nfnetlink_log.o
 
diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
new file mode 100644
index 0000000..3ec407f
--- /dev/null
+++ b/net/netfilter/nfnetlink_acct.c
@@ -0,0 +1,352 @@
+/*
+ * (C) 2011 Pablo Neira Ayuso <pablo@netfilter.org>
+ * (C) 2011 Intra2net AG <http://www.intra2net.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation (or any later at your option).
+ */
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/skbuff.h>
+#include <linux/netlink.h>
+#include <linux/rculist.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <net/netlink.h>
+#include <net/sock.h>
+#include <asm/atomic.h>
+
+#include <linux/netfilter.h>
+#include <linux/netfilter/nfnetlink.h>
+#include <linux/netfilter/nfnetlink_acct.h>
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Pablo Neira Ayuso <pablo@netfilter.org>");
+MODULE_DESCRIPTION("nfacct: Extended Netfilter accounting infrastructure");
+
+static LIST_HEAD(nfnl_acct_list);
+
+struct nf_acct {
+	struct rcu_head		rcu_head;
+	struct list_head	head;
+	spinlock_t		lock;	/* to update the counters. */
+	atomic_t		refcnt;
+
+	char			name[NFACCT_NAME_MAX];
+	__u64			pkts;
+	__u64			bytes;
+};
+
+static void nfnl_acct_free_rcu(struct rcu_head *rcu_head)
+{
+	struct nf_acct *acct = container_of(rcu_head, struct nf_acct, rcu_head);
+
+	kfree(acct);
+}
+
+static int
+nfnl_acct_new(struct sock *nfnl, struct sk_buff *skb,
+	     const struct nlmsghdr *nlh, const struct nlattr * const tb[])
+{
+	int ret;
+	struct nf_acct *acct, *matching = NULL;
+	char *acct_name;
+
+	if (!tb[NFACCT_NAME])
+		return -EINVAL;
+
+	acct_name = nla_data(tb[NFACCT_NAME]);
+
+	rcu_read_lock();
+	list_for_each_entry(acct, &nfnl_acct_list, head) {
+		if (strncmp(acct->name, acct_name, NFACCT_NAME_MAX) != 0)
+			continue;
+
+                if (nlh->nlmsg_flags & NLM_F_EXCL) {
+			rcu_read_unlock();
+			ret = -EEXIST;
+			goto err;
+		}
+		matching = acct;
+        }
+	rcu_read_unlock();
+
+	acct = kzalloc(sizeof(struct nf_acct), GFP_KERNEL);
+	if (acct == NULL) {
+		ret = -ENOMEM;
+		goto err;
+	}
+	spin_lock_init(&acct->lock);
+	strncpy(acct->name, nla_data(tb[NFACCT_NAME]), NFACCT_NAME_MAX);
+	if (tb[NFACCT_BYTES])
+		acct->bytes = be64_to_cpu(nla_get_u64(tb[NFACCT_BYTES]));
+	if (tb[NFACCT_PKTS])
+		acct->pkts = be64_to_cpu(nla_get_u64(tb[NFACCT_PKTS]));
+
+	atomic_inc(&acct->refcnt);
+
+	/* We are protected by nfnl mutex. */
+	if (matching) {
+		list_del_rcu(&matching->head);
+		if (atomic_dec_and_test(&matching->refcnt))
+			call_rcu(&matching->rcu_head, nfnl_acct_free_rcu);
+
+	}
+	list_add_tail_rcu(&acct->head, &nfnl_acct_list);
+	return 0;
+err:
+	return ret;
+}
+
+static int
+nfnl_acct_fill_info(struct sk_buff *skb, u32 pid, u32 seq,
+		   int event, struct nf_acct *acct)
+{
+	struct nlmsghdr *nlh;
+	struct nfgenmsg *nfmsg;
+	unsigned int flags = pid ? NLM_F_MULTI : 0;
+
+	event |= NFNL_SUBSYS_ACCT << 8;
+	nlh = nlmsg_put(skb, pid, seq, event, sizeof(*nfmsg), flags);
+	if (nlh == NULL)
+		goto nlmsg_failure;
+
+	nfmsg = nlmsg_data(nlh);
+	nfmsg->nfgen_family = AF_UNSPEC;
+	nfmsg->version = NFNETLINK_V0;
+	nfmsg->res_id = 0;
+
+	NLA_PUT_STRING(skb, NFACCT_NAME, acct->name);
+	NLA_PUT_BE64(skb, NFACCT_PKTS, cpu_to_be64(acct->pkts));
+	NLA_PUT_BE64(skb, NFACCT_BYTES, cpu_to_be64(acct->bytes));
+
+	nlmsg_end(skb, nlh);
+	return skb->len;
+
+nlmsg_failure:
+nla_put_failure:
+	nlmsg_cancel(skb, nlh);
+	return -1;
+}
+
+static int
+nfnl_acct_dump(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	struct nf_acct *cur, *last;
+
+	if (cb->args[2])
+		return 0;
+
+	last = (struct nf_acct *)cb->args[1];
+	if (cb->args[1])
+		cb->args[1] = 0;
+
+	rcu_read_lock();
+	list_for_each_entry(cur, &nfnl_acct_list, head) {
+		if (last && cur != last)
+			continue;
+
+		if (nfnl_acct_fill_info(skb, NETLINK_CB(cb->skb).pid,
+				       cb->nlh->nlmsg_seq,
+				       NFNL_MSG_ACCT_NEW, cur) < 0) {
+			cb->args[1] = (unsigned long)cur;
+			break;
+		}
+
+		if (NFNL_MSG_TYPE(cb->nlh->nlmsg_type) ==
+						NFNL_MSG_ACCT_GET_CTRZERO) {
+			spin_lock_bh(&cur->lock);
+			cur->pkts = 0;
+			cur->bytes = 0;
+			spin_unlock_bh(&cur->lock);
+		}
+	}
+	if (!cb->args[1])
+		cb->args[2] = 1;
+	rcu_read_unlock();
+	return skb->len;
+}
+
+static int
+nfnl_acct_get(struct sock *nfnl, struct sk_buff *skb,
+	     const struct nlmsghdr *nlh, const struct nlattr * const tb[])
+{
+	int ret = 0;
+	struct nf_acct *cur;
+	char *acct_name;
+
+	if (nlh->nlmsg_flags & NLM_F_DUMP) {
+		return netlink_dump_start(nfnl, skb, nlh, nfnl_acct_dump,
+					  NULL, 0);
+	}
+
+	if (!tb[NFACCT_NAME])
+		return -EINVAL;
+	acct_name = nla_data(tb[NFACCT_NAME]);
+
+	rcu_read_lock();
+	list_for_each_entry(cur, &nfnl_acct_list, head) {
+		struct sk_buff *skb2;
+
+		if (strncmp(cur->name, acct_name, NFACCT_NAME_MAX)!= 0)
+			continue;
+
+		skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+		if (skb2 == NULL)
+			break;
+
+		ret = nfnl_acct_fill_info(skb2, NETLINK_CB(skb).pid,
+					 nlh->nlmsg_seq,
+					 NFNL_MSG_ACCT_NEW, cur);
+		if (ret <= 0)
+			kfree_skb(skb2);
+
+		if (NFNL_MSG_TYPE(nlh->nlmsg_type) ==
+						NFNL_MSG_ACCT_GET_CTRZERO) {
+			spin_lock_bh(&cur->lock);
+			cur->pkts = 0;
+			cur->bytes = 0;
+			spin_unlock_bh(&cur->lock);
+		}
+		break;
+	}
+	rcu_read_unlock();
+	return ret;
+}
+
+static int
+nfnl_acct_del(struct sock *nfnl, struct sk_buff *skb,
+	     const struct nlmsghdr *nlh, const struct nlattr * const tb[])
+{
+	char *acct_name;
+	struct nf_acct *cur;
+	int ret = -ENOENT;
+
+	if (!tb[NFACCT_NAME]) {
+		rcu_read_lock();
+		list_for_each_entry(cur, &nfnl_acct_list, head) {
+			/* We are protected by nfnl mutex. */
+			list_del_rcu(&cur->head);
+			if (atomic_dec_and_test(&cur->refcnt))
+				call_rcu(&cur->rcu_head, nfnl_acct_free_rcu);
+		}
+		rcu_read_lock();
+		return 0;
+	}
+	acct_name = nla_data(tb[NFACCT_NAME]);
+
+	rcu_read_lock();
+	list_for_each_entry(cur, &nfnl_acct_list, head) {
+		if (strncmp(cur->name, acct_name, NFACCT_NAME_MAX) != 0)
+			continue;
+
+		/* We are protected by nfnl mutex. */
+		list_del_rcu(&cur->head);
+		if (atomic_dec_and_test(&cur->refcnt))
+			call_rcu(&cur->rcu_head, nfnl_acct_free_rcu);
+		ret = 0;
+		break;
+	}
+	rcu_read_lock();
+	return ret;
+}
+
+static const struct nla_policy nfnl_acct_policy[NFACCT_MAX+1] = {
+	[NFACCT_NAME] = { .type = NLA_NUL_STRING, .len = NFACCT_NAME_MAX-1 },
+	[NFACCT_BYTES] = { .type = NLA_U64 },
+	[NFACCT_PKTS] = { .type = NLA_U64 },
+};
+
+static const struct nfnl_callback nfnl_acct_cb[NFNL_MSG_ACCT_MAX] = {
+	[NFNL_MSG_ACCT_NEW]		= { .call = nfnl_acct_new,
+					    .attr_count = NFACCT_MAX,
+					    .policy = nfnl_acct_policy },
+	[NFNL_MSG_ACCT_GET] 		= { .call = nfnl_acct_get,
+					    .attr_count = NFACCT_MAX,
+					    .policy = nfnl_acct_policy },
+	[NFNL_MSG_ACCT_GET_CTRZERO] 	= { .call = nfnl_acct_get,
+					    .attr_count = NFACCT_MAX,
+					    .policy = nfnl_acct_policy },
+	[NFNL_MSG_ACCT_DEL]		= { .call = nfnl_acct_del,
+					    .attr_count = NFACCT_MAX,
+					    .policy = nfnl_acct_policy },
+};
+
+static const struct nfnetlink_subsystem nfnl_acct_subsys = {
+	.name				= "acct",
+	.subsys_id			= NFNL_SUBSYS_ACCT,
+	.cb_count			= NFNL_MSG_ACCT_MAX,
+	.cb				= nfnl_acct_cb,
+};
+
+MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_ACCT);
+
+struct nf_acct *nfnl_acct_find_get(const char *acct_name)
+{
+	struct nf_acct *cur, *acct = NULL;
+
+	rcu_read_lock();
+	list_for_each_entry(cur, &nfnl_acct_list, head) {
+		if (strncmp(cur->name, acct_name, NFACCT_NAME_MAX)!= 0)
+			continue;
+
+		acct = cur;
+		atomic_inc(&acct->refcnt);
+		break;
+	}
+	rcu_read_unlock();
+	return acct;
+}
+EXPORT_SYMBOL_GPL(nfnl_acct_find_get);
+
+void nfnl_acct_put(struct nf_acct *acct)
+{
+	if (atomic_dec_and_test(&acct->refcnt))
+		call_rcu(&acct->rcu_head, nfnl_acct_free_rcu);
+}
+EXPORT_SYMBOL_GPL(nfnl_acct_put);
+
+void nfnl_acct_update(const struct sk_buff *skb, struct nf_acct *nfacct)
+{
+	spin_lock_bh(&nfacct->lock);
+	nfacct->pkts++;
+	nfacct->bytes += skb->len;
+	spin_unlock_bh(&nfacct->lock);
+}
+EXPORT_SYMBOL_GPL(nfnl_acct_update);
+
+static int __init nfnl_acct_init(void)
+{
+	int ret;
+
+	pr_info("nfnl_acct: registering with nfnetlink.\n");
+	ret = nfnetlink_subsys_register(&nfnl_acct_subsys);
+	if (ret < 0) {
+		pr_err("nfnl_acct_init: cannot register with nfnetlink.\n");
+		goto err_out;
+	}
+	return 0;
+err_out:
+	return ret;
+}
+
+static void __exit nfnl_acct_exit(void)
+{
+	struct nf_acct *cur, *tmp;
+
+	pr_info("nfnl_acct: unregistering from nfnetlink.\n");
+	nfnetlink_subsys_unregister(&nfnl_acct_subsys);
+
+	/* if we can remove this module, it means that it has no clients. */
+	list_for_each_entry_safe(cur, tmp, &nfnl_acct_list, head) {
+		list_del_rcu(&cur->head);
+		if (atomic_dec_and_test(&cur->refcnt))
+			kfree(cur);
+	}
+}
+
+module_init(nfnl_acct_init);
+module_exit(nfnl_acct_exit);
-- 
1.7.2.5

[PATCH 2/2] netfilter: xtables: add NFACCT target to support extended accounting

[pablo]

From: Pablo Neira Ayuso <pablo@netfilter.org>

This patch adds the target that allows to perform extended
accounting. It requires the new nfnetlink_acct infrastructure.

 # iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name http-traffic
 # iptables -I OUTPUT -p tcp --dport 80 -j NFACCT --nfacct-name http-traffic

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/linux/netfilter/Kbuild      |    1 +
 include/linux/netfilter/xt_NFACCT.h |   17 ++++++++
 net/netfilter/Kconfig               |   10 ++++
 net/netfilter/Makefile              |    1 +
 net/netfilter/xt_NFACCT.c           |   78 +++++++++++++++++++++++++++++++++++
 5 files changed, 107 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/netfilter/xt_NFACCT.h
 create mode 100644 net/netfilter/xt_NFACCT.c

diff --git a/include/linux/netfilter/Kbuild b/include/linux/netfilter/Kbuild
index 8995867..16473ff 100644
--- a/include/linux/netfilter/Kbuild
+++ b/include/linux/netfilter/Kbuild
@@ -22,6 +22,7 @@ header-y += xt_DSCP.h
 header-y += xt_IDLETIMER.h
 header-y += xt_LED.h
 header-y += xt_MARK.h
+header-y += xt_NFACCT.h
 header-y += xt_NFLOG.h
 header-y += xt_NFQUEUE.h
 header-y += xt_RATEEST.h
diff --git a/include/linux/netfilter/xt_NFACCT.h b/include/linux/netfilter/xt_NFACCT.h
new file mode 100644
index 0000000..63a2d55
--- /dev/null
+++ b/include/linux/netfilter/xt_NFACCT.h
@@ -0,0 +1,17 @@
+#ifndef _XT_NFACCT_TARGET_H
+#define _XT_NFACCT_TARGET_H
+
+#include <linux/types.h>
+
+#ifndef NFACCT_NAME_MAX
+#define NFACCT_NAME_MAX 64
+#endif
+
+struct nf_acct;
+
+struct xt_nfacct_target_info {
+	char		name[NFACCT_NAME_MAX];
+	struct nf_acct	*nfacct;
+};
+
+#endif /* _XT_NFACCT_TARGET_H */
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 77326ac..8dfb5ac 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -523,6 +523,16 @@ config NETFILTER_XT_TARGET_MARK
 	(e.g. when running oldconfig). It selects
 	CONFIG_NETFILTER_XT_MARK (combined mark/MARK module).
 
+config NETFILTER_XT_TARGET_NFACCT
+	tristate '"NFACCT" target support'
+	default m if NETFILTER_ADVANCED=n
+	select NETFILTER_NETLINK_ACCT
+	help
+	  This option enables the NFACCT target, which allows extended
+	  accounting through nfnetlink_acct.
+
+	  To compile it as a module, choose M here.  If unsure, say N.
+
 config NETFILTER_XT_TARGET_NFLOG
 	tristate '"NFLOG" target support'
 	default m if NETFILTER_ADVANCED=n
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 4da1c87..af11b81 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -58,6 +58,7 @@ obj-$(CONFIG_NETFILTER_XT_TARGET_CT) += xt_CT.o
 obj-$(CONFIG_NETFILTER_XT_TARGET_DSCP) += xt_DSCP.o
 obj-$(CONFIG_NETFILTER_XT_TARGET_HL) += xt_HL.o
 obj-$(CONFIG_NETFILTER_XT_TARGET_LED) += xt_LED.o
+obj-$(CONFIG_NETFILTER_XT_TARGET_NFACCT) += xt_NFACCT.o
 obj-$(CONFIG_NETFILTER_XT_TARGET_NFLOG) += xt_NFLOG.o
 obj-$(CONFIG_NETFILTER_XT_TARGET_NFQUEUE) += xt_NFQUEUE.o
 obj-$(CONFIG_NETFILTER_XT_TARGET_NOTRACK) += xt_NOTRACK.o
diff --git a/net/netfilter/xt_NFACCT.c b/net/netfilter/xt_NFACCT.c
new file mode 100644
index 0000000..7d26c5f
--- /dev/null
+++ b/net/netfilter/xt_NFACCT.c
@@ -0,0 +1,78 @@
+/*
+ * (C) 2011 Pablo Neira Ayuso <pablo@netfilter.org>
+ * (C) 2011 Intra2net AG <http://www.intra2net.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 (or any
+ * later at your option) as published by the Free Software Foundation.
+ */
+#include <linux/module.h>
+#include <linux/skbuff.h>
+
+#include <linux/netfilter/x_tables.h>
+#include <linux/netfilter/nfnetlink_acct.h>
+#include <linux/netfilter/xt_NFACCT.h>
+
+MODULE_AUTHOR("Pablo Neira Ayuso <pablo@netfilter.org>");
+MODULE_DESCRIPTION("Xtables: area-based accouting target");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("ipt_NFACCT");
+MODULE_ALIAS("ip6t_NFACCT");
+
+static unsigned int
+nfacct_tg(struct sk_buff *skb, const struct xt_action_param *par)
+{
+	const struct xt_nfacct_target_info *info = par->targinfo;
+
+	/* This update function internally uses one spin lock. */
+	nfnl_acct_update(skb, info->nfacct);
+
+	return XT_CONTINUE;
+}
+
+static int
+nfacct_tg_checkentry(const struct xt_tgchk_param *par)
+{
+	struct xt_nfacct_target_info *info = par->targinfo;
+	struct nf_acct *nfacct;
+
+	nfacct = nfnl_acct_find_get(info->name);
+	if (nfacct == NULL) {
+		pr_info("accounting area with name `%s' does not exists\n",
+			info->name);
+		return -ENOENT;
+	}
+	info->nfacct = nfacct;
+	return 0;
+}
+
+static void
+nfacct_tg_destroy(const struct xt_tgdtor_param *par)
+{
+	const struct xt_nfacct_target_info *info = par->targinfo;
+
+	nfnl_acct_put(info->nfacct);
+}
+
+static struct xt_target nfacct_tg_reg __read_mostly = {
+	.name       = "NFACCT",
+	.family     = NFPROTO_UNSPEC,
+	.checkentry = nfacct_tg_checkentry,
+	.target     = nfacct_tg,
+	.destroy    = nfacct_tg_destroy,
+	.targetsize = sizeof(struct xt_nfacct_target_info),
+	.me         = THIS_MODULE,
+};
+
+static int __init nfacct_tg_init(void)
+{
+	return xt_register_target(&nfacct_tg_reg);
+}
+
+static void __exit nfacct_tg_exit(void)
+{
+	xt_unregister_target(&nfacct_tg_reg);
+}
+
+module_init(nfacct_tg_init);
+module_exit(nfacct_tg_exit);
-- 
1.7.2.5

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Eric Dumazet]

Le mercredi 14 décembre 2011 à 12:00 +0100, pablo@netfilter.org a
écrit :
> From: Pablo Neira Ayuso <pablo@netfilter.org>
> 
> We currently have two ways to account traffic in netfilter:
> 
> - iptables chain and rule counters:
> 
>  # iptables -L -n -v
> Chain INPUT (policy DROP 3 packets, 867 bytes)
>  pkts bytes target     prot opt in     out     source               destination
>     8  1104 ACCEPT     all  --  lo     *       0.0.0.0/0            0.0.0.0/0
> 
> - use flow-based accounting provided by ctnetlink:
> 
>  # conntrack -L
> tcp      6 431999 ESTABLISHED src=192.168.1.130 dst=212.106.219.168 sport=58152 dport=80 packets=47 bytes=7654 src=212.106.219.168 dst=192.168.1.130 sport=80 dport=58152 packets=49 bytes=66340 [ASSURED] mark=0 use=1
> 
> While trying to display real-time accounting statistics, we require
> to pool the kernel periodically to obtain this information. This is
> OK if the number of flows is relatively low. However, in case that
> the number of flows is huge, we can spend a considerable amount of
> cycles to iterate over the list of flows that have been obtained.
> 
> Moreover, if we want to obtain the sum of the flow accounting results
> that match some criteria, we have to iterate over the whole list of
> existing flows, look for matchings and update the counters.
> 
> This patch adds the extended accounting infrastructure for
> nfnetlink which aims to allow displaying real-time traffic accounting
> without the need of complicated and resource-consuming implementation
> in user-space. Basically, this new infrastructure allows you to create
> accounting objects. One accounting object is composed of packet and
> byte counters.
> 
> In order to manipulate create accounting objects, you require the
> new libnetfilter_acct library. It contains several examples of use:
> 
> libnetfilter_acct/examples# ./nfacct-add http-traffic
> libnetfilter_acct/examples# ./nfacct-get
> http-traffic = { pkts = 000000000000,   bytes = 000000000000 };
> 
> Then, you can use one of this accounting objects in several iptables
> rules using the new NFACCT target (which comes in a follow-up patch):
> 
>  # iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name http-traffic
>  # iptables -I OUTPUT -p tcp --dport 80 -j NFACCT --nfacct-name http-traffic
> 
> The idea is simple: if one packet matches the rule, the NFACCT target
> updates the counters.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  include/linux/netfilter/Kbuild           |    1 +
>  include/linux/netfilter/nfnetlink.h      |    3 +-
>  include/linux/netfilter/nfnetlink_acct.h |   34 +++
>  net/netfilter/Kconfig                    |    8 +
>  net/netfilter/Makefile                   |    1 +
>  net/netfilter/nfnetlink_acct.c           |  352 ++++++++++++++++++++++++++++++
>  6 files changed, 398 insertions(+), 1 deletions(-)
>  create mode 100644 include/linux/netfilter/nfnetlink_acct.h
>  create mode 100644 net/netfilter/nfnetlink_acct.c
> 
> diff --git a/include/linux/netfilter/Kbuild b/include/linux/netfilter/Kbuild
> index a1b410c..8995867 100644
> --- a/include/linux/netfilter/Kbuild
> +++ b/include/linux/netfilter/Kbuild
> @@ -6,6 +6,7 @@ header-y += nf_conntrack_sctp.h
>  header-y += nf_conntrack_tcp.h
>  header-y += nf_conntrack_tuple_common.h
>  header-y += nfnetlink.h
> +header-y += nfnetlink_acct.h
>  header-y += nfnetlink_compat.h
>  header-y += nfnetlink_conntrack.h
>  header-y += nfnetlink_log.h
> diff --git a/include/linux/netfilter/nfnetlink.h b/include/linux/netfilter/nfnetlink.h
> index 74d3386..b64454c 100644
> --- a/include/linux/netfilter/nfnetlink.h
> +++ b/include/linux/netfilter/nfnetlink.h
> @@ -48,7 +48,8 @@ struct nfgenmsg {
>  #define NFNL_SUBSYS_ULOG		4
>  #define NFNL_SUBSYS_OSF			5
>  #define NFNL_SUBSYS_IPSET		6
> -#define NFNL_SUBSYS_COUNT		7
> +#define NFNL_SUBSYS_ACCT		7
> +#define NFNL_SUBSYS_COUNT		8
>  
>  #ifdef __KERNEL__
>  
> diff --git a/include/linux/netfilter/nfnetlink_acct.h b/include/linux/netfilter/nfnetlink_acct.h
> new file mode 100644
> index 0000000..9a1a119
> --- /dev/null
> +++ b/include/linux/netfilter/nfnetlink_acct.h
> @@ -0,0 +1,34 @@
> +#ifndef _NFNL_ACCT_H_
> +#define _NFNL_ACCT_H_
> +#include <linux/netfilter/nfnetlink.h>
> +
> +#define NFACCT_NAME_MAX		64
> +
> +enum nfnl_acct_msg_types {
> +	NFNL_MSG_ACCT_NEW,
> +	NFNL_MSG_ACCT_GET,
> +	NFNL_MSG_ACCT_GET_CTRZERO,
> +	NFNL_MSG_ACCT_DEL,
> +	NFNL_MSG_ACCT_MAX
> +};
> +
> +enum nfnl_acct_type {
> +	NFACCT_UNSPEC,
> +	NFACCT_NAME,
> +	NFACCT_PKTS,
> +	NFACCT_BYTES,
> +	__NFACCT_MAX
> +};
> +#define NFACCT_MAX (__NFACCT_MAX - 1)
> +
> +#ifdef __KERNEL__
> +
> +struct nf_acct;
> +
> +extern struct nf_acct *nfnl_acct_find_get(const char *filter_name);
> +extern void nfnl_acct_put(struct nf_acct *acct);
> +extern void nfnl_acct_update(const struct sk_buff *skb, struct nf_acct *nfacct);
> +
> +#endif /* __KERNEL__ */
> +
> +#endif /* _NFNL_ACCT_H */
> diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
> index d5597b7..77326ac 100644
> --- a/net/netfilter/Kconfig
> +++ b/net/netfilter/Kconfig
> @@ -4,6 +4,14 @@ menu "Core Netfilter Configuration"
>  config NETFILTER_NETLINK
>  	tristate
>  
> +config NETFILTER_NETLINK_ACCT
> +tristate "Netfilter NFACCT over NFNETLINK interface"
> +	depends on NETFILTER_ADVANCED
> +	select NETFILTER_NETLINK
> +	help
> +	  If this option is enabled, the kernel will include support
> +	  for extended accounting via NFNETLINK.
> +
>  config NETFILTER_NETLINK_QUEUE
>  	tristate "Netfilter NFQUEUE over NFNETLINK interface"
>  	depends on NETFILTER_ADVANCED
> diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
> index 1a02853..4da1c87 100644
> --- a/net/netfilter/Makefile
> +++ b/net/netfilter/Makefile
> @@ -7,6 +7,7 @@ nf_conntrack-$(CONFIG_NF_CONNTRACK_EVENTS) += nf_conntrack_ecache.o
>  obj-$(CONFIG_NETFILTER) = netfilter.o
>  
>  obj-$(CONFIG_NETFILTER_NETLINK) += nfnetlink.o
> +obj-$(CONFIG_NETFILTER_NETLINK_ACCT) += nfnetlink_acct.o
>  obj-$(CONFIG_NETFILTER_NETLINK_QUEUE) += nfnetlink_queue.o
>  obj-$(CONFIG_NETFILTER_NETLINK_LOG) += nfnetlink_log.o
>  
> diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
> new file mode 100644
> index 0000000..3ec407f
> --- /dev/null
> +++ b/net/netfilter/nfnetlink_acct.c
> @@ -0,0 +1,352 @@
> +/*
> + * (C) 2011 Pablo Neira Ayuso <pablo@netfilter.org>
> + * (C) 2011 Intra2net AG <http://www.intra2net.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation (or any later at your option).
> + */
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/skbuff.h>
> +#include <linux/netlink.h>
> +#include <linux/rculist.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/errno.h>
> +#include <net/netlink.h>
> +#include <net/sock.h>
> +#include <asm/atomic.h>
> +
> +#include <linux/netfilter.h>
> +#include <linux/netfilter/nfnetlink.h>
> +#include <linux/netfilter/nfnetlink_acct.h>
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Pablo Neira Ayuso <pablo@netfilter.org>");
> +MODULE_DESCRIPTION("nfacct: Extended Netfilter accounting infrastructure");
> +
> +static LIST_HEAD(nfnl_acct_list);
> +
> +struct nf_acct {
> +	struct rcu_head		rcu_head;
> +	struct list_head	head;
> +	spinlock_t		lock;	/* to update the counters. */
> +	atomic_t		refcnt;
> +
> +	char			name[NFACCT_NAME_MAX];
> +	__u64			pkts;
> +	__u64			bytes;

atomic64_t ?

This would remove use of spinlock in fast path

Also, you put lock and pkts,bytes in different cache lines :(



--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Patrick McHardy]

On 12/14/2011 12:00 PM, pablo@netfilter.org wrote:
>
> In order to manipulate create accounting objects, you require the
> new libnetfilter_acct library. It contains several examples of use:
>
> libnetfilter_acct/examples# ./nfacct-add http-traffic
> libnetfilter_acct/examples# ./nfacct-get
> http-traffic = { pkts = 000000000000,   bytes = 000000000000 };
>
> Then, you can use one of this accounting objects in several iptables
> rules using the new NFACCT target (which comes in a follow-up patch):
>
>   # iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name http-traffic
>   # iptables -I OUTPUT -p tcp --dport 80 -j NFACCT --nfacct-name http-traffic
>
> The idea is simple: if one packet matches the rule, the NFACCT target
> updates the counters.

I like the idea.
>
> diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
> new file mode 100644
> index 0000000..3ec407f
> --- /dev/null
> +++ b/net/netfilter/nfnetlink_acct.c
> @@ -0,0 +1,352 @@
> +/*
> + * (C) 2011 Pablo Neira Ayuso<pablo@netfilter.org>
> + * (C) 2011 Intra2net AG<http://www.intra2net.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation (or any later at your option).
> + */
> +#include<linux/init.h>
> +#include<linux/module.h>
> +#include<linux/kernel.h>
> +#include<linux/skbuff.h>
> +#include<linux/netlink.h>
> +#include<linux/rculist.h>
> +#include<linux/slab.h>
> +#include<linux/types.h>
> +#include<linux/errno.h>
> +#include<net/netlink.h>
> +#include<net/sock.h>
> +#include<asm/atomic.h>
> +
> +#include<linux/netfilter.h>
> +#include<linux/netfilter/nfnetlink.h>
> +#include<linux/netfilter/nfnetlink_acct.h>
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Pablo Neira Ayuso<pablo@netfilter.org>");
> +MODULE_DESCRIPTION("nfacct: Extended Netfilter accounting infrastructure");
> +
> +static LIST_HEAD(nfnl_acct_list);
> +
> +struct nf_acct {
> +	struct rcu_head		rcu_head;
> +	struct list_head	head;
> +	spinlock_t		lock;	/* to update the counters. */
> +	atomic_t		refcnt;
> +
> +	char			name[NFACCT_NAME_MAX];
> +	__u64			pkts;
> +	__u64			bytes;
> +};
> +
> +static void nfnl_acct_free_rcu(struct rcu_head *rcu_head)
> +{
> +	struct nf_acct *acct = container_of(rcu_head, struct nf_acct, rcu_head);
> +
> +	kfree(acct);
> +}
> +
> +static int
> +nfnl_acct_new(struct sock *nfnl, struct sk_buff *skb,
> +	     const struct nlmsghdr *nlh, const struct nlattr * const tb[])
> +{
> +	int ret;
> +	struct nf_acct *acct, *matching = NULL;
> +	char *acct_name;
> +
> +	if (!tb[NFACCT_NAME])
> +		return -EINVAL;
> +
> +	acct_name = nla_data(tb[NFACCT_NAME]);
> +
> +	rcu_read_lock();
> +	list_for_each_entry(acct,&nfnl_acct_list, head) {

I don't really get the locking concept. All netlink operations happen under
the nfnl mutex, so using RCU for the lookup shouldn't be necessary
(applied to all netlink operations).

> +		if (strncmp(acct->name, acct_name, NFACCT_NAME_MAX) != 0)
> +			continue;
> +
> +                if (nlh->nlmsg_flags&  NLM_F_EXCL) {
> +			rcu_read_unlock();
> +			ret = -EEXIST;
> +			goto err;
> +		}
> +		matching = acct;

break?
> +        }
> +	rcu_read_unlock();
> +
> +	acct = kzalloc(sizeof(struct nf_acct), GFP_KERNEL);
> +	if (acct == NULL) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +	spin_lock_init(&acct->lock);
> +	strncpy(acct->name, nla_data(tb[NFACCT_NAME]), NFACCT_NAME_MAX);
> +	if (tb[NFACCT_BYTES])
> +		acct->bytes = be64_to_cpu(nla_get_u64(tb[NFACCT_BYTES]));
> +	if (tb[NFACCT_PKTS])
> +		acct->pkts = be64_to_cpu(nla_get_u64(tb[NFACCT_PKTS]));
> +
> +	atomic_inc(&acct->refcnt);
> +
> +	/* We are protected by nfnl mutex. */
> +	if (matching) {

This seems to be a replace operation, so I think you should
require NLM_F_REPLACE. Also it seems you could just
reinitialize the existing counter instead of unconditionally
allocating a new one.

> +		list_del_rcu(&matching->head);
> +		if (atomic_dec_and_test(&matching->refcnt))
> +			call_rcu(&matching->rcu_head, nfnl_acct_free_rcu);
> +
> +	}
> +	list_add_tail_rcu(&acct->head,&nfnl_acct_list);
> +	return 0;
> +err:
> +	return ret;
> +}
> +
> +static int
> +nfnl_acct_fill_info(struct sk_buff *skb, u32 pid, u32 seq,
> +		   int event, struct nf_acct *acct)
> +{
> +	struct nlmsghdr *nlh;
> +	struct nfgenmsg *nfmsg;
> +	unsigned int flags = pid ? NLM_F_MULTI : 0;
> +
> +	event |= NFNL_SUBSYS_ACCT<<  8;
> +	nlh = nlmsg_put(skb, pid, seq, event, sizeof(*nfmsg), flags);
> +	if (nlh == NULL)
> +		goto nlmsg_failure;
> +
> +	nfmsg = nlmsg_data(nlh);
> +	nfmsg->nfgen_family = AF_UNSPEC;
> +	nfmsg->version = NFNETLINK_V0;
> +	nfmsg->res_id = 0;
> +
> +	NLA_PUT_STRING(skb, NFACCT_NAME, acct->name);
> +	NLA_PUT_BE64(skb, NFACCT_PKTS, cpu_to_be64(acct->pkts));
> +	NLA_PUT_BE64(skb, NFACCT_BYTES, cpu_to_be64(acct->bytes));
> +
> +	nlmsg_end(skb, nlh);
> +	return skb->len;
> +
> +nlmsg_failure:
> +nla_put_failure:
> +	nlmsg_cancel(skb, nlh);
> +	return -1;
> +}
> +
> +static int
> +nfnl_acct_dump(struct sk_buff *skb, struct netlink_callback *cb)
> +{
> +	struct nf_acct *cur, *last;
> +
> +	if (cb->args[2])
> +		return 0;
> +
> +	last = (struct nf_acct *)cb->args[1];
> +	if (cb->args[1])
> +		cb->args[1] = 0;
> +
> +	rcu_read_lock();
> +	list_for_each_entry(cur,&nfnl_acct_list, head) {
> +		if (last&&  cur != last)
> +			continue;
> +
> +		if (nfnl_acct_fill_info(skb, NETLINK_CB(cb->skb).pid,
> +				       cb->nlh->nlmsg_seq,
> +				       NFNL_MSG_ACCT_NEW, cur)<  0) {
> +			cb->args[1] = (unsigned long)cur;
> +			break;
> +		}
> +
> +		if (NFNL_MSG_TYPE(cb->nlh->nlmsg_type) ==
> +						NFNL_MSG_ACCT_GET_CTRZERO) {
> +			spin_lock_bh(&cur->lock);
> +			cur->pkts = 0;
> +			cur->bytes = 0;
> +			spin_unlock_bh(&cur->lock);
> +		}
> +	}
> +	if (!cb->args[1])
> +		cb->args[2] = 1;
> +	rcu_read_unlock();
> +	return skb->len;
> +}
> +
> +static int
> +nfnl_acct_get(struct sock *nfnl, struct sk_buff *skb,
> +	     const struct nlmsghdr *nlh, const struct nlattr * const tb[])
> +{
> +	int ret = 0;
> +	struct nf_acct *cur;
> +	char *acct_name;
> +
> +	if (nlh->nlmsg_flags&  NLM_F_DUMP) {
> +		return netlink_dump_start(nfnl, skb, nlh, nfnl_acct_dump,
> +					  NULL, 0);
> +	}
> +
> +	if (!tb[NFACCT_NAME])
> +		return -EINVAL;
> +	acct_name = nla_data(tb[NFACCT_NAME]);
> +
> +	rcu_read_lock();
> +	list_for_each_entry(cur,&nfnl_acct_list, head) {
> +		struct sk_buff *skb2;
> +
> +		if (strncmp(cur->name, acct_name, NFACCT_NAME_MAX)!= 0)
> +			continue;
> +
> +		skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> +		if (skb2 == NULL)
> +			break;
> +
> +		ret = nfnl_acct_fill_info(skb2, NETLINK_CB(skb).pid,
> +					 nlh->nlmsg_seq,
> +					 NFNL_MSG_ACCT_NEW, cur);
> +		if (ret<= 0)
> +			kfree_skb(skb2);
> +
> +		if (NFNL_MSG_TYPE(nlh->nlmsg_type) ==
> +						NFNL_MSG_ACCT_GET_CTRZERO) {
> +			spin_lock_bh(&cur->lock);
> +			cur->pkts = 0;
> +			cur->bytes = 0;
> +			spin_unlock_bh(&cur->lock);
> +		}
> +		break;
> +	}
> +	rcu_read_unlock();
> +	return ret;
> +}
> +
> +static int
> +nfnl_acct_del(struct sock *nfnl, struct sk_buff *skb,
> +	     const struct nlmsghdr *nlh, const struct nlattr * const tb[])
> +{
> +	char *acct_name;
> +	struct nf_acct *cur;
> +	int ret = -ENOENT;
> +
> +	if (!tb[NFACCT_NAME]) {
> +		rcu_read_lock();
> +		list_for_each_entry(cur,&nfnl_acct_list, head) {
> +			/* We are protected by nfnl mutex. */
> +			list_del_rcu(&cur->head);
> +			if (atomic_dec_and_test(&cur->refcnt))
> +				call_rcu(&cur->rcu_head, nfnl_acct_free_rcu);

I think its strange to keep the object around after deletion if
it is still in use. In case it is still in use, I'd return -EBUSY.

> +		}
> +		rcu_read_lock();
> +		return 0;
> +	}
> +	acct_name = nla_data(tb[NFACCT_NAME]);
> +
> +	rcu_read_lock();
> +	list_for_each_entry(cur,&nfnl_acct_list, head) {
> +		if (strncmp(cur->name, acct_name, NFACCT_NAME_MAX) != 0)
> +			continue;
> +
> +		/* We are protected by nfnl mutex. */
> +		list_del_rcu(&cur->head);
> +		if (atomic_dec_and_test(&cur->refcnt))
> +			call_rcu(&cur->rcu_head, nfnl_acct_free_rcu);
> +		ret = 0;
> +		break;
> +	}
> +	rcu_read_lock();
> +	return ret;
> +}
> +
> +static const struct nla_policy nfnl_acct_policy[NFACCT_MAX+1] = {
> +	[NFACCT_NAME] = { .type = NLA_NUL_STRING, .len = NFACCT_NAME_MAX-1 },
> +	[NFACCT_BYTES] = { .type = NLA_U64 },
> +	[NFACCT_PKTS] = { .type = NLA_U64 },
> +};
> +
> +static const struct nfnl_callback nfnl_acct_cb[NFNL_MSG_ACCT_MAX] = {
> +	[NFNL_MSG_ACCT_NEW]		= { .call = nfnl_acct_new,
> +					    .attr_count = NFACCT_MAX,
> +					    .policy = nfnl_acct_policy },
> +	[NFNL_MSG_ACCT_GET] 		= { .call = nfnl_acct_get,
> +					    .attr_count = NFACCT_MAX,
> +					    .policy = nfnl_acct_policy },
> +	[NFNL_MSG_ACCT_GET_CTRZERO] 	= { .call = nfnl_acct_get,
> +					    .attr_count = NFACCT_MAX,
> +					    .policy = nfnl_acct_policy },
> +	[NFNL_MSG_ACCT_DEL]		= { .call = nfnl_acct_del,
> +					    .attr_count = NFACCT_MAX,
> +					    .policy = nfnl_acct_policy },
> +};
> +
> +static const struct nfnetlink_subsystem nfnl_acct_subsys = {
> +	.name				= "acct",
> +	.subsys_id			= NFNL_SUBSYS_ACCT,
> +	.cb_count			= NFNL_MSG_ACCT_MAX,
> +	.cb				= nfnl_acct_cb,
> +};
> +
> +MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_ACCT);
> +
> +struct nf_acct *nfnl_acct_find_get(const char *acct_name)
> +{
> +	struct nf_acct *cur, *acct = NULL;
> +
> +	rcu_read_lock();
> +	list_for_each_entry(cur,&nfnl_acct_list, head) {
> +		if (strncmp(cur->name, acct_name, NFACCT_NAME_MAX)!= 0)
> +			continue;
> +
> +		acct = cur;
> +		atomic_inc(&acct->refcnt);

This probably needs atomic_inc_not_zero() since the
lookup might race with deletion.

> +		break;
> +	}
> +	rcu_read_unlock();
> +	return acct;
> +}
> +EXPORT_SYMBOL_GPL(nfnl_acct_find_get);
> +
> +void nfnl_acct_put(struct nf_acct *acct)
> +{
> +	if (atomic_dec_and_test(&acct->refcnt))
> +		call_rcu(&acct->rcu_head, nfnl_acct_free_rcu);
> +}
> +EXPORT_SYMBOL_GPL(nfnl_acct_put);
> +
> +void nfnl_acct_update(const struct sk_buff *skb, struct nf_acct *nfacct)
> +{
> +	spin_lock_bh(&nfacct->lock);
> +	nfacct->pkts++;
> +	nfacct->bytes += skb->len;
> +	spin_unlock_bh(&nfacct->lock);
> +}
> +EXPORT_SYMBOL_GPL(nfnl_acct_update);
> +
> +static int __init nfnl_acct_init(void)
> +{
> +	int ret;
> +
> +	pr_info("nfnl_acct: registering with nfnetlink.\n");
> +	ret = nfnetlink_subsys_register(&nfnl_acct_subsys);
> +	if (ret<  0) {
> +		pr_err("nfnl_acct_init: cannot register with nfnetlink.\n");
> +		goto err_out;
> +	}
> +	return 0;
> +err_out:
> +	return ret;
> +}
> +
> +static void __exit nfnl_acct_exit(void)
> +{
> +	struct nf_acct *cur, *tmp;
> +
> +	pr_info("nfnl_acct: unregistering from nfnetlink.\n");
> +	nfnetlink_subsys_unregister(&nfnl_acct_subsys);
> +
> +	/* if we can remove this module, it means that it has no clients. */
> +	list_for_each_entry_safe(cur, tmp,&nfnl_acct_list, head) {
> +		list_del_rcu(&cur->head);
> +		if (atomic_dec_and_test(&cur->refcnt))
> +			kfree(cur);

What happens if it is non-zero? The iptables target should
take a module reference as long as its using objects that
this module is responsible for managing.

> +	}
> +}
> +
> +module_init(nfnl_acct_init);
> +module_exit(nfnl_acct_exit);

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Pablo Neira Ayuso]

On Wed, Dec 14, 2011 at 12:16:48PM +0100, Eric Dumazet wrote:
> Le mercredi 14 décembre 2011 à 12:00 +0100, pablo@netfilter.org a
> écrit :
> > From: Pablo Neira Ayuso <pablo@netfilter.org>
> > 
> > We currently have two ways to account traffic in netfilter:
> > 
> > - iptables chain and rule counters:
> > 
> >  # iptables -L -n -v
> > Chain INPUT (policy DROP 3 packets, 867 bytes)
> >  pkts bytes target     prot opt in     out     source               destination
> >     8  1104 ACCEPT     all  --  lo     *       0.0.0.0/0            0.0.0.0/0
> > 
> > - use flow-based accounting provided by ctnetlink:
> > 
> >  # conntrack -L
> > tcp      6 431999 ESTABLISHED src=192.168.1.130 dst=212.106.219.168 sport=58152 dport=80 packets=47 bytes=7654 src=212.106.219.168 dst=192.168.1.130 sport=80 dport=58152 packets=49 bytes=66340 [ASSURED] mark=0 use=1
> > 
> > While trying to display real-time accounting statistics, we require
> > to pool the kernel periodically to obtain this information. This is
> > OK if the number of flows is relatively low. However, in case that
> > the number of flows is huge, we can spend a considerable amount of
> > cycles to iterate over the list of flows that have been obtained.
> > 
> > Moreover, if we want to obtain the sum of the flow accounting results
> > that match some criteria, we have to iterate over the whole list of
> > existing flows, look for matchings and update the counters.
> > 
> > This patch adds the extended accounting infrastructure for
> > nfnetlink which aims to allow displaying real-time traffic accounting
> > without the need of complicated and resource-consuming implementation
> > in user-space. Basically, this new infrastructure allows you to create
> > accounting objects. One accounting object is composed of packet and
> > byte counters.
> > 
> > In order to manipulate create accounting objects, you require the
> > new libnetfilter_acct library. It contains several examples of use:
> > 
> > libnetfilter_acct/examples# ./nfacct-add http-traffic
> > libnetfilter_acct/examples# ./nfacct-get
> > http-traffic = { pkts = 000000000000,   bytes = 000000000000 };
> > 
> > Then, you can use one of this accounting objects in several iptables
> > rules using the new NFACCT target (which comes in a follow-up patch):
> > 
> >  # iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name http-traffic
> >  # iptables -I OUTPUT -p tcp --dport 80 -j NFACCT --nfacct-name http-traffic
> > 
> > The idea is simple: if one packet matches the rule, the NFACCT target
> > updates the counters.
> > 
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> >  include/linux/netfilter/Kbuild           |    1 +
> >  include/linux/netfilter/nfnetlink.h      |    3 +-
> >  include/linux/netfilter/nfnetlink_acct.h |   34 +++
> >  net/netfilter/Kconfig                    |    8 +
> >  net/netfilter/Makefile                   |    1 +
> >  net/netfilter/nfnetlink_acct.c           |  352 ++++++++++++++++++++++++++++++
> >  6 files changed, 398 insertions(+), 1 deletions(-)
> >  create mode 100644 include/linux/netfilter/nfnetlink_acct.h
> >  create mode 100644 net/netfilter/nfnetlink_acct.c
> > 
> > diff --git a/include/linux/netfilter/Kbuild b/include/linux/netfilter/Kbuild
> > index a1b410c..8995867 100644
> > --- a/include/linux/netfilter/Kbuild
> > +++ b/include/linux/netfilter/Kbuild
> > @@ -6,6 +6,7 @@ header-y += nf_conntrack_sctp.h
> >  header-y += nf_conntrack_tcp.h
> >  header-y += nf_conntrack_tuple_common.h
> >  header-y += nfnetlink.h
> > +header-y += nfnetlink_acct.h
> >  header-y += nfnetlink_compat.h
> >  header-y += nfnetlink_conntrack.h
> >  header-y += nfnetlink_log.h
> > diff --git a/include/linux/netfilter/nfnetlink.h b/include/linux/netfilter/nfnetlink.h
> > index 74d3386..b64454c 100644
> > --- a/include/linux/netfilter/nfnetlink.h
> > +++ b/include/linux/netfilter/nfnetlink.h
> > @@ -48,7 +48,8 @@ struct nfgenmsg {
> >  #define NFNL_SUBSYS_ULOG		4
> >  #define NFNL_SUBSYS_OSF			5
> >  #define NFNL_SUBSYS_IPSET		6
> > -#define NFNL_SUBSYS_COUNT		7
> > +#define NFNL_SUBSYS_ACCT		7
> > +#define NFNL_SUBSYS_COUNT		8
> >  
> >  #ifdef __KERNEL__
> >  
> > diff --git a/include/linux/netfilter/nfnetlink_acct.h b/include/linux/netfilter/nfnetlink_acct.h
> > new file mode 100644
> > index 0000000..9a1a119
> > --- /dev/null
> > +++ b/include/linux/netfilter/nfnetlink_acct.h
> > @@ -0,0 +1,34 @@
> > +#ifndef _NFNL_ACCT_H_
> > +#define _NFNL_ACCT_H_
> > +#include <linux/netfilter/nfnetlink.h>
> > +
> > +#define NFACCT_NAME_MAX		64
> > +
> > +enum nfnl_acct_msg_types {
> > +	NFNL_MSG_ACCT_NEW,
> > +	NFNL_MSG_ACCT_GET,
> > +	NFNL_MSG_ACCT_GET_CTRZERO,
> > +	NFNL_MSG_ACCT_DEL,
> > +	NFNL_MSG_ACCT_MAX
> > +};
> > +
> > +enum nfnl_acct_type {
> > +	NFACCT_UNSPEC,
> > +	NFACCT_NAME,
> > +	NFACCT_PKTS,
> > +	NFACCT_BYTES,
> > +	__NFACCT_MAX
> > +};
> > +#define NFACCT_MAX (__NFACCT_MAX - 1)
> > +
> > +#ifdef __KERNEL__
> > +
> > +struct nf_acct;
> > +
> > +extern struct nf_acct *nfnl_acct_find_get(const char *filter_name);
> > +extern void nfnl_acct_put(struct nf_acct *acct);
> > +extern void nfnl_acct_update(const struct sk_buff *skb, struct nf_acct *nfacct);
> > +
> > +#endif /* __KERNEL__ */
> > +
> > +#endif /* _NFNL_ACCT_H */
> > diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
> > index d5597b7..77326ac 100644
> > --- a/net/netfilter/Kconfig
> > +++ b/net/netfilter/Kconfig
> > @@ -4,6 +4,14 @@ menu "Core Netfilter Configuration"
> >  config NETFILTER_NETLINK
> >  	tristate
> >  
> > +config NETFILTER_NETLINK_ACCT
> > +tristate "Netfilter NFACCT over NFNETLINK interface"
> > +	depends on NETFILTER_ADVANCED
> > +	select NETFILTER_NETLINK
> > +	help
> > +	  If this option is enabled, the kernel will include support
> > +	  for extended accounting via NFNETLINK.
> > +
> >  config NETFILTER_NETLINK_QUEUE
> >  	tristate "Netfilter NFQUEUE over NFNETLINK interface"
> >  	depends on NETFILTER_ADVANCED
> > diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
> > index 1a02853..4da1c87 100644
> > --- a/net/netfilter/Makefile
> > +++ b/net/netfilter/Makefile
> > @@ -7,6 +7,7 @@ nf_conntrack-$(CONFIG_NF_CONNTRACK_EVENTS) += nf_conntrack_ecache.o
> >  obj-$(CONFIG_NETFILTER) = netfilter.o
> >  
> >  obj-$(CONFIG_NETFILTER_NETLINK) += nfnetlink.o
> > +obj-$(CONFIG_NETFILTER_NETLINK_ACCT) += nfnetlink_acct.o
> >  obj-$(CONFIG_NETFILTER_NETLINK_QUEUE) += nfnetlink_queue.o
> >  obj-$(CONFIG_NETFILTER_NETLINK_LOG) += nfnetlink_log.o
> >  
> > diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
> > new file mode 100644
> > index 0000000..3ec407f
> > --- /dev/null
> > +++ b/net/netfilter/nfnetlink_acct.c
> > @@ -0,0 +1,352 @@
> > +/*
> > + * (C) 2011 Pablo Neira Ayuso <pablo@netfilter.org>
> > + * (C) 2011 Intra2net AG <http://www.intra2net.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation (or any later at your option).
> > + */
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +#include <linux/skbuff.h>
> > +#include <linux/netlink.h>
> > +#include <linux/rculist.h>
> > +#include <linux/slab.h>
> > +#include <linux/types.h>
> > +#include <linux/errno.h>
> > +#include <net/netlink.h>
> > +#include <net/sock.h>
> > +#include <asm/atomic.h>
> > +
> > +#include <linux/netfilter.h>
> > +#include <linux/netfilter/nfnetlink.h>
> > +#include <linux/netfilter/nfnetlink_acct.h>
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Pablo Neira Ayuso <pablo@netfilter.org>");
> > +MODULE_DESCRIPTION("nfacct: Extended Netfilter accounting infrastructure");
> > +
> > +static LIST_HEAD(nfnl_acct_list);
> > +
> > +struct nf_acct {
> > +	struct rcu_head		rcu_head;
> > +	struct list_head	head;
> > +	spinlock_t		lock;	/* to update the counters. */
> > +	atomic_t		refcnt;
> > +
> > +	char			name[NFACCT_NAME_MAX];
> > +	__u64			pkts;
> > +	__u64			bytes;
> 
> atomic64_t ?
>
> This would remove use of spinlock in fast path

Good idea :-).

Not related to this, but we can also replace this in the connection
tracking system.

> Also, you put lock and pkts,bytes in different cache lines :(

Sorry, I added the locking in a later stage while in the rush, I
completely missed this.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] [RFC] Extended accounting infrastructure for iptables

[Changli Gao]

On Wed, Dec 14, 2011 at 7:00 PM,  <pablo@netfilter.org> wrote:
> From: Pablo Neira Ayuso <pablo@netfilter.org>
>
> Hi!
>
> We currently have two ways to account traffic in netfilter:
>
> - iptables chain and rule counters:
>
>  # iptables -L -n -v
> Chain INPUT (policy DROP 3 packets, 867 bytes)
>  pkts bytes target     prot opt in     out     source               destinat
>    8  1104 ACCEPT     all  --  lo     *       0.0.0.0/0            0.0.0.0/
>
> - use flow-based accounting provided by ctnetlink:
>
>  # conntrack -L
> tcp      6 431999 ESTABLISHED src=192.168.1.130 dst=212.106.219.168 sport=58
>
> While trying to display real-time accounting statistics, we require
> to pool the kernel periodically to obtain this information. This is
> OK if the number of flows is relatively low. However, in case that
> the number of flows is huge, we can spend a considerable amount of
> cycles to iterate over the list of flows that have been obtained.
>
> Moreover, if we want to obtain the sum of the flow accounting results
> that match some criteria, we have to iterate over the whole list of
> existing flows, look for matchings and update the counters.
>
> This patchset adds the extended accounting infrastructure in
> kernel-space. It is composed of one nfnetlink interface that
> allows you to create, to update and to retrieve accounting objects.
> These objects can be used to account traffic with the flexibility
> that iptables rules provide (by means of the new NFACCT target).
>
> Quick example of use:
>
> 1) You create the accounting object:
>
> libnetfilter_acct/examples# ./nfacct-add http-traffic
>
> 2) Add the iptables rules for traffic you want to account:
>
> # iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name http-traffic
> # iptables -I OUTPUT -p tcp --dport 80 -j NFACCT --nfacct-name http-traffic
>

Why not use the counters of iptables instead?

iptables-save -c

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Pablo Neira Ayuso]

On Wed, Dec 14, 2011 at 12:23:21PM +0100, Patrick McHardy wrote:
[...]
> >+nfnl_acct_new(struct sock *nfnl, struct sk_buff *skb,
> >+	     const struct nlmsghdr *nlh, const struct nlattr * const tb[])
> >+{
> >+	int ret;
> >+	struct nf_acct *acct, *matching = NULL;
> >+	char *acct_name;
> >+
> >+	if (!tb[NFACCT_NAME])
> >+		return -EINVAL;
> >+
> >+	acct_name = nla_data(tb[NFACCT_NAME]);
> >+
> >+	rcu_read_lock();
> >+	list_for_each_entry(acct,&nfnl_acct_list, head) {
> 
> I don't really get the locking concept. All netlink operations happen under
> the nfnl mutex, so using RCU for the lookup shouldn't be necessary
> (applied to all netlink operations).

If you add one iptables rule with NFACCT, you have to iterate over the
list (without the mutex). Very unlikely, but we may delete one
accounting object via nfnetlink at the same time of adding the rule
that refers to it.

> >+		if (strncmp(acct->name, acct_name, NFACCT_NAME_MAX) != 0)
> >+			continue;
> >+
> >+                if (nlh->nlmsg_flags&  NLM_F_EXCL) {
> >+			rcu_read_unlock();
> >+			ret = -EEXIST;
> >+			goto err;
> >+		}
> >+		matching = acct;
> 
> break?

Indeed.

> >+        }
> >+	rcu_read_unlock();
> >+
> >+	acct = kzalloc(sizeof(struct nf_acct), GFP_KERNEL);
> >+	if (acct == NULL) {
> >+		ret = -ENOMEM;
> >+		goto err;
> >+	}
> >+	spin_lock_init(&acct->lock);
> >+	strncpy(acct->name, nla_data(tb[NFACCT_NAME]), NFACCT_NAME_MAX);
> >+	if (tb[NFACCT_BYTES])
> >+		acct->bytes = be64_to_cpu(nla_get_u64(tb[NFACCT_BYTES]));
> >+	if (tb[NFACCT_PKTS])
> >+		acct->pkts = be64_to_cpu(nla_get_u64(tb[NFACCT_PKTS]));
> >+
> >+	atomic_inc(&acct->refcnt);
> >+
> >+	/* We are protected by nfnl mutex. */
> >+	if (matching) {
> 
> This seems to be a replace operation, so I think you should
> require NLM_F_REPLACE. Also it seems you could just
> reinitialize the existing counter instead of unconditionally
> allocating a new one.

I think it's easier to return -EBUSY as you suggested.

> >+		list_del_rcu(&matching->head);
> >+		if (atomic_dec_and_test(&matching->refcnt))
> >+			call_rcu(&matching->rcu_head, nfnl_acct_free_rcu);
> >+
> >+	}
> >+	list_add_tail_rcu(&acct->head,&nfnl_acct_list);
> >+	return 0;
> >+err:
> >+	return ret;
> >+}
> >+
> >+static int
> >+nfnl_acct_fill_info(struct sk_buff *skb, u32 pid, u32 seq,
> >+		   int event, struct nf_acct *acct)
> >+{
> >+	struct nlmsghdr *nlh;
> >+	struct nfgenmsg *nfmsg;
> >+	unsigned int flags = pid ? NLM_F_MULTI : 0;
> >+
> >+	event |= NFNL_SUBSYS_ACCT<<  8;
> >+	nlh = nlmsg_put(skb, pid, seq, event, sizeof(*nfmsg), flags);
> >+	if (nlh == NULL)
> >+		goto nlmsg_failure;
> >+
> >+	nfmsg = nlmsg_data(nlh);
> >+	nfmsg->nfgen_family = AF_UNSPEC;
> >+	nfmsg->version = NFNETLINK_V0;
> >+	nfmsg->res_id = 0;
> >+
> >+	NLA_PUT_STRING(skb, NFACCT_NAME, acct->name);
> >+	NLA_PUT_BE64(skb, NFACCT_PKTS, cpu_to_be64(acct->pkts));
> >+	NLA_PUT_BE64(skb, NFACCT_BYTES, cpu_to_be64(acct->bytes));
> >+
> >+	nlmsg_end(skb, nlh);
> >+	return skb->len;
> >+
> >+nlmsg_failure:
> >+nla_put_failure:
> >+	nlmsg_cancel(skb, nlh);
> >+	return -1;
> >+}
> >+
> >+static int
> >+nfnl_acct_dump(struct sk_buff *skb, struct netlink_callback *cb)
> >+{
> >+	struct nf_acct *cur, *last;
> >+
> >+	if (cb->args[2])
> >+		return 0;
> >+
> >+	last = (struct nf_acct *)cb->args[1];
> >+	if (cb->args[1])
> >+		cb->args[1] = 0;
> >+
> >+	rcu_read_lock();
> >+	list_for_each_entry(cur,&nfnl_acct_list, head) {
> >+		if (last&&  cur != last)
> >+			continue;
> >+
> >+		if (nfnl_acct_fill_info(skb, NETLINK_CB(cb->skb).pid,
> >+				       cb->nlh->nlmsg_seq,
> >+				       NFNL_MSG_ACCT_NEW, cur)<  0) {
> >+			cb->args[1] = (unsigned long)cur;
> >+			break;
> >+		}
> >+
> >+		if (NFNL_MSG_TYPE(cb->nlh->nlmsg_type) ==
> >+						NFNL_MSG_ACCT_GET_CTRZERO) {
> >+			spin_lock_bh(&cur->lock);
> >+			cur->pkts = 0;
> >+			cur->bytes = 0;
> >+			spin_unlock_bh(&cur->lock);
> >+		}
> >+	}
> >+	if (!cb->args[1])
> >+		cb->args[2] = 1;
> >+	rcu_read_unlock();
> >+	return skb->len;
> >+}
> >+
> >+static int
> >+nfnl_acct_get(struct sock *nfnl, struct sk_buff *skb,
> >+	     const struct nlmsghdr *nlh, const struct nlattr * const tb[])
> >+{
> >+	int ret = 0;
> >+	struct nf_acct *cur;
> >+	char *acct_name;
> >+
> >+	if (nlh->nlmsg_flags&  NLM_F_DUMP) {
> >+		return netlink_dump_start(nfnl, skb, nlh, nfnl_acct_dump,
> >+					  NULL, 0);
> >+	}
> >+
> >+	if (!tb[NFACCT_NAME])
> >+		return -EINVAL;
> >+	acct_name = nla_data(tb[NFACCT_NAME]);
> >+
> >+	rcu_read_lock();
> >+	list_for_each_entry(cur,&nfnl_acct_list, head) {
> >+		struct sk_buff *skb2;
> >+
> >+		if (strncmp(cur->name, acct_name, NFACCT_NAME_MAX)!= 0)
> >+			continue;
> >+
> >+		skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> >+		if (skb2 == NULL)
> >+			break;
> >+
> >+		ret = nfnl_acct_fill_info(skb2, NETLINK_CB(skb).pid,
> >+					 nlh->nlmsg_seq,
> >+					 NFNL_MSG_ACCT_NEW, cur);
> >+		if (ret<= 0)
> >+			kfree_skb(skb2);
> >+
> >+		if (NFNL_MSG_TYPE(nlh->nlmsg_type) ==
> >+						NFNL_MSG_ACCT_GET_CTRZERO) {
> >+			spin_lock_bh(&cur->lock);
> >+			cur->pkts = 0;
> >+			cur->bytes = 0;
> >+			spin_unlock_bh(&cur->lock);
> >+		}
> >+		break;
> >+	}
> >+	rcu_read_unlock();
> >+	return ret;
> >+}
> >+
> >+static int
> >+nfnl_acct_del(struct sock *nfnl, struct sk_buff *skb,
> >+	     const struct nlmsghdr *nlh, const struct nlattr * const tb[])
> >+{
> >+	char *acct_name;
> >+	struct nf_acct *cur;
> >+	int ret = -ENOENT;
> >+
> >+	if (!tb[NFACCT_NAME]) {
> >+		rcu_read_lock();
> >+		list_for_each_entry(cur,&nfnl_acct_list, head) {
> >+			/* We are protected by nfnl mutex. */
> >+			list_del_rcu(&cur->head);
> >+			if (atomic_dec_and_test(&cur->refcnt))
> >+				call_rcu(&cur->rcu_head, nfnl_acct_free_rcu);
> 
> I think its strange to keep the object around after deletion if
> it is still in use. In case it is still in use, I'd return -EBUSY.

-EBUSY sounds fine to me.

> >+		}
> >+		rcu_read_lock();
> >+		return 0;
> >+	}
> >+	acct_name = nla_data(tb[NFACCT_NAME]);
> >+
> >+	rcu_read_lock();
> >+	list_for_each_entry(cur,&nfnl_acct_list, head) {
> >+		if (strncmp(cur->name, acct_name, NFACCT_NAME_MAX) != 0)
> >+			continue;
> >+
> >+		/* We are protected by nfnl mutex. */
> >+		list_del_rcu(&cur->head);
> >+		if (atomic_dec_and_test(&cur->refcnt))
> >+			call_rcu(&cur->rcu_head, nfnl_acct_free_rcu);
> >+		ret = 0;
> >+		break;
> >+	}
> >+	rcu_read_lock();
> >+	return ret;
> >+}
> >+
> >+static const struct nla_policy nfnl_acct_policy[NFACCT_MAX+1] = {
> >+	[NFACCT_NAME] = { .type = NLA_NUL_STRING, .len = NFACCT_NAME_MAX-1 },
> >+	[NFACCT_BYTES] = { .type = NLA_U64 },
> >+	[NFACCT_PKTS] = { .type = NLA_U64 },
> >+};
> >+
> >+static const struct nfnl_callback nfnl_acct_cb[NFNL_MSG_ACCT_MAX] = {
> >+	[NFNL_MSG_ACCT_NEW]		= { .call = nfnl_acct_new,
> >+					    .attr_count = NFACCT_MAX,
> >+					    .policy = nfnl_acct_policy },
> >+	[NFNL_MSG_ACCT_GET] 		= { .call = nfnl_acct_get,
> >+					    .attr_count = NFACCT_MAX,
> >+					    .policy = nfnl_acct_policy },
> >+	[NFNL_MSG_ACCT_GET_CTRZERO] 	= { .call = nfnl_acct_get,
> >+					    .attr_count = NFACCT_MAX,
> >+					    .policy = nfnl_acct_policy },
> >+	[NFNL_MSG_ACCT_DEL]		= { .call = nfnl_acct_del,
> >+					    .attr_count = NFACCT_MAX,
> >+					    .policy = nfnl_acct_policy },
> >+};
> >+
> >+static const struct nfnetlink_subsystem nfnl_acct_subsys = {
> >+	.name				= "acct",
> >+	.subsys_id			= NFNL_SUBSYS_ACCT,
> >+	.cb_count			= NFNL_MSG_ACCT_MAX,
> >+	.cb				= nfnl_acct_cb,
> >+};
> >+
> >+MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_ACCT);
> >+
> >+struct nf_acct *nfnl_acct_find_get(const char *acct_name)
> >+{
> >+	struct nf_acct *cur, *acct = NULL;
> >+
> >+	rcu_read_lock();
> >+	list_for_each_entry(cur,&nfnl_acct_list, head) {
> >+		if (strncmp(cur->name, acct_name, NFACCT_NAME_MAX)!= 0)
> >+			continue;
> >+
> >+		acct = cur;
> >+		atomic_inc(&acct->refcnt);
> 
> This probably needs atomic_inc_not_zero() since the
> lookup might race with deletion.

I'll fix this.

> >+		break;
> >+	}
> >+	rcu_read_unlock();
> >+	return acct;
> >+}
> >+EXPORT_SYMBOL_GPL(nfnl_acct_find_get);
> >+
> >+void nfnl_acct_put(struct nf_acct *acct)
> >+{
> >+	if (atomic_dec_and_test(&acct->refcnt))
> >+		call_rcu(&acct->rcu_head, nfnl_acct_free_rcu);
> >+}
> >+EXPORT_SYMBOL_GPL(nfnl_acct_put);
> >+
> >+void nfnl_acct_update(const struct sk_buff *skb, struct nf_acct *nfacct)
> >+{
> >+	spin_lock_bh(&nfacct->lock);
> >+	nfacct->pkts++;
> >+	nfacct->bytes += skb->len;
> >+	spin_unlock_bh(&nfacct->lock);
> >+}
> >+EXPORT_SYMBOL_GPL(nfnl_acct_update);
> >+
> >+static int __init nfnl_acct_init(void)
> >+{
> >+	int ret;
> >+
> >+	pr_info("nfnl_acct: registering with nfnetlink.\n");
> >+	ret = nfnetlink_subsys_register(&nfnl_acct_subsys);
> >+	if (ret<  0) {
> >+		pr_err("nfnl_acct_init: cannot register with nfnetlink.\n");
> >+		goto err_out;
> >+	}
> >+	return 0;
> >+err_out:
> >+	return ret;
> >+}
> >+
> >+static void __exit nfnl_acct_exit(void)
> >+{
> >+	struct nf_acct *cur, *tmp;
> >+
> >+	pr_info("nfnl_acct: unregistering from nfnetlink.\n");
> >+	nfnetlink_subsys_unregister(&nfnl_acct_subsys);
> >+
> >+	/* if we can remove this module, it means that it has no clients. */
> >+	list_for_each_entry_safe(cur, tmp,&nfnl_acct_list, head) {
> >+		list_del_rcu(&cur->head);
> >+		if (atomic_dec_and_test(&cur->refcnt))
> >+			kfree(cur);
> 
> What happens if it is non-zero? The iptables target should
> take a module reference as long as its using objects that
> this module is responsible for managing.

I'll fix this as well.

Thanks for the review!

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Eric Dumazet]

Le mercredi 14 décembre 2011 à 13:41 +0100, Pablo Neira Ayuso a écrit :

> Not related to this, but we can also replace this in the connection
> tracking system.

Very good point indeed ;)



--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Changli Gao]

On Wed, Dec 14, 2011 at 7:00 PM,  <pablo@netfilter.org> wrote:
> From: Pablo Neira Ayuso <pablo@netfilter.org>
>
> We currently have two ways to account traffic in netfilter:
>
> - iptables chain and rule counters:
>
>  # iptables -L -n -v
> Chain INPUT (policy DROP 3 packets, 867 bytes)
>  pkts bytes target     prot opt in     out     source               destination
>    8  1104 ACCEPT     all  --  lo     *       0.0.0.0/0            0.0.0.0/0
>
> - use flow-based accounting provided by ctnetlink:
>
>  # conntrack -L
> tcp      6 431999 ESTABLISHED src=192.168.1.130 dst=212.106.219.168 sport=58152 dport=80 packets=47 bytes=7654 src=212.106.219.168 dst=192.168.1.130 sport=80 dport=58152 packets=49 bytes=66340 [ASSURED] mark=0 use=1
>
> While trying to display real-time accounting statistics, we require
> to pool the kernel periodically to obtain this information. This is
> OK if the number of flows is relatively low. However, in case that
> the number of flows is huge, we can spend a considerable amount of
> cycles to iterate over the list of flows that have been obtained.
>
> Moreover, if we want to obtain the sum of the flow accounting results
> that match some criteria, we have to iterate over the whole list of
> existing flows, look for matchings and update the counters.
>
> This patch adds the extended accounting infrastructure for
> nfnetlink which aims to allow displaying real-time traffic accounting
> without the need of complicated and resource-consuming implementation
> in user-space. Basically, this new infrastructure allows you to create
> accounting objects. One accounting object is composed of packet and
> byte counters.
>
> In order to manipulate create accounting objects, you require the
> new libnetfilter_acct library. It contains several examples of use:
>
> libnetfilter_acct/examples# ./nfacct-add http-traffic
> libnetfilter_acct/examples# ./nfacct-get
> http-traffic = { pkts = 000000000000,   bytes = 000000000000 };
>
> Then, you can use one of this accounting objects in several iptables
> rules using the new NFACCT target (which comes in a follow-up patch):
>

But you can replace nfacct with a separated chain, then the iptables
statistics counter of this chain can be used.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] [RFC] Extended accounting infrastructure for iptables

[Pablo Neira Ayuso]

On Wed, Dec 14, 2011 at 09:12:52PM +0800, Changli Gao wrote:
> On Wed, Dec 14, 2011 at 7:00 PM,  <pablo@netfilter.org> wrote:
> > From: Pablo Neira Ayuso <pablo@netfilter.org>
> >
> > Hi!
> >
> > We currently have two ways to account traffic in netfilter:
> >
> > - iptables chain and rule counters:
> >
> >  # iptables -L -n -v
> > Chain INPUT (policy DROP 3 packets, 867 bytes)
> >  pkts bytes target     prot opt in     out     source               destinat
> >    8  1104 ACCEPT     all  --  lo     *       0.0.0.0/0            0.0.0.0/
> >
> > - use flow-based accounting provided by ctnetlink:
> >
> >  # conntrack -L
> > tcp      6 431999 ESTABLISHED src=192.168.1.130 dst=212.106.219.168 sport=58
> >
> > While trying to display real-time accounting statistics, we require
> > to pool the kernel periodically to obtain this information. This is
> > OK if the number of flows is relatively low. However, in case that
> > the number of flows is huge, we can spend a considerable amount of
> > cycles to iterate over the list of flows that have been obtained.
> >
> > Moreover, if we want to obtain the sum of the flow accounting results
> > that match some criteria, we have to iterate over the whole list of
> > existing flows, look for matchings and update the counters.
> >
> > This patchset adds the extended accounting infrastructure in
> > kernel-space. It is composed of one nfnetlink interface that
> > allows you to create, to update and to retrieve accounting objects.
> > These objects can be used to account traffic with the flexibility
> > that iptables rules provide (by means of the new NFACCT target).
> >
> > Quick example of use:
> >
> > 1) You create the accounting object:
> >
> > libnetfilter_acct/examples# ./nfacct-add http-traffic
> >
> > 2) Add the iptables rules for traffic you want to account:
> >
> > # iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name http-traffic
> > # iptables -I OUTPUT -p tcp --dport 80 -j NFACCT --nfacct-name http-traffic
> >
> 
> Why not use the counters of iptables instead?
> 
> iptables-save -c

If you want to obtain the sum of the counters that match some criteria,
you have to iterate over the whole list of existing rules, look for
matchings and update the counters.

Moreover, if you have a large rule-set, polling periodically
iptables-save -c can be expensive.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] [RFC] Extended accounting infrastructure for iptables

[Anand Raj Manickam]

Great Stuff!

On Wed, Dec 14, 2011 at 7:00 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Wed, Dec 14, 2011 at 09:12:52PM +0800, Changli Gao wrote:
>> On Wed, Dec 14, 2011 at 7:00 PM,  <pablo@netfilter.org> wrote:
>> > From: Pablo Neira Ayuso <pablo@netfilter.org>
>> >
>> > Hi!
>> >
>> > We currently have two ways to account traffic in netfilter:
>> >
>> > - iptables chain and rule counters:
>> >
>> >  # iptables -L -n -v
>> > Chain INPUT (policy DROP 3 packets, 867 bytes)
>> >  pkts bytes target     prot opt in     out     source               destinat
>> >    8  1104 ACCEPT     all  --  lo     *       0.0.0.0/0            0.0.0.0/
>> >
>> > - use flow-based accounting provided by ctnetlink:
>> >
>> >  # conntrack -L
>> > tcp      6 431999 ESTABLISHED src=192.168.1.130 dst=212.106.219.168 sport=58
>> >
>> > While trying to display real-time accounting statistics, we require
>> > to pool the kernel periodically to obtain this information. This is
>> > OK if the number of flows is relatively low. However, in case that
>> > the number of flows is huge, we can spend a considerable amount of
>> > cycles to iterate over the list of flows that have been obtained.
>> >
>> > Moreover, if we want to obtain the sum of the flow accounting results
>> > that match some criteria, we have to iterate over the whole list of
>> > existing flows, look for matchings and update the counters.
>> >
>> > This patchset adds the extended accounting infrastructure in
>> > kernel-space. It is composed of one nfnetlink interface that
>> > allows you to create, to update and to retrieve accounting objects.
>> > These objects can be used to account traffic with the flexibility
>> > that iptables rules provide (by means of the new NFACCT target).
>> >
>> > Quick example of use:
>> >
>> > 1) You create the accounting object:
>> >
>> > libnetfilter_acct/examples# ./nfacct-add http-traffic
>> >
>> > 2) Add the iptables rules for traffic you want to account:
>> >
>> > # iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name http-traffic
>> > # iptables -I OUTPUT -p tcp --dport 80 -j NFACCT --nfacct-name http-traffic
>> >
>>
>> Why not use the counters of iptables instead?
>>
>> iptables-save -c
>
> If you want to obtain the sum of the counters that match some criteria,
> you have to iterate over the whole list of existing rules, look for
> matchings and update the counters.
>
> Moreover, if you have a large rule-set, polling periodically
> iptables-save -c can be expensive.
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Jan Engelhardt]

On Wednesday 2011-12-14 12:00, pablo@netfilter.org wrote:

>Then, you can use one of this accounting objects in several iptables
>rules using the new NFACCT target (which comes in a follow-up patch):
>
> # iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name http-traffic
> # iptables -I OUTPUT -p tcp --dport 80 -j NFACCT --nfacct-name http-traffic
>
>The idea is simple: if one packet matches the rule, the NFACCT target
>updates the counters.

This smells a lot like -m quota2 --grow, except that yours uses 
netlink instead of procfs and can only update the counters.

I suggest to turn -j NFACCT into -m nfacct instead, so that we can add 
counting-down mode and matching capabilities, so as to replace 
xt_quota*.

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Eric Dumazet]

Le mercredi 14 décembre 2011 à 14:18 +0100, Eric Dumazet a écrit :
> Le mercredi 14 décembre 2011 à 13:41 +0100, Pablo Neira Ayuso a écrit :
> 
> > Not related to this, but we can also replace this in the connection
> > tracking system.
> 
> Very good point indeed ;)
> 
> 

[PATCH] conntrack: use atomic64 for acct counters

We can use atomic64_t infrastructure to avoid taking a spinlock in fast
path, and remove inaccuracies while reading values in
ctnetlink_dump_counters() and connbytes_mt() on 32bit arches.

Suggested by Pablo

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/net/netfilter/nf_conntrack_acct.h |    4 +-
 net/netfilter/nf_conntrack_acct.c         |    4 +-
 net/netfilter/nf_conntrack_core.c         |   14 +++-----
 net/netfilter/nf_conntrack_netlink.c      |   12 +++++--
 net/netfilter/xt_connbytes.c              |   32 ++++++++++----------
 5 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_acct.h b/include/net/netfilter/nf_conntrack_acct.h
index 4e9c63a..463ae8e 100644
--- a/include/net/netfilter/nf_conntrack_acct.h
+++ b/include/net/netfilter/nf_conntrack_acct.h
@@ -15,8 +15,8 @@
 #include <net/netfilter/nf_conntrack_extend.h>
 
 struct nf_conn_counter {
-	u_int64_t packets;
-	u_int64_t bytes;
+	atomic64_t packets;
+	atomic64_t bytes;
 };
 
 static inline
diff --git a/net/netfilter/nf_conntrack_acct.c b/net/netfilter/nf_conntrack_acct.c
index 369df3f..9332906 100644
--- a/net/netfilter/nf_conntrack_acct.c
+++ b/net/netfilter/nf_conntrack_acct.c
@@ -46,8 +46,8 @@ seq_print_acct(struct seq_file *s, const struct nf_conn *ct, int dir)
 		return 0;
 
 	return seq_printf(s, "packets=%llu bytes=%llu ",
-			  (unsigned long long)acct[dir].packets,
-			  (unsigned long long)acct[dir].bytes);
+			  (unsigned long long)atomic64_read(&acct[dir].packets),
+			  (unsigned long long)atomic64_read(&acct[dir].bytes));
 };
 EXPORT_SYMBOL_GPL(seq_print_acct);
 
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 7202b06..8b2842e 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1044,10 +1044,8 @@ acct:
 
 		acct = nf_conn_acct_find(ct);
 		if (acct) {
-			spin_lock_bh(&ct->lock);
-			acct[CTINFO2DIR(ctinfo)].packets++;
-			acct[CTINFO2DIR(ctinfo)].bytes += skb->len;
-			spin_unlock_bh(&ct->lock);
+			atomic64_inc(&acct[CTINFO2DIR(ctinfo)].packets);
+			atomic64_add(skb->len, &acct[CTINFO2DIR(ctinfo)].bytes);
 		}
 	}
 }
@@ -1063,11 +1061,9 @@ bool __nf_ct_kill_acct(struct nf_conn *ct,
 
 		acct = nf_conn_acct_find(ct);
 		if (acct) {
-			spin_lock_bh(&ct->lock);
-			acct[CTINFO2DIR(ctinfo)].packets++;
-			acct[CTINFO2DIR(ctinfo)].bytes +=
-				skb->len - skb_network_offset(skb);
-			spin_unlock_bh(&ct->lock);
+			atomic64_inc(&acct[CTINFO2DIR(ctinfo)].packets);
+			atomic64_add(skb->len - skb_network_offset(skb),
+				     &acct[CTINFO2DIR(ctinfo)].bytes);
 		}
 	}
 
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index ef21b22..a36e655 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -219,9 +219,9 @@ ctnetlink_dump_counters(struct sk_buff *skb, const struct nf_conn *ct,
 		goto nla_put_failure;
 
 	NLA_PUT_BE64(skb, CTA_COUNTERS_PACKETS,
-		     cpu_to_be64(acct[dir].packets));
+		     cpu_to_be64(atomic64_read(&acct[dir].packets)));
 	NLA_PUT_BE64(skb, CTA_COUNTERS_BYTES,
-		     cpu_to_be64(acct[dir].bytes));
+		     cpu_to_be64(atomic64_read(&acct[dir].bytes)));
 
 	nla_nest_end(skb, nest_count);
 
@@ -720,8 +720,12 @@ restart:
 				struct nf_conn_counter *acct;
 
 				acct = nf_conn_acct_find(ct);
-				if (acct)
-					memset(acct, 0, sizeof(struct nf_conn_counter[IP_CT_DIR_MAX]));
+				if (acct) {
+					atomic64_set(&acct[IP_CT_DIR_ORIGINAL].bytes, 0);
+					atomic64_set(&acct[IP_CT_DIR_ORIGINAL].packets, 0);
+					atomic64_set(&acct[IP_CT_DIR_REPLY].bytes, 0);
+					atomic64_set(&acct[IP_CT_DIR_REPLY].packets, 0);
+					}
 			}
 		}
 		if (cb->args[1]) {
diff --git a/net/netfilter/xt_connbytes.c b/net/netfilter/xt_connbytes.c
index 5b13850..2b8418c 100644
--- a/net/netfilter/xt_connbytes.c
+++ b/net/netfilter/xt_connbytes.c
@@ -40,46 +40,46 @@ connbytes_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	case XT_CONNBYTES_PKTS:
 		switch (sinfo->direction) {
 		case XT_CONNBYTES_DIR_ORIGINAL:
-			what = counters[IP_CT_DIR_ORIGINAL].packets;
+			what = atomic64_read(&counters[IP_CT_DIR_ORIGINAL].packets);
 			break;
 		case XT_CONNBYTES_DIR_REPLY:
-			what = counters[IP_CT_DIR_REPLY].packets;
+			what = atomic64_read(&counters[IP_CT_DIR_REPLY].packets);
 			break;
 		case XT_CONNBYTES_DIR_BOTH:
-			what = counters[IP_CT_DIR_ORIGINAL].packets;
-			what += counters[IP_CT_DIR_REPLY].packets;
+			what = atomic64_read(&counters[IP_CT_DIR_ORIGINAL].packets);
+			what += atomic64_read(&counters[IP_CT_DIR_REPLY].packets);
 			break;
 		}
 		break;
 	case XT_CONNBYTES_BYTES:
 		switch (sinfo->direction) {
 		case XT_CONNBYTES_DIR_ORIGINAL:
-			what = counters[IP_CT_DIR_ORIGINAL].bytes;
+			what = atomic64_read(&counters[IP_CT_DIR_ORIGINAL].bytes);
 			break;
 		case XT_CONNBYTES_DIR_REPLY:
-			what = counters[IP_CT_DIR_REPLY].bytes;
+			what = atomic64_read(&counters[IP_CT_DIR_REPLY].bytes);
 			break;
 		case XT_CONNBYTES_DIR_BOTH:
-			what = counters[IP_CT_DIR_ORIGINAL].bytes;
-			what += counters[IP_CT_DIR_REPLY].bytes;
+			what = atomic64_read(&counters[IP_CT_DIR_ORIGINAL].bytes);
+			what += atomic64_read(&counters[IP_CT_DIR_REPLY].bytes);
 			break;
 		}
 		break;
 	case XT_CONNBYTES_AVGPKT:
 		switch (sinfo->direction) {
 		case XT_CONNBYTES_DIR_ORIGINAL:
-			bytes = counters[IP_CT_DIR_ORIGINAL].bytes;
-			pkts  = counters[IP_CT_DIR_ORIGINAL].packets;
+			bytes = atomic64_read(&counters[IP_CT_DIR_ORIGINAL].bytes);
+			pkts  = atomic64_read(&counters[IP_CT_DIR_ORIGINAL].packets);
 			break;
 		case XT_CONNBYTES_DIR_REPLY:
-			bytes = counters[IP_CT_DIR_REPLY].bytes;
-			pkts  = counters[IP_CT_DIR_REPLY].packets;
+			bytes = atomic64_read(&counters[IP_CT_DIR_REPLY].bytes);
+			pkts  = atomic64_read(&counters[IP_CT_DIR_REPLY].packets);
 			break;
 		case XT_CONNBYTES_DIR_BOTH:
-			bytes = counters[IP_CT_DIR_ORIGINAL].bytes +
-				counters[IP_CT_DIR_REPLY].bytes;
-			pkts  = counters[IP_CT_DIR_ORIGINAL].packets +
-				counters[IP_CT_DIR_REPLY].packets;
+			bytes = atomic64_read(&counters[IP_CT_DIR_ORIGINAL].bytes) +
+				atomic64_read(&counters[IP_CT_DIR_REPLY].bytes);
+			pkts  = atomic64_read(&counters[IP_CT_DIR_ORIGINAL].packets) +
+				atomic64_read(&counters[IP_CT_DIR_REPLY].packets);
 			break;
 		}
 		if (pkts != 0)


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Anand Raj Manickam]

A clarification :
If its about flow based accounting , wont the below rules double the counter ?
# iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name
http-traffic# iptables -I OUTPUT -p tcp --dport 80 -j NFACCT
--nfacct-name http-traffic







On Wed, Dec 14, 2011 at 4:30 PM,  <pablo@netfilter.org> wrote:
> From: Pablo Neira Ayuso <pablo@netfilter.org>
>
> We currently have two ways to account traffic in netfilter:
>
> - iptables chain and rule counters:
>
>  # iptables -L -n -v
> Chain INPUT (policy DROP 3 packets, 867 bytes)
>  pkts bytes target     prot opt in     out     source               destination
>    8  1104 ACCEPT     all  --  lo     *       0.0.0.0/0            0.0.0.0/0
>
> - use flow-based accounting provided by ctnetlink:
>
>  # conntrack -L
> tcp      6 431999 ESTABLISHED src=192.168.1.130 dst=212.106.219.168 sport=58152 dport=80 packets=47 bytes=7654 src=212.106.219.168 dst=192.168.1.130 sport=80 dport=58152 packets=49 bytes=66340 [ASSURED] mark=0 use=1
>
> While trying to display real-time accounting statistics, we require
> to pool the kernel periodically to obtain this information. This is
> OK if the number of flows is relatively low. However, in case that
> the number of flows is huge, we can spend a considerable amount of
> cycles to iterate over the list of flows that have been obtained.
>
> Moreover, if we want to obtain the sum of the flow accounting results
> that match some criteria, we have to iterate over the whole list of
> existing flows, look for matchings and update the counters.
>
> This patch adds the extended accounting infrastructure for
> nfnetlink which aims to allow displaying real-time traffic accounting
> without the need of complicated and resource-consuming implementation
> in user-space. Basically, this new infrastructure allows you to create
> accounting objects. One accounting object is composed of packet and
> byte counters.
>
> In order to manipulate create accounting objects, you require the
> new libnetfilter_acct library. It contains several examples of use:
>
> libnetfilter_acct/examples# ./nfacct-add http-traffic
> libnetfilter_acct/examples# ./nfacct-get
> http-traffic = { pkts = 000000000000,   bytes = 000000000000 };
>
> Then, you can use one of this accounting objects in several iptables
> rules using the new NFACCT target (which comes in a follow-up patch):
>
>  # iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name http-traffic
>  # iptables -I OUTPUT -p tcp --dport 80 -j NFACCT --nfacct-name http-traffic
>
> The idea is simple: if one packet matches the rule, the NFACCT target
> updates the counters.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  include/linux/netfilter/Kbuild           |    1 +
>  include/linux/netfilter/nfnetlink.h      |    3 +-
>  include/linux/netfilter/nfnetlink_acct.h |   34 +++
>  net/netfilter/Kconfig                    |    8 +
>  net/netfilter/Makefile                   |    1 +
>  net/netfilter/nfnetlink_acct.c           |  352 ++++++++++++++++++++++++++++++
>  6 files changed, 398 insertions(+), 1 deletions(-)
>  create mode 100644 include/linux/netfilter/nfnetlink_acct.h
>  create mode 100644 net/netfilter/nfnetlink_acct.c
>
> diff --git a/include/linux/netfilter/Kbuild b/include/linux/netfilter/Kbuild
> index a1b410c..8995867 100644
> --- a/include/linux/netfilter/Kbuild
> +++ b/include/linux/netfilter/Kbuild
> @@ -6,6 +6,7 @@ header-y += nf_conntrack_sctp.h
>  header-y += nf_conntrack_tcp.h
>  header-y += nf_conntrack_tuple_common.h
>  header-y += nfnetlink.h
> +header-y += nfnetlink_acct.h
>  header-y += nfnetlink_compat.h
>  header-y += nfnetlink_conntrack.h
>  header-y += nfnetlink_log.h
> diff --git a/include/linux/netfilter/nfnetlink.h b/include/linux/netfilter/nfnetlink.h
> index 74d3386..b64454c 100644
> --- a/include/linux/netfilter/nfnetlink.h
> +++ b/include/linux/netfilter/nfnetlink.h
> @@ -48,7 +48,8 @@ struct nfgenmsg {
>  #define NFNL_SUBSYS_ULOG               4
>  #define NFNL_SUBSYS_OSF                        5
>  #define NFNL_SUBSYS_IPSET              6
> -#define NFNL_SUBSYS_COUNT              7
> +#define NFNL_SUBSYS_ACCT               7
> +#define NFNL_SUBSYS_COUNT              8
>
>  #ifdef __KERNEL__
>
> diff --git a/include/linux/netfilter/nfnetlink_acct.h b/include/linux/netfilter/nfnetlink_acct.h
> new file mode 100644
> index 0000000..9a1a119
> --- /dev/null
> +++ b/include/linux/netfilter/nfnetlink_acct.h
> @@ -0,0 +1,34 @@
> +#ifndef _NFNL_ACCT_H_
> +#define _NFNL_ACCT_H_
> +#include <linux/netfilter/nfnetlink.h>
> +
> +#define NFACCT_NAME_MAX                64
> +
> +enum nfnl_acct_msg_types {
> +       NFNL_MSG_ACCT_NEW,
> +       NFNL_MSG_ACCT_GET,
> +       NFNL_MSG_ACCT_GET_CTRZERO,
> +       NFNL_MSG_ACCT_DEL,
> +       NFNL_MSG_ACCT_MAX
> +};
> +
> +enum nfnl_acct_type {
> +       NFACCT_UNSPEC,
> +       NFACCT_NAME,
> +       NFACCT_PKTS,
> +       NFACCT_BYTES,
> +       __NFACCT_MAX
> +};
> +#define NFACCT_MAX (__NFACCT_MAX - 1)
> +
> +#ifdef __KERNEL__
> +
> +struct nf_acct;
> +
> +extern struct nf_acct *nfnl_acct_find_get(const char *filter_name);
> +extern void nfnl_acct_put(struct nf_acct *acct);
> +extern void nfnl_acct_update(const struct sk_buff *skb, struct nf_acct *nfacct);
> +
> +#endif /* __KERNEL__ */
> +
> +#endif /* _NFNL_ACCT_H */
> diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
> index d5597b7..77326ac 100644
> --- a/net/netfilter/Kconfig
> +++ b/net/netfilter/Kconfig
> @@ -4,6 +4,14 @@ menu "Core Netfilter Configuration"
>  config NETFILTER_NETLINK
>        tristate
>
> +config NETFILTER_NETLINK_ACCT
> +tristate "Netfilter NFACCT over NFNETLINK interface"
> +       depends on NETFILTER_ADVANCED
> +       select NETFILTER_NETLINK
> +       help
> +         If this option is enabled, the kernel will include support
> +         for extended accounting via NFNETLINK.
> +
>  config NETFILTER_NETLINK_QUEUE
>        tristate "Netfilter NFQUEUE over NFNETLINK interface"
>        depends on NETFILTER_ADVANCED
> diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
> index 1a02853..4da1c87 100644
> --- a/net/netfilter/Makefile
> +++ b/net/netfilter/Makefile
> @@ -7,6 +7,7 @@ nf_conntrack-$(CONFIG_NF_CONNTRACK_EVENTS) += nf_conntrack_ecache.o
>  obj-$(CONFIG_NETFILTER) = netfilter.o
>
>  obj-$(CONFIG_NETFILTER_NETLINK) += nfnetlink.o
> +obj-$(CONFIG_NETFILTER_NETLINK_ACCT) += nfnetlink_acct.o
>  obj-$(CONFIG_NETFILTER_NETLINK_QUEUE) += nfnetlink_queue.o
>  obj-$(CONFIG_NETFILTER_NETLINK_LOG) += nfnetlink_log.o
>
> diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
> new file mode 100644
> index 0000000..3ec407f
> --- /dev/null
> +++ b/net/netfilter/nfnetlink_acct.c
> @@ -0,0 +1,352 @@
> +/*
> + * (C) 2011 Pablo Neira Ayuso <pablo@netfilter.org>
> + * (C) 2011 Intra2net AG <http://www.intra2net.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation (or any later at your option).
> + */
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/skbuff.h>
> +#include <linux/netlink.h>
> +#include <linux/rculist.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/errno.h>
> +#include <net/netlink.h>
> +#include <net/sock.h>
> +#include <asm/atomic.h>
> +
> +#include <linux/netfilter.h>
> +#include <linux/netfilter/nfnetlink.h>
> +#include <linux/netfilter/nfnetlink_acct.h>
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Pablo Neira Ayuso <pablo@netfilter.org>");
> +MODULE_DESCRIPTION("nfacct: Extended Netfilter accounting infrastructure");
> +
> +static LIST_HEAD(nfnl_acct_list);
> +
> +struct nf_acct {
> +       struct rcu_head         rcu_head;
> +       struct list_head        head;
> +       spinlock_t              lock;   /* to update the counters. */
> +       atomic_t                refcnt;
> +
> +       char                    name[NFACCT_NAME_MAX];
> +       __u64                   pkts;
> +       __u64                   bytes;
> +};
> +
> +static void nfnl_acct_free_rcu(struct rcu_head *rcu_head)
> +{
> +       struct nf_acct *acct = container_of(rcu_head, struct nf_acct, rcu_head);
> +
> +       kfree(acct);
> +}
> +
> +static int
> +nfnl_acct_new(struct sock *nfnl, struct sk_buff *skb,
> +            const struct nlmsghdr *nlh, const struct nlattr * const tb[])
> +{
> +       int ret;
> +       struct nf_acct *acct, *matching = NULL;
> +       char *acct_name;
> +
> +       if (!tb[NFACCT_NAME])
> +               return -EINVAL;
> +
> +       acct_name = nla_data(tb[NFACCT_NAME]);
> +
> +       rcu_read_lock();
> +       list_for_each_entry(acct, &nfnl_acct_list, head) {
> +               if (strncmp(acct->name, acct_name, NFACCT_NAME_MAX) != 0)
> +                       continue;
> +
> +                if (nlh->nlmsg_flags & NLM_F_EXCL) {
> +                       rcu_read_unlock();
> +                       ret = -EEXIST;
> +                       goto err;
> +               }
> +               matching = acct;
> +        }
> +       rcu_read_unlock();
> +
> +       acct = kzalloc(sizeof(struct nf_acct), GFP_KERNEL);
> +       if (acct == NULL) {
> +               ret = -ENOMEM;
> +               goto err;
> +       }
> +       spin_lock_init(&acct->lock);
> +       strncpy(acct->name, nla_data(tb[NFACCT_NAME]), NFACCT_NAME_MAX);
> +       if (tb[NFACCT_BYTES])
> +               acct->bytes = be64_to_cpu(nla_get_u64(tb[NFACCT_BYTES]));
> +       if (tb[NFACCT_PKTS])
> +               acct->pkts = be64_to_cpu(nla_get_u64(tb[NFACCT_PKTS]));
> +
> +       atomic_inc(&acct->refcnt);
> +
> +       /* We are protected by nfnl mutex. */
> +       if (matching) {
> +               list_del_rcu(&matching->head);
> +               if (atomic_dec_and_test(&matching->refcnt))
> +                       call_rcu(&matching->rcu_head, nfnl_acct_free_rcu);
> +
> +       }
> +       list_add_tail_rcu(&acct->head, &nfnl_acct_list);
> +       return 0;
> +err:
> +       return ret;
> +}
> +
> +static int
> +nfnl_acct_fill_info(struct sk_buff *skb, u32 pid, u32 seq,
> +                  int event, struct nf_acct *acct)
> +{
> +       struct nlmsghdr *nlh;
> +       struct nfgenmsg *nfmsg;
> +       unsigned int flags = pid ? NLM_F_MULTI : 0;
> +
> +       event |= NFNL_SUBSYS_ACCT << 8;
> +       nlh = nlmsg_put(skb, pid, seq, event, sizeof(*nfmsg), flags);
> +       if (nlh == NULL)
> +               goto nlmsg_failure;
> +
> +       nfmsg = nlmsg_data(nlh);
> +       nfmsg->nfgen_family = AF_UNSPEC;
> +       nfmsg->version = NFNETLINK_V0;
> +       nfmsg->res_id = 0;
> +
> +       NLA_PUT_STRING(skb, NFACCT_NAME, acct->name);
> +       NLA_PUT_BE64(skb, NFACCT_PKTS, cpu_to_be64(acct->pkts));
> +       NLA_PUT_BE64(skb, NFACCT_BYTES, cpu_to_be64(acct->bytes));
> +
> +       nlmsg_end(skb, nlh);
> +       return skb->len;
> +
> +nlmsg_failure:
> +nla_put_failure:
> +       nlmsg_cancel(skb, nlh);
> +       return -1;
> +}
> +
> +static int
> +nfnl_acct_dump(struct sk_buff *skb, struct netlink_callback *cb)
> +{
> +       struct nf_acct *cur, *last;
> +
> +       if (cb->args[2])
> +               return 0;
> +
> +       last = (struct nf_acct *)cb->args[1];
> +       if (cb->args[1])
> +               cb->args[1] = 0;
> +
> +       rcu_read_lock();
> +       list_for_each_entry(cur, &nfnl_acct_list, head) {
> +               if (last && cur != last)
> +                       continue;
> +
> +               if (nfnl_acct_fill_info(skb, NETLINK_CB(cb->skb).pid,
> +                                      cb->nlh->nlmsg_seq,
> +                                      NFNL_MSG_ACCT_NEW, cur) < 0) {
> +                       cb->args[1] = (unsigned long)cur;
> +                       break;
> +               }
> +
> +               if (NFNL_MSG_TYPE(cb->nlh->nlmsg_type) ==
> +                                               NFNL_MSG_ACCT_GET_CTRZERO) {
> +                       spin_lock_bh(&cur->lock);
> +                       cur->pkts = 0;
> +                       cur->bytes = 0;
> +                       spin_unlock_bh(&cur->lock);
> +               }
> +       }
> +       if (!cb->args[1])
> +               cb->args[2] = 1;
> +       rcu_read_unlock();
> +       return skb->len;
> +}
> +
> +static int
> +nfnl_acct_get(struct sock *nfnl, struct sk_buff *skb,
> +            const struct nlmsghdr *nlh, const struct nlattr * const tb[])
> +{
> +       int ret = 0;
> +       struct nf_acct *cur;
> +       char *acct_name;
> +
> +       if (nlh->nlmsg_flags & NLM_F_DUMP) {
> +               return netlink_dump_start(nfnl, skb, nlh, nfnl_acct_dump,
> +                                         NULL, 0);
> +       }
> +
> +       if (!tb[NFACCT_NAME])
> +               return -EINVAL;
> +       acct_name = nla_data(tb[NFACCT_NAME]);
> +
> +       rcu_read_lock();
> +       list_for_each_entry(cur, &nfnl_acct_list, head) {
> +               struct sk_buff *skb2;
> +
> +               if (strncmp(cur->name, acct_name, NFACCT_NAME_MAX)!= 0)
> +                       continue;
> +
> +               skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> +               if (skb2 == NULL)
> +                       break;
> +
> +               ret = nfnl_acct_fill_info(skb2, NETLINK_CB(skb).pid,
> +                                        nlh->nlmsg_seq,
> +                                        NFNL_MSG_ACCT_NEW, cur);
> +               if (ret <= 0)
> +                       kfree_skb(skb2);
> +
> +               if (NFNL_MSG_TYPE(nlh->nlmsg_type) ==
> +                                               NFNL_MSG_ACCT_GET_CTRZERO) {
> +                       spin_lock_bh(&cur->lock);
> +                       cur->pkts = 0;
> +                       cur->bytes = 0;
> +                       spin_unlock_bh(&cur->lock);
> +               }
> +               break;
> +       }
> +       rcu_read_unlock();
> +       return ret;
> +}
> +
> +static int
> +nfnl_acct_del(struct sock *nfnl, struct sk_buff *skb,
> +            const struct nlmsghdr *nlh, const struct nlattr * const tb[])
> +{
> +       char *acct_name;
> +       struct nf_acct *cur;
> +       int ret = -ENOENT;
> +
> +       if (!tb[NFACCT_NAME]) {
> +               rcu_read_lock();
> +               list_for_each_entry(cur, &nfnl_acct_list, head) {
> +                       /* We are protected by nfnl mutex. */
> +                       list_del_rcu(&cur->head);
> +                       if (atomic_dec_and_test(&cur->refcnt))
> +                               call_rcu(&cur->rcu_head, nfnl_acct_free_rcu);
> +               }
> +               rcu_read_lock();
> +               return 0;
> +       }
> +       acct_name = nla_data(tb[NFACCT_NAME]);
> +
> +       rcu_read_lock();
> +       list_for_each_entry(cur, &nfnl_acct_list, head) {
> +               if (strncmp(cur->name, acct_name, NFACCT_NAME_MAX) != 0)
> +                       continue;
> +
> +               /* We are protected by nfnl mutex. */
> +               list_del_rcu(&cur->head);
> +               if (atomic_dec_and_test(&cur->refcnt))
> +                       call_rcu(&cur->rcu_head, nfnl_acct_free_rcu);
> +               ret = 0;
> +               break;
> +       }
> +       rcu_read_lock();
> +       return ret;
> +}
> +
> +static const struct nla_policy nfnl_acct_policy[NFACCT_MAX+1] = {
> +       [NFACCT_NAME] = { .type = NLA_NUL_STRING, .len = NFACCT_NAME_MAX-1 },
> +       [NFACCT_BYTES] = { .type = NLA_U64 },
> +       [NFACCT_PKTS] = { .type = NLA_U64 },
> +};
> +
> +static const struct nfnl_callback nfnl_acct_cb[NFNL_MSG_ACCT_MAX] = {
> +       [NFNL_MSG_ACCT_NEW]             = { .call = nfnl_acct_new,
> +                                           .attr_count = NFACCT_MAX,
> +                                           .policy = nfnl_acct_policy },
> +       [NFNL_MSG_ACCT_GET]             = { .call = nfnl_acct_get,
> +                                           .attr_count = NFACCT_MAX,
> +                                           .policy = nfnl_acct_policy },
> +       [NFNL_MSG_ACCT_GET_CTRZERO]     = { .call = nfnl_acct_get,
> +                                           .attr_count = NFACCT_MAX,
> +                                           .policy = nfnl_acct_policy },
> +       [NFNL_MSG_ACCT_DEL]             = { .call = nfnl_acct_del,
> +                                           .attr_count = NFACCT_MAX,
> +                                           .policy = nfnl_acct_policy },
> +};
> +
> +static const struct nfnetlink_subsystem nfnl_acct_subsys = {
> +       .name                           = "acct",
> +       .subsys_id                      = NFNL_SUBSYS_ACCT,
> +       .cb_count                       = NFNL_MSG_ACCT_MAX,
> +       .cb                             = nfnl_acct_cb,
> +};
> +
> +MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_ACCT);
> +
> +struct nf_acct *nfnl_acct_find_get(const char *acct_name)
> +{
> +       struct nf_acct *cur, *acct = NULL;
> +
> +       rcu_read_lock();
> +       list_for_each_entry(cur, &nfnl_acct_list, head) {
> +               if (strncmp(cur->name, acct_name, NFACCT_NAME_MAX)!= 0)
> +                       continue;
> +
> +               acct = cur;
> +               atomic_inc(&acct->refcnt);
> +               break;
> +       }
> +       rcu_read_unlock();
> +       return acct;
> +}
> +EXPORT_SYMBOL_GPL(nfnl_acct_find_get);
> +
> +void nfnl_acct_put(struct nf_acct *acct)
> +{
> +       if (atomic_dec_and_test(&acct->refcnt))
> +               call_rcu(&acct->rcu_head, nfnl_acct_free_rcu);
> +}
> +EXPORT_SYMBOL_GPL(nfnl_acct_put);
> +
> +void nfnl_acct_update(const struct sk_buff *skb, struct nf_acct *nfacct)
> +{
> +       spin_lock_bh(&nfacct->lock);
> +       nfacct->pkts++;
> +       nfacct->bytes += skb->len;
> +       spin_unlock_bh(&nfacct->lock);
> +}
> +EXPORT_SYMBOL_GPL(nfnl_acct_update);
> +
> +static int __init nfnl_acct_init(void)
> +{
> +       int ret;
> +
> +       pr_info("nfnl_acct: registering with nfnetlink.\n");
> +       ret = nfnetlink_subsys_register(&nfnl_acct_subsys);
> +       if (ret < 0) {
> +               pr_err("nfnl_acct_init: cannot register with nfnetlink.\n");
> +               goto err_out;
> +       }
> +       return 0;
> +err_out:
> +       return ret;
> +}
> +
> +static void __exit nfnl_acct_exit(void)
> +{
> +       struct nf_acct *cur, *tmp;
> +
> +       pr_info("nfnl_acct: unregistering from nfnetlink.\n");
> +       nfnetlink_subsys_unregister(&nfnl_acct_subsys);
> +
> +       /* if we can remove this module, it means that it has no clients. */
> +       list_for_each_entry_safe(cur, tmp, &nfnl_acct_list, head) {
> +               list_del_rcu(&cur->head);
> +               if (atomic_dec_and_test(&cur->refcnt))
> +                       kfree(cur);
> +       }
> +}
> +
> +module_init(nfnl_acct_init);
> +module_exit(nfnl_acct_exit);
> --
> 1.7.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Eric Dumazet]

Le mercredi 14 décembre 2011 à 19:19 +0530, Anand Raj Manickam a écrit :
> A clarification :
> If its about flow based accounting , wont the below rules double the counter ?
> # iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name
> http-traffic# iptables -I OUTPUT -p tcp --dport 80 -j NFACCT
> --nfacct-name http-traffic
> 
> 

Basically yes (if you want to account both input and output trafic)

If you want to separate counters, use :

# iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name http-traffic-in

# iptables -I OUTPUT -p tcp --dport 80 -j NFACCT --nfacct-name http-traffic-out

?


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] [RFC] Extended accounting infrastructure for iptables

[Changli Gao]

On Wed, Dec 14, 2011 at 9:30 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Wed, Dec 14, 2011 at 09:12:52PM +0800, Changli Gao wrote:
>>
>> Why not use the counters of iptables instead?
>>
>> iptables-save -c
>
> If you want to obtain the sum of the counters that match some criteria,
> you have to iterate over the whole list of existing rules, look for
> matchings and update the counters.

As I said in another thread, you can redirect the traffic to a
separated chain, and use the counters of that chain.

>
> Moreover, if you have a large rule-set, polling periodically
> iptables-save -c can be expensive.

I got it. Thanks. Maybe we can index the entries in the kernel, and
add a new interface to get the counters of a special entry with a
entry ID.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

Re: [PATCH 0/2] [RFC] Extended accounting infrastructure for iptables

[Jan Engelhardt]

On Wednesday 2011-12-14 15:52, Changli Gao wrote:

>On Wed, Dec 14, 2011 at 9:30 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>> On Wed, Dec 14, 2011 at 09:12:52PM +0800, Changli Gao wrote:
>>>
>>> Why not use the counters of iptables instead?
>>>
>>> iptables-save -c
>>
>> If you want to obtain the sum of the counters that match some criteria,
>> you have to iterate over the whole list of existing rules, look for
>> matchings and update the counters.
>
>As I said in another thread, you can redirect the traffic to a
>separated chain, and use the counters of that chain.

UDCs (user defined chains) don't have counters, though.

>> Moreover, if you have a large rule-set, polling periodically
>> iptables-save -c can be expensive.
>
>I got it. Thanks. Maybe we can index the entries in the kernel, and
>add a new interface to get the counters of a special entry with a
>entry ID.

Relying on the rule number is a terrible idea (just like 
iptables-save|head -n5|tail -n1 would be). Unique persistend IDs are 
unfavorable as well; names, as used with xt_quota2/xt_NFACCT can be 
remembered much more easily.

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Patrick McHardy]

On 14.12.2011 14:18, Pablo Neira Ayuso wrote:
> On Wed, Dec 14, 2011 at 12:23:21PM +0100, Patrick McHardy wrote:
> [...]
>>> +nfnl_acct_new(struct sock *nfnl, struct sk_buff *skb,
>>> +	     const struct nlmsghdr *nlh, const struct nlattr * const tb[])
>>> +{
>>> +	int ret;
>>> +	struct nf_acct *acct, *matching = NULL;
>>> +	char *acct_name;
>>> +
>>> +	if (!tb[NFACCT_NAME])
>>> +		return -EINVAL;
>>> +
>>> +	acct_name = nla_data(tb[NFACCT_NAME]);
>>> +
>>> +	rcu_read_lock();
>>> +	list_for_each_entry(acct,&nfnl_acct_list, head) {
>>
>> I don't really get the locking concept. All netlink operations happen under
>> the nfnl mutex, so using RCU for the lookup shouldn't be necessary
>> (applied to all netlink operations).
> 
> If you add one iptables rule with NFACCT, you have to iterate over the
> list (without the mutex). Very unlikely, but we may delete one
> accounting object via nfnetlink at the same time of adding the rule
> that refers to it.

Sure. But we don't need it for any of the netlink operations.

> 
>>> +        }
>>> +	rcu_read_unlock();
>>> +
>>> +	acct = kzalloc(sizeof(struct nf_acct), GFP_KERNEL);
>>> +	if (acct == NULL) {
>>> +		ret = -ENOMEM;
>>> +		goto err;
>>> +	}
>>> +	spin_lock_init(&acct->lock);
>>> +	strncpy(acct->name, nla_data(tb[NFACCT_NAME]), NFACCT_NAME_MAX);
>>> +	if (tb[NFACCT_BYTES])
>>> +		acct->bytes = be64_to_cpu(nla_get_u64(tb[NFACCT_BYTES]));
>>> +	if (tb[NFACCT_PKTS])
>>> +		acct->pkts = be64_to_cpu(nla_get_u64(tb[NFACCT_PKTS]));
>>> +
>>> +	atomic_inc(&acct->refcnt);
>>> +
>>> +	/* We are protected by nfnl mutex. */
>>> +	if (matching) {
>>
>> This seems to be a replace operation, so I think you should
>> require NLM_F_REPLACE. Also it seems you could just
>> reinitialize the existing counter instead of unconditionally
>> allocating a new one.
> 
> I think it's easier to return -EBUSY as you suggested.

That was for deletion. For addition supporting replace is fine, but
we should use the correct netlink flags.

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Pablo Neira Ayuso]

On Wed, Dec 14, 2011 at 02:43:40PM +0100, Jan Engelhardt wrote:
> On Wednesday 2011-12-14 12:00, pablo@netfilter.org wrote:
> 
> >Then, you can use one of this accounting objects in several iptables
> >rules using the new NFACCT target (which comes in a follow-up patch):
> >
> > # iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name http-traffic
> > # iptables -I OUTPUT -p tcp --dport 80 -j NFACCT --nfacct-name http-traffic
> >
> >The idea is simple: if one packet matches the rule, the NFACCT target
> >updates the counters.
> 
> This smells a lot like -m quota2 --grow, except that yours uses 
> netlink instead of procfs and can only update the counters.
> 
> I suggest to turn -j NFACCT into -m nfacct instead, so that we can add 
> counting-down mode and matching capabilities, so as to replace 
> xt_quota*.

This makes sense.

My only concern is that -m nfacct will not really match anything (not
by default at least).

But with -m nfacct, we can use it in one single multi-match rule, which
comes in handy.

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Jozsef Kadlecsik]

On Wed, 14 Dec 2011, Pablo Neira Ayuso wrote:

> On Wed, Dec 14, 2011 at 02:43:40PM +0100, Jan Engelhardt wrote:
> > On Wednesday 2011-12-14 12:00, pablo@netfilter.org wrote:
> > 
> > >Then, you can use one of this accounting objects in several iptables
> > >rules using the new NFACCT target (which comes in a follow-up patch):
> > >
> > > # iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name http-traffic
> > > # iptables -I OUTPUT -p tcp --dport 80 -j NFACCT --nfacct-name http-traffic
> > >
> > >The idea is simple: if one packet matches the rule, the NFACCT target
> > >updates the counters.
> > 
> > This smells a lot like -m quota2 --grow, except that yours uses 
> > netlink instead of procfs and can only update the counters.
> > 
> > I suggest to turn -j NFACCT into -m nfacct instead, so that we can add 
> > counting-down mode and matching capabilities, so as to replace 
> > xt_quota*.
> 
> This makes sense.
> 
> My only concern is that -m nfacct will not really match anything (not
> by default at least).
> 
> But with -m nfacct, we can use it in one single multi-match rule, which
> comes in handy.

I second that turning it into a "match" makes it more flexible.

Best regards,
Joysef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH 0/2] [RFC] Extended accounting infrastructure for iptables

[Pete Holland]

This is a great idea, thank you!  This is a problem I'm trying to
solve right now and have found that using the rule counters is clunky
and restrictive.  Specifically, this provides the kind of
functionality that's needed to support changing the rule definitions
without losing the accounting information.

Quick question, would this be limited to the filter table, or would we
be able to use the target (or match as I saw suggested) in other
tables?
This would be most useful if it were available in the mangle (so we
could mark+account in one spot... and if it were a match, in one rule)
and nat tables (so we can account for traffic pre and post NAT
treatment).

- Pete

On Wed, Dec 14, 2011 at 3:00 AM,  <pablo@netfilter.org> wrote:
> From: Pablo Neira Ayuso <pablo@netfilter.org>
>
> Hi!
>
> We currently have two ways to account traffic in netfilter:
>
> - iptables chain and rule counters:
>
>  # iptables -L -n -v
> Chain INPUT (policy DROP 3 packets, 867 bytes)
>  pkts bytes target     prot opt in     out     source               destinat
>    8  1104 ACCEPT     all  --  lo     *       0.0.0.0/0            0.0.0.0/
>
> - use flow-based accounting provided by ctnetlink:
>
>  # conntrack -L
> tcp      6 431999 ESTABLISHED src=192.168.1.130 dst=212.106.219.168 sport=58
>
> While trying to display real-time accounting statistics, we require
> to pool the kernel periodically to obtain this information. This is
> OK if the number of flows is relatively low. However, in case that
> the number of flows is huge, we can spend a considerable amount of
> cycles to iterate over the list of flows that have been obtained.
>
> Moreover, if we want to obtain the sum of the flow accounting results
> that match some criteria, we have to iterate over the whole list of
> existing flows, look for matchings and update the counters.
>
> This patchset adds the extended accounting infrastructure in
> kernel-space. It is composed of one nfnetlink interface that
> allows you to create, to update and to retrieve accounting objects.
> These objects can be used to account traffic with the flexibility
> that iptables rules provide (by means of the new NFACCT target).
>
> Quick example of use:
>
> 1) You create the accounting object:
>
> libnetfilter_acct/examples# ./nfacct-add http-traffic
>
> 2) Add the iptables rules for traffic you want to account:
>
> # iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name http-traffic
> # iptables -I OUTPUT -p tcp --dport 80 -j NFACCT --nfacct-name http-traffic
>
> 3) Retrieve the counters.
>
> libnetfilter_acct/examples# ./nfacct-get
> http-traffic = { pkts = 000000000125,   bytes = 000000023162 };
>
> You can also atomically retrieve and zero counters in case that you
> want to collect information periodically.
>
> This comes with the user-space libnetfilter_acct library:
>
> http://1984.lsi.us.es/git/libnetfilter_acct/
>
> [Note: The current library API is still quite preliminary. I plan
> to heavily rework it]
>
> And the NFACCT extension for user-space iptables is here:
>
> http://1984.lsi.us.es/git/iptables/commit/?id=850104ff9d619a7e0bc561aa16fee838fcd1937c
>
> Comments welcome!
>
> Pablo Neira Ayuso (2):
>  netfilter: add extended accounting infrastructure over nfnetlink
>  netfilter: xtables: add NFACCT target to support extended accounting
>
>  include/linux/netfilter/Kbuild           |    2 +
>  include/linux/netfilter/nfnetlink.h      |    3 +-
>  include/linux/netfilter/nfnetlink_acct.h |   34 +++
>  include/linux/netfilter/xt_NFACCT.h      |   17 ++
>  net/netfilter/Kconfig                    |   18 ++
>  net/netfilter/Makefile                   |    2 +
>  net/netfilter/nfnetlink_acct.c           |  352 ++++++++++++++++++++++++++++++
>  net/netfilter/xt_NFACCT.c                |   78 +++++++
>  8 files changed, 505 insertions(+), 1 deletions(-)
>  create mode 100644 include/linux/netfilter/nfnetlink_acct.h
>  create mode 100644 include/linux/netfilter/xt_NFACCT.h
>  create mode 100644 net/netfilter/nfnetlink_acct.c
>  create mode 100644 net/netfilter/xt_NFACCT.c
>
> --
> 1.7.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Maciej Żenczykowski]

>> > I suggest to turn -j NFACCT into -m nfacct instead, so that we can add
>> > counting-down mode and matching capabilities, so as to replace
>> > xt_quota*.
>>
>> This makes sense.
>>
>> My only concern is that -m nfacct will not really match anything (not
>> by default at least).
>>
>> But with -m nfacct, we can use it in one single multi-match rule, which
>> comes in handy.
>
> I second that turning it into a "match" makes it more flexible.

I've often wished I could apply multiple targets to a single rule, ie.
mangle like so, and then ACCEPT, instead of having to create a
separate chain...
It sounds like there should be matches, targets, and non-decisive
actions, which happen after the matches, don't affect matching, and
before the targets...

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Pablo Neira Ayuso]

On Wed, Dec 14, 2011 at 05:31:07PM +0100, Patrick McHardy wrote:
> On 14.12.2011 14:18, Pablo Neira Ayuso wrote:
> > On Wed, Dec 14, 2011 at 12:23:21PM +0100, Patrick McHardy wrote:
> > [...]
> >>> +nfnl_acct_new(struct sock *nfnl, struct sk_buff *skb,
> >>> +	     const struct nlmsghdr *nlh, const struct nlattr * const tb[])
> >>> +{
> >>> +	int ret;
> >>> +	struct nf_acct *acct, *matching = NULL;
> >>> +	char *acct_name;
> >>> +
> >>> +	if (!tb[NFACCT_NAME])
> >>> +		return -EINVAL;
> >>> +
> >>> +	acct_name = nla_data(tb[NFACCT_NAME]);
> >>> +
> >>> +	rcu_read_lock();
> >>> +	list_for_each_entry(acct,&nfnl_acct_list, head) {
> >>
> >> I don't really get the locking concept. All netlink operations happen under
> >> the nfnl mutex, so using RCU for the lookup shouldn't be necessary
> >> (applied to all netlink operations).
> > 
> > If you add one iptables rule with NFACCT, you have to iterate over the
> > list (without the mutex). Very unlikely, but we may delete one
> > accounting object via nfnetlink at the same time of adding the rule
> > that refers to it.
> 
> Sure. But we don't need it for any of the netlink operations.

Indeed. I have removed all rcu_read_[un]lock in the list iterations of
the netlink operations.

> >>> +        }
> >>> +	rcu_read_unlock();
> >>> +
> >>> +	acct = kzalloc(sizeof(struct nf_acct), GFP_KERNEL);
> >>> +	if (acct == NULL) {
> >>> +		ret = -ENOMEM;
> >>> +		goto err;
> >>> +	}
> >>> +	spin_lock_init(&acct->lock);
> >>> +	strncpy(acct->name, nla_data(tb[NFACCT_NAME]), NFACCT_NAME_MAX);
> >>> +	if (tb[NFACCT_BYTES])
> >>> +		acct->bytes = be64_to_cpu(nla_get_u64(tb[NFACCT_BYTES]));
> >>> +	if (tb[NFACCT_PKTS])
> >>> +		acct->pkts = be64_to_cpu(nla_get_u64(tb[NFACCT_PKTS]));
> >>> +
> >>> +	atomic_inc(&acct->refcnt);
> >>> +
> >>> +	/* We are protected by nfnl mutex. */
> >>> +	if (matching) {
> >>
> >> This seems to be a replace operation, so I think you should
> >> require NLM_F_REPLACE. Also it seems you could just
> >> reinitialize the existing counter instead of unconditionally
> >> allocating a new one.
> > 
> > I think it's easier to return -EBUSY as you suggested.
> 
> That was for deletion. For addition supporting replace is fine, but
> we should use the correct netlink flags.

This is what I'm doing now in this case:

        if (matching) {
                if (nlh->nlmsg_flags & NLM_F_REPLACE) {
                        /* reset counters if you request a replacement. */
                        atomic64_set(&cur->pkts, 0);
                        atomic64_set(&cur->bytes, 0);
                        return 0;
                }
                        return -EBUSY;
        }

before that code (in the lookup) you have:

                if (nlh->nlmsg_flags & NLM_F_EXCL)
                        return -EEXIST;

So basically, if one object already exists and you try to create it:

1) with NLM_F_EXCL, you hit EEXIST.

1) with MLM_F_REPLACE, it sets counters to zero.

2) without NLM_F_EXCL and NLM_F_REPLACE, you hit EBUSY.

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Pablo Neira Ayuso]

On Wed, Dec 14, 2011 at 07:30:29PM +0100, Jozsef Kadlecsik wrote:
> On Wed, 14 Dec 2011, Pablo Neira Ayuso wrote:
> 
> > On Wed, Dec 14, 2011 at 02:43:40PM +0100, Jan Engelhardt wrote:
> > > On Wednesday 2011-12-14 12:00, pablo@netfilter.org wrote:
> > > 
> > > >Then, you can use one of this accounting objects in several iptables
> > > >rules using the new NFACCT target (which comes in a follow-up patch):
> > > >
> > > > # iptables -I INPUT -p tcp --sport 80 -j NFACCT --nfacct-name http-traffic
> > > > # iptables -I OUTPUT -p tcp --dport 80 -j NFACCT --nfacct-name http-traffic
> > > >
> > > >The idea is simple: if one packet matches the rule, the NFACCT target
> > > >updates the counters.
> > > 
> > > This smells a lot like -m quota2 --grow, except that yours uses 
> > > netlink instead of procfs and can only update the counters.
> > > 
> > > I suggest to turn -j NFACCT into -m nfacct instead, so that we can add 
> > > counting-down mode and matching capabilities, so as to replace 
> > > xt_quota*.
> > 
> > This makes sense.
> > 
> > My only concern is that -m nfacct will not really match anything (not
> > by default at least).
> > 
> > But with -m nfacct, we can use it in one single multi-match rule, which
> > comes in handy.
> 
> I second that turning it into a "match" makes it more flexible.

I'll make it.

Probably we can add some --nfacct NAME as shortcut for -m nfacct
--nfacct-name NAME, to hide that this is a match? Hm, probably too
nasty.

I have concerns about the fact that this wil not really match
anything (although it is going to (ab)use the match infrastructure.

This makes me think that we probably need that multitarget (for those
that just return to continue with the rule traversal in the chain).

Just wild thoughts. The quick way is to make this a match of course.

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Jan Engelhardt]

On Thursday 2011-12-15 13:26, Pablo Neira Ayuso wrote:
>> 
>> I second that turning it into a "match" makes it more flexible.
>
>I'll make it.
>
>Probably we can add some --nfacct NAME as shortcut for -m nfacct
>--nfacct-name NAME, to hide that this is a match? Hm, probably too
>nasty.

You decide. --nfacct will already be automatically accepted by getopt as an
abbreviation (provided it the unique solution to /^--nfacct*/).

Re: [PATCH 0/2] [RFC] Extended accounting infrastructure for iptables

[Pablo Neira Ayuso]

On Wed, Dec 14, 2011 at 11:29:42AM -0800, Pete Holland wrote:
> This is a great idea, thank you!  This is a problem I'm trying to
> solve right now and have found that using the rule counters is clunky
> and restrictive.  Specifically, this provides the kind of
> functionality that's needed to support changing the rule definitions
> without losing the accounting information.
> 
> Quick question, would this be limited to the filter table, or would we
> be able to use the target (or match as I saw suggested) in other
> tables?
> This would be most useful if it were available in the mangle (so we
> could mark+account in one spot... and if it were a match, in one rule)
> and nat tables (so we can account for traffic pre and post NAT
> treatment).

No restriction regarding what table you want to use this.

P.S: please, don't top-post, that's the nf-ml policy.

Re: [PATCH 0/2] [RFC] Extended accounting infrastructure for iptables

[Ferenc Wagner]

Jan Engelhardt <jengelh@medozas.de> writes:

> On Wednesday 2011-12-14 15:52, Changli Gao wrote:
>
>> On Wed, Dec 14, 2011 at 9:30 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>>
>>> On Wed, Dec 14, 2011 at 09:12:52PM +0800, Changli Gao wrote:
>>>>
>>>> Why not use the counters of iptables instead?
>>>>
>>>> iptables-save -c
>>>
>>> If you want to obtain the sum of the counters that match some criteria,
>>> you have to iterate over the whole list of existing rules, look for
>>> matchings and update the counters.
>>
>> As I said in another thread, you can redirect the traffic to a
>> separated chain, and use the counters of that chain.
>
> UDCs (user defined chains) don't have counters, though.

So put an empty rule into them.  The ip_ plugin of Munin uses this
technique for quite some time.

>>> Moreover, if you have a large rule-set, polling periodically
>>> iptables-save -c can be expensive.
>>
>> I got it. Thanks. Maybe we can index the entries in the kernel, and
>> add a new interface to get the counters of a special entry with a
>> entry ID.
>
> Relying on the rule number is a terrible idea (just like 
> iptables-save|head -n5|tail -n1 would be). Unique persistend IDs are 
> unfavorable as well; names, as used with xt_quota2/xt_NFACCT can be 
> remembered much more easily.

Rule names could serve this, couldn't they?  And rules can be identified
by -m comment if batch processing is required.
-- 
Regards,
Feri.

Re: [PATCH 0/2] [RFC] Extended accounting infrastructure for iptables

[Jan Engelhardt]

On Thursday 2011-12-15 21:23, Ferenc Wagner wrote:

>Jan Engelhardt <jengelh@medozas.de> writes:
>
>> On Wednesday 2011-12-14 15:52, Changli Gao wrote:
>>
>>> On Wed, Dec 14, 2011 at 9:30 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>>>
>>>> On Wed, Dec 14, 2011 at 09:12:52PM +0800, Changli Gao wrote:
>>>>>
>>>>> Why not use the counters of iptables instead?
>>>>>
>>>>> iptables-save -c
>>>>
>>>> If you want to obtain the sum of the counters that match some criteria,
>>>> you have to iterate over the whole list of existing rules, look for
>>>> matchings and update the counters.
>>>
>>> As I said in another thread, you can redirect the traffic to a
>>> separated chain, and use the counters of that chain.
>>
>> UDCs (user defined chains) don't have counters, though.
>
>So put an empty rule into them.  The ip_ plugin of Munin uses this
>technique for quite some time.
>
>>>> Moreover, if you have a large rule-set, polling periodically
>>>> iptables-save -c can be expensive.
>>>
>>> I got it. Thanks. Maybe we can index the entries in the kernel, and
>>> add a new interface to get the counters of a special entry with a
>>> entry ID.
>>
>> Relying on the rule number is a terrible idea (just like 
>> iptables-save|head -n5|tail -n1 would be). Unique persistend IDs are 
>> unfavorable as well; names, as used with xt_quota2/xt_NFACCT can be 
>> remembered much more easily.
>
>Rule names could serve this, couldn't they?  And rules can be identified
>by -m comment if batch processing is required.

Then you can just as well use nfacct's name.

Re: [PATCH 0/2] [RFC] Extended accounting infrastructure for iptables

[Pablo Neira Ayuso]

On Thu, Dec 15, 2011 at 09:23:34PM +0100, Ferenc Wagner wrote:
> Jan Engelhardt <jengelh@medozas.de> writes:
> 
> > On Wednesday 2011-12-14 15:52, Changli Gao wrote:
> >
> >> On Wed, Dec 14, 2011 at 9:30 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >>
> >>> On Wed, Dec 14, 2011 at 09:12:52PM +0800, Changli Gao wrote:
> >>>>
> >>>> Why not use the counters of iptables instead?
> >>>>
> >>>> iptables-save -c
> >>>
> >>> If you want to obtain the sum of the counters that match some criteria,
> >>> you have to iterate over the whole list of existing rules, look for
> >>> matchings and update the counters.
> >>
> >> As I said in another thread, you can redirect the traffic to a
> >> separated chain, and use the counters of that chain.
> >
> > UDCs (user defined chains) don't have counters, though.
> 
> So put an empty rule into them.  The ip_ plugin of Munin uses this
> technique for quite some time.
> 
> >>> Moreover, if you have a large rule-set, polling periodically
> >>> iptables-save -c can be expensive.
> >>
> >> I got it. Thanks. Maybe we can index the entries in the kernel, and
> >> add a new interface to get the counters of a special entry with a
> >> entry ID.
> >
> > Relying on the rule number is a terrible idea (just like 
> > iptables-save|head -n5|tail -n1 would be). Unique persistend IDs are 
> > unfavorable as well; names, as used with xt_quota2/xt_NFACCT can be 
> > remembered much more easily.
> 
> Rule names could serve this, couldn't they?  And rules can be identified
> by -m comment if batch processing is required.

What you propose is hackish. You parse text-based outputs, which is
not the nice way to make things.

Re: [PATCH 0/2] [RFC] Extended accounting infrastructure for iptables

[Ferenc Wagner]

Jan Engelhardt <jengelh@medozas.de> writes:

> On Thursday 2011-12-15 21:23, Ferenc Wagner wrote:
>
>> Jan Engelhardt <jengelh@medozas.de> writes:
>>
>>> On Wednesday 2011-12-14 15:52, Changli Gao wrote:
>>>
>>>> On Wed, Dec 14, 2011 at 9:30 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>>>>
>>>>> On Wed, Dec 14, 2011 at 09:12:52PM +0800, Changli Gao wrote:
>>>>>>
>>>>>> Why not use the counters of iptables instead?
>>>>>>
>>>>>> iptables-save -c
>>>>>
>>>>> If you want to obtain the sum of the counters that match some criteria,
>>>>> you have to iterate over the whole list of existing rules, look for
>>>>> matchings and update the counters.
>>>>
>>>> As I said in another thread, you can redirect the traffic to a
>>>> separated chain, and use the counters of that chain.
>>>
>>> UDCs (user defined chains) don't have counters, though.
>>
>> So put an empty rule into them.  The ip_ plugin of Munin uses this
>> technique for quite some time.
>>
>>>>> Moreover, if you have a large rule-set, polling periodically
>>>>> iptables-save -c can be expensive.
>>>>
>>>> I got it. Thanks. Maybe we can index the entries in the kernel, and
>>>> add a new interface to get the counters of a special entry with a
>>>> entry ID.
>>>
>>> Relying on the rule number is a terrible idea (just like 
>>> iptables-save|head -n5|tail -n1 would be). Unique persistend IDs are 
>>> unfavorable as well; names, as used with xt_quota2/xt_NFACCT can be 
>>> remembered much more easily.
>>
>> Rule names could serve this, couldn't they?  And rules can be identified
>> by -m comment if batch processing is required.
>
> Then you can just as well use nfacct's name.

Sorry, I didn't write what I wanted to.  I meant chain names, not rule
names (which don't even exist).  Also, bringing up -m comment was a
mistake, please disregard it.

Pablo Neira Ayuso <pablo@netfilter.org> writes:

> What you propose is hackish.

Do you consider creating a new chain with a single empty rule hackish?
I accept that nfacct is a more transparent solution.  But I don't think
those single rule counter chains are that bad, either.  And they are
potentially more flexible (which may be an advantage or a disadvantage
as well).  And they don't require adding (and maintaining) new code.

> You parse text-based outputs, which is not the nice way to make
> things.

Agreed.  But I don't see the principal difference: just as you provide
libnetfilter_acct, someone could provide a similar library for handling
the rule counters (maybe such a library is already available, I don't
know).  Also, I bet 98% of the uses would involve shell scripts anyway,
using nfacct_get http-traffic or iptables -vL http-traffic for much the
same effect. :)

Please don't think I'm against your code.  My point was that there is a
viable alternative already present in the kernel.  I will shut up now.
-- 
Regards,
Feri.

Re: [PATCH 0/2] [RFC] Extended accounting infrastructure for iptables

[Pablo Neira Ayuso]

On Fri, Dec 16, 2011 at 04:25:54PM +0100, Ferenc Wagner wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> writes:
> 
> > What you propose is hackish.
> 
> Do you consider creating a new chain with a single empty rule hackish?

No. What I consider hackish is to parse the output of iptables -Lnv,
most likely looking for some pattern that -m comment displays to
collect the counters.

> I accept that nfacct is a more transparent solution.  But I don't think
> those single rule counter chains are that bad, either.  And they are
> potentially more flexible (which may be an advantage or a disadvantage
> as well).  And they don't require adding (and maintaining) new code.
>
> > You parse text-based outputs, which is not the nice way to make
> > things.
> 
> Agreed.  But I don't see the principal difference: just as you provide
> libnetfilter_acct, someone could provide a similar library for handling
> the rule counters (maybe such a library is already available, I don't
> know). Also, I bet 98% of the uses would involve shell scripts anyway,
> using nfacct_get http-traffic or iptables -vL http-traffic for much the
> same effect. :)

Bad betting, you owe me one beer ;-).

With nfacct you will not need to make shell scripts at all for your
applications. You've got one library that provides one netlink
interface that you can use in your C programs (or whatever language
that allows to make native calls to C functions).

Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink

[Pablo Neira Ayuso]

On Wed, Dec 14, 2011 at 02:45:20PM +0100, Eric Dumazet wrote:
> [PATCH] conntrack: use atomic64 for acct counters
> 
> We can use atomic64_t infrastructure to avoid taking a spinlock in fast
> path, and remove inaccuracies while reading values in
> ctnetlink_dump_counters() and connbytes_mt() on 32bit arches.
> 
> Suggested by Pablo
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied to nf-next, thanks Eric!