* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-14 11:00 ` [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink pablo
@ 2011-12-14 11:16 ` Eric Dumazet
2011-12-14 12:41 ` Pablo Neira Ayuso
2011-12-14 11:23 ` Patrick McHardy
` (3 subsequent siblings)
4 siblings, 1 reply; 28+ messages in thread
From: Eric Dumazet @ 2011-12-14 11:16 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, kadlec, kaber, jengelh, thomas.jarosch
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
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-14 11:16 ` Eric Dumazet
@ 2011-12-14 12:41 ` Pablo Neira Ayuso
2011-12-14 13:18 ` Eric Dumazet
0 siblings, 1 reply; 28+ messages in thread
From: Pablo Neira Ayuso @ 2011-12-14 12:41 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netfilter-devel, kadlec, kaber, jengelh, thomas.jarosch
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
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-14 12:41 ` Pablo Neira Ayuso
@ 2011-12-14 13:18 ` Eric Dumazet
2011-12-14 13:45 ` Eric Dumazet
0 siblings, 1 reply; 28+ messages in thread
From: Eric Dumazet @ 2011-12-14 13:18 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, kadlec, kaber, jengelh, thomas.jarosch
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-14 13:18 ` Eric Dumazet
@ 2011-12-14 13:45 ` Eric Dumazet
2011-12-18 0:21 ` Pablo Neira Ayuso
0 siblings, 1 reply; 28+ messages in thread
From: Eric Dumazet @ 2011-12-14 13:45 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, kadlec, kaber, jengelh, thomas.jarosch
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
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-14 13:45 ` Eric Dumazet
@ 2011-12-18 0:21 ` Pablo Neira Ayuso
0 siblings, 0 replies; 28+ messages in thread
From: Pablo Neira Ayuso @ 2011-12-18 0:21 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netfilter-devel, kadlec, kaber, jengelh, thomas.jarosch
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!
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-14 11:00 ` [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink pablo
2011-12-14 11:16 ` Eric Dumazet
@ 2011-12-14 11:23 ` Patrick McHardy
2011-12-14 13:18 ` Pablo Neira Ayuso
2011-12-14 13:23 ` Changli Gao
` (2 subsequent siblings)
4 siblings, 1 reply; 28+ messages in thread
From: Patrick McHardy @ 2011-12-14 11:23 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, kadlec, jengelh, thomas.jarosch
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);
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-14 11:23 ` Patrick McHardy
@ 2011-12-14 13:18 ` Pablo Neira Ayuso
2011-12-14 16:31 ` Patrick McHardy
0 siblings, 1 reply; 28+ messages in thread
From: Pablo Neira Ayuso @ 2011-12-14 13:18 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel, kadlec, jengelh, thomas.jarosch
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!
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-14 13:18 ` Pablo Neira Ayuso
@ 2011-12-14 16:31 ` Patrick McHardy
2011-12-15 12:20 ` Pablo Neira Ayuso
0 siblings, 1 reply; 28+ messages in thread
From: Patrick McHardy @ 2011-12-14 16:31 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, kadlec, jengelh, thomas.jarosch
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.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-14 16:31 ` Patrick McHardy
@ 2011-12-15 12:20 ` Pablo Neira Ayuso
0 siblings, 0 replies; 28+ messages in thread
From: Pablo Neira Ayuso @ 2011-12-15 12:20 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel, kadlec, jengelh, thomas.jarosch
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-14 11:00 ` [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink pablo
2011-12-14 11:16 ` Eric Dumazet
2011-12-14 11:23 ` Patrick McHardy
@ 2011-12-14 13:23 ` Changli Gao
2011-12-14 13:43 ` Jan Engelhardt
2011-12-14 13:49 ` Anand Raj Manickam
4 siblings, 0 replies; 28+ messages in thread
From: Changli Gao @ 2011-12-14 13:23 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, kadlec, kaber, jengelh, thomas.jarosch
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
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-14 11:00 ` [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink pablo
` (2 preceding siblings ...)
2011-12-14 13:23 ` Changli Gao
@ 2011-12-14 13:43 ` Jan Engelhardt
2011-12-14 16:50 ` Pablo Neira Ayuso
2011-12-14 13:49 ` Anand Raj Manickam
4 siblings, 1 reply; 28+ messages in thread
From: Jan Engelhardt @ 2011-12-14 13:43 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, kadlec, kaber, thomas.jarosch
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*.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-14 13:43 ` Jan Engelhardt
@ 2011-12-14 16:50 ` Pablo Neira Ayuso
2011-12-14 18:30 ` Jozsef Kadlecsik
0 siblings, 1 reply; 28+ messages in thread
From: Pablo Neira Ayuso @ 2011-12-14 16:50 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: netfilter-devel, kadlec, kaber, thomas.jarosch
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-14 16:50 ` Pablo Neira Ayuso
@ 2011-12-14 18:30 ` Jozsef Kadlecsik
2011-12-14 23:06 ` Maciej Żenczykowski
2011-12-15 12:26 ` Pablo Neira Ayuso
0 siblings, 2 replies; 28+ messages in thread
From: Jozsef Kadlecsik @ 2011-12-14 18:30 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Jan Engelhardt, netfilter-devel, kaber, thomas.jarosch
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
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-14 18:30 ` Jozsef Kadlecsik
@ 2011-12-14 23:06 ` Maciej Żenczykowski
2011-12-15 12:26 ` Pablo Neira Ayuso
1 sibling, 0 replies; 28+ messages in thread
From: Maciej Żenczykowski @ 2011-12-14 23:06 UTC (permalink / raw)
To: Jozsef Kadlecsik
Cc: Pablo Neira Ayuso, Jan Engelhardt, netfilter-devel, kaber,
thomas.jarosch
>> > 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...
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-14 18:30 ` Jozsef Kadlecsik
2011-12-14 23:06 ` Maciej Żenczykowski
@ 2011-12-15 12:26 ` Pablo Neira Ayuso
2011-12-15 12:32 ` Jan Engelhardt
1 sibling, 1 reply; 28+ messages in thread
From: Pablo Neira Ayuso @ 2011-12-15 12:26 UTC (permalink / raw)
To: Jozsef Kadlecsik; +Cc: Jan Engelhardt, netfilter-devel, kaber, thomas.jarosch
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-15 12:26 ` Pablo Neira Ayuso
@ 2011-12-15 12:32 ` Jan Engelhardt
0 siblings, 0 replies; 28+ messages in thread
From: Jan Engelhardt @ 2011-12-15 12:32 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Jozsef Kadlecsik, netfilter-devel, kaber, thomas.jarosch
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*/).
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-14 11:00 ` [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink pablo
` (3 preceding siblings ...)
2011-12-14 13:43 ` Jan Engelhardt
@ 2011-12-14 13:49 ` Anand Raj Manickam
2011-12-14 13:54 ` Eric Dumazet
4 siblings, 1 reply; 28+ messages in thread
From: Anand Raj Manickam @ 2011-12-14 13:49 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, kadlec, kaber, jengelh, thomas.jarosch
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
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/2] netfilter: add extended accounting infrastructure over nfnetlink
2011-12-14 13:49 ` Anand Raj Manickam
@ 2011-12-14 13:54 ` Eric Dumazet
0 siblings, 0 replies; 28+ messages in thread
From: Eric Dumazet @ 2011-12-14 13:54 UTC (permalink / raw)
To: Anand Raj Manickam
Cc: pablo, netfilter-devel, kadlec, kaber, jengelh, thomas.jarosch
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
^ permalink raw reply [flat|nested] 28+ messages in thread