Patchwork [v3,kernel,19/29] add byte threshold capability to nfacct

login
register
mail settings
Submitter Michael Zintakis
Date July 10, 2013, 6:25 p.m.
Message ID <1373480727-11254-20-git-send-email-michael.zintakis@googlemail.com>
Download mbox | patch
Permalink /patch/258207/
State Not Applicable
Headers show

Comments

Michael Zintakis - July 10, 2013, 6:25 p.m.
* add a 'bthr' variable to each nfacct object, allowing a bytes 'threshold'
to be stored and then reported if/when traffic breaches it.

Signed-off-by: Michael Zintakis <michael.zintakis@googlemail.com>
---
 include/uapi/linux/netfilter/nfnetlink_acct.h |  9 ++++++
 net/netfilter/nfnetlink_acct.c                | 43 ++++++++++++++++++++++++++-
 2 files changed, 51 insertions(+), 1 deletion(-)
Florian Westphal - July 10, 2013, 8 p.m.
Michael Zintakis <michael.zintakis@googlemail.com> wrote:
> * add a 'bthr' variable to each nfacct object, allowing a bytes 'threshold'
> to be stored and then reported if/when traffic breaches it.

Again, why is this needed?
Why is it useful?
--
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
Michael Zintakis - July 11, 2013, 6:56 p.m.
Florian Westphal wrote:
> Michael Zintakis <michael.zintakis@googlemail.com> wrote:
>> * add a 'bthr' variable to each nfacct object, allowing a bytes 'threshold'
>> to be stored and then reported if/when traffic breaches it.
> 
> Again, why is this needed?
> Why is it useful?
This is used for measuring traffic "expectancy", i.e. allows one to be able to register what amount of traffic is "expected" to pass through this accounting object. If that traffic threshold is exceeded, this is properly indicated when the accounting object is listed or any statistics for that object are being collected by the nfacct daemon.

That traffic "expectancy" can be set/reset depending on the nature of the traffic or its source/destination etc, so it is pretty flexible. Again, there is extensive information on this in the (revised) man page if you decide to look at it.
--
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
Florian Westphal - July 11, 2013, 8:25 p.m.
Michael Zintakis <michael.zintakis@googlemail.com> wrote:
> Florian Westphal wrote:
> > Michael Zintakis <michael.zintakis@googlemail.com> wrote:
> >> * add a 'bthr' variable to each nfacct object, allowing a bytes 'threshold'
> >> to be stored and then reported if/when traffic breaches it.
> > 
> > Again, why is this needed?
> > Why is it useful?
> This is used for measuring traffic "expectancy", i.e. allows one to be able to register what amount of traffic is "expected" to pass through this accounting object. If that traffic threshold is exceeded, this is properly indicated when the accounting object is listed or any statistics for that object are being collected by the nfacct daemon.
> 
> That traffic "expectancy" can be set/reset depending on the nature of the traffic or its source/destination etc, so it is pretty flexible. Again, there is extensive information on this in the (revised) man page if you decide to look at it.

I still don't understand why this needs to be in the kernel.
nfacct gives you the counters, how these are interpreted (e.g. 'higher
than expected' should be entirely up to userspace).

In case you need some way of reacting to excess counters, then perhaps
it makes sense to change nfacct match to allow "greater/less than"
matching expression instead?

E.g.:
-A bla -m nfacct --nfacct-name bla --nfacct-packets 1000000: -m limit
--limit 1/hour -j NFLOG --nflog-prefix 'bla packet threshold'

or something like that?

There is something similar for the conntrack accounting (-m connbytes).
--
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
Alexey Perevalov - July 17, 2013, 7:44 p.m.
> Date: Thu, 11 Jul 2013 22:25:47 +0200
> From: fw@strlen.de
> To: michael.zintakis@googlemail.com
> CC: fw@strlen.de; netfilter-devel@vger.kernel.org; pablo@netfilter.org
> Subject: Re: [PATCH v3 kernel 19/29] add byte threshold capability to nfacct
> 
> Michael Zintakis <michael.zintakis@googlemail.com> wrote:
>> Florian Westphal wrote:
>>> Michael Zintakis <michael.zintakis@googlemail.com> wrote:
>>>> * add a 'bthr' variable to each nfacct object, allowing a bytes 'threshold'
>>>> to be stored and then reported if/when traffic breaches it.
>>> 
>>> Again, why is this needed?
>>> Why is it useful?
>> This is used for measuring traffic "expectancy", i.e. allows one to be able to register what amount of traffic is "expected" to pass through this accounting object. If that traffic threshold is exceeded, this is properly indicated when the accounting object is listed or any statistics for that object are being collected by the nfacct daemon.
>> 
>> That traffic "expectancy" can be set/reset depending on the nature of the traffic or its source/destination etc, so it is pretty flexible. Again, there is extensive information on this in the (revised) man page if you decide to look at it.
> 
> I still don't understand why this needs to be in the kernel.
> nfacct gives you the counters, how these are interpreted (e.g. 'higher
> than expected' should be entirely up to userspace).

I also vote for this patch.
I'll try here to describe our use case.
We checking counter every minute, why not to check it more often? It's possible and doesn't lead to huge performance problems, but we want to save battery power and we don't want such big resolution in measurements. But also we need to restrict traffic according to predefined user quota. To not exceed such restriction we need to deligate such responsobility to kernel.
Now I talking only about wireless connection, but even on 3G it's possible to download more than 50Mb per one minute.
Also we need to inform user about quota comming beforehand. Based on given kernel couters update time interval, quota value and bandwidth I came to conclusion what it's better to have some warning threshold for informing user space from kernel.
I predict here comment about none general use case :)

I thought to add it to xt_quota, but nfacct it's better place.


> In case you need some way of reacting to excess counters, then perhaps
> it makes sense to change nfacct match to allow "greater/less than"
> matching expression instead?
> 
> E.g.:
> -A bla -m nfacct --nfacct-name bla --nfacct-packets 1000000: -m limit
> --limit 1/hour -j NFLOG --nflog-prefix 'bla packet threshold'
> 
> or something like that?
It's fit too.

P.S.  we don't use neither nfacct now, nor xt_quota due we counting based on net_cls (cls_cgroup) marks and as I understand it's not possible for incomming traffic without netfilter refactoring.
> 
> There is something similar for the conntrack accounting (-m connbytes).
> --
> 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

Patch

diff --git a/include/uapi/linux/netfilter/nfnetlink_acct.h b/include/uapi/linux/netfilter/nfnetlink_acct.h
index 0b65f9c1..e972970 100644
--- a/include/uapi/linux/netfilter/nfnetlink_acct.h
+++ b/include/uapi/linux/netfilter/nfnetlink_acct.h
@@ -19,10 +19,19 @@  enum nfnl_acct_type {
 	NFACCT_PKTS,
 	NFACCT_BYTES,
 	NFACCT_USE,
+	NFACCT_BTHR,
 	NFACCT_FMT,
+	NFACCT_FLAGS,
+	NFACCT_CMD,
 	__NFACCT_MAX
 };
 #define NFACCT_MAX (__NFACCT_MAX - 1)
 
+enum nfnl_acct_flags {
+	NFACCT_FLAG_BIT_BTHR 	= 0,
+	NFACCT_FLAG_BTHR	= (1 << NFACCT_FLAG_BIT_BTHR),
+	NFACCT_FLAG_BIT_MAX	= 1,
+	NFACCT_FLAG_MAX		= (1 << NFACCT_FLAG_BIT_MAX),
+};
 
 #endif /* _UAPI_NFNL_ACCT_H_ */
diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
index 92ecad1..18cd28e 100644
--- a/net/netfilter/nfnetlink_acct.c
+++ b/net/netfilter/nfnetlink_acct.c
@@ -32,7 +32,9 @@  static LIST_HEAD(nfnl_acct_list);
 struct nf_acct {
 	atomic64_t		pkts;
 	atomic64_t		bytes;
+	u64			bthr;
 	u16			fmt;
+	u16			flags;
 	struct list_head	head;
 	atomic_t		refcnt;
 	char			name[NFACCT_NAME_MAX];
@@ -44,6 +46,7 @@  nfnl_acct_new(struct sock *nfnl, struct sk_buff *skb,
 	     const struct nlmsghdr *nlh, const struct nlattr * const tb[])
 {
 	struct nf_acct *nfacct, *matching = NULL;
+	unsigned int flags = 0, cmd = 0;
 	char *acct_name;
 
 	if (!tb[NFACCT_NAME] ||
@@ -51,6 +54,21 @@  nfnl_acct_new(struct sock *nfnl, struct sk_buff *skb,
 	    (!tb[NFACCT_BYTES] && tb[NFACCT_PKTS]))
 		return -EINVAL;
 
+	if (tb[NFACCT_CMD]) {
+		if (!tb[NFACCT_FLAGS])
+			return -EINVAL;
+
+		cmd = be16_to_cpu(nla_get_be16(tb[NFACCT_CMD]));
+		flags = be16_to_cpu(nla_get_be16(tb[NFACCT_FLAGS]));
+
+		if (cmd & NFACCT_FLAG_BTHR &&
+		    ((flags & NFACCT_FLAG_BTHR && !tb[NFACCT_BTHR]) ||
+		     (!(flags & NFACCT_FLAG_BTHR) && tb[NFACCT_BTHR])))
+			return -EINVAL;
+
+	} else if (tb[NFACCT_FLAGS])
+		return -EINVAL;
+
 	acct_name = nla_data(tb[NFACCT_NAME]);
 	if (strlen(acct_name) == 0)
 		return -EINVAL;
@@ -86,7 +104,7 @@  nfnl_acct_new(struct sock *nfnl, struct sk_buff *skb,
 				 * and == 0.
 				 *
 				 */
-				if (!tb[NFACCT_FMT]) {
+				if (!tb[NFACCT_FMT] && !cmd) {
 					atomic64_set(&matching->pkts, 0);
 					atomic64_set(&matching->bytes, 0);
 				}
@@ -96,6 +114,17 @@  nfnl_acct_new(struct sock *nfnl, struct sk_buff *skb,
 				matching->fmt =
 				be16_to_cpu(nla_get_be16(tb[NFACCT_FMT]));
 			}
+			/* ... and finally set the bytes threshold */
+			if (cmd & NFACCT_FLAG_BTHR) {
+				if (flags & NFACCT_FLAG_BTHR) {
+					matching->bthr = be64_to_cpu(
+					  nla_get_be64(tb[NFACCT_BTHR]));
+					matching->flags |= NFACCT_FLAG_BTHR;
+				} else {
+					matching->bthr = 0;
+					matching->flags &= ~NFACCT_FLAG_BTHR;
+				}
+			}
 			return 0;
 		}
 		return -EBUSY;
@@ -118,6 +147,13 @@  nfnl_acct_new(struct sock *nfnl, struct sk_buff *skb,
 	if (tb[NFACCT_FMT]) {
 		nfacct->fmt = be16_to_cpu(nla_get_be16(tb[NFACCT_FMT]));
 	}
+	if (cmd & NFACCT_FLAG_BTHR && flags & NFACCT_FLAG_BTHR) {
+		if (tb[NFACCT_BTHR])
+			nfacct->bthr = be64_to_cpu(
+				nla_get_be64(tb[NFACCT_BTHR]));
+
+		nfacct->flags |= flags & NFACCT_FLAG_BTHR;
+	}
 	atomic_set(&nfacct->refcnt, 1);
 	list_add_tail_rcu(&nfacct->head, &nfnl_acct_list);
 	return 0;
@@ -154,7 +190,9 @@  nfnl_acct_fill_info(struct sk_buff *skb, u32 portid, u32 seq, u32 type,
 	}
 	if (nla_put_be64(skb, NFACCT_PKTS, cpu_to_be64(pkts)) ||
 	    nla_put_be64(skb, NFACCT_BYTES, cpu_to_be64(bytes)) ||
+	    nla_put_be64(skb, NFACCT_BTHR, cpu_to_be64(acct->bthr)) ||
 	    nla_put_be16(skb, NFACCT_FMT, htons(acct->fmt)) ||
+	    nla_put_be16(skb, NFACCT_FLAGS, htons(acct->flags)) ||
 	    nla_put_be32(skb, NFACCT_USE, htonl(atomic_read(&acct->refcnt))))
 		goto nla_put_failure;
 
@@ -302,7 +340,10 @@  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 },
+	[NFACCT_BTHR] = { .type = NLA_U64 },
 	[NFACCT_FMT] = { .type = NLA_U16 },
+	[NFACCT_FLAGS] = { .type = NLA_U16 },
+	[NFACCT_CMD] = { .type = NLA_U16 },
 };
 
 static const struct nfnl_callback nfnl_acct_cb[NFNL_MSG_ACCT_MAX] = {