[RFC PATCH 0/2] netfilter: Adding quota capabilities to nfnetlink

[RFC PATCH 0/2] netfilter: Adding quota capabilities to nfnetlink

[mathieu.poirier]

From: Mathieu Poirier <mathieu.poirier@linaro.org>

This is another re-spin on adding quota capabilities to the nfnetlink
accounting framework.  This time most of the quota support is located
in the framework itself rather than the filter.

Comments welcomed.

Thanks,
Mathieu

Mathieu Poirier (1):
  netfilter: xt_nfacct: Add quota to nfacct filter

Pablo Neira Ayuso (1):
  netfilter: nfnetlink_acct: Adding quota support to nf_acct objects

 include/linux/netfilter/nfnetlink_acct.h      |  3 +-
 include/uapi/linux/netfilter/nfnetlink.h      |  2 +
 include/uapi/linux/netfilter/nfnetlink_acct.h |  9 +++
 net/netfilter/nfnetlink_acct.c                | 93 +++++++++++++++++++++++++++
 net/netfilter/xt_nfacct.c                     | 13 ++++
 5 files changed, 119 insertions(+), 1 deletion(-)

-- 
1.8.3.2

[RFC PATCH 1/2] netfilter: nfnetlink_acct: Adding quota support to nf_acct objects

[mathieu.poirier]

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

The nf_acct objects already support accounting at the byte and
packet level.  As such it is a natural extention to add the
capability to define a ceiling limit for both metrics.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 include/uapi/linux/netfilter/nfnetlink.h      |  2 +
 include/uapi/linux/netfilter/nfnetlink_acct.h |  9 +++
 net/netfilter/nfnetlink_acct.c                | 79 +++++++++++++++++++++++++++
 3 files changed, 90 insertions(+)

diff --git a/include/uapi/linux/netfilter/nfnetlink.h b/include/uapi/linux/netfilter/nfnetlink.h
index 596ddd4..354a7e5 100644
--- a/include/uapi/linux/netfilter/nfnetlink.h
+++ b/include/uapi/linux/netfilter/nfnetlink.h
@@ -20,6 +20,8 @@ enum nfnetlink_groups {
 #define NFNLGRP_CONNTRACK_EXP_DESTROY	NFNLGRP_CONNTRACK_EXP_DESTROY
 	NFNLGRP_NFTABLES,
 #define NFNLGRP_NFTABLES                NFNLGRP_NFTABLES
+	NFNLGRP_ACCT_QUOTA,
+#define NFNLGRP_ACCT_QUOTA		NFNLGRP_ACCT_QUOTA
 	__NFNLGRP_MAX,
 };
 #define NFNLGRP_MAX	(__NFNLGRP_MAX - 1)
diff --git a/include/uapi/linux/netfilter/nfnetlink_acct.h b/include/uapi/linux/netfilter/nfnetlink_acct.h
index c7b6269..51404ec 100644
--- a/include/uapi/linux/netfilter/nfnetlink_acct.h
+++ b/include/uapi/linux/netfilter/nfnetlink_acct.h
@@ -10,15 +10,24 @@ enum nfnl_acct_msg_types {
 	NFNL_MSG_ACCT_GET,
 	NFNL_MSG_ACCT_GET_CTRZERO,
 	NFNL_MSG_ACCT_DEL,
+	NFNL_MSG_ACCT_OVERQUOTA,
 	NFNL_MSG_ACCT_MAX
 };
 
+enum nfnl_acct_flags {
+	NFACCT_F_QUOTA_PKTS	= (1 << 0),
+	NFACCT_F_QUOTA_BYTES	= (1 << 1),
+	NFACCT_F_OVERQUOTA	= (1 << 2), /* can't be set from userspace */
+};
+
 enum nfnl_acct_type {
 	NFACCT_UNSPEC,
 	NFACCT_NAME,
 	NFACCT_PKTS,
 	NFACCT_BYTES,
 	NFACCT_USE,
+	NFACCT_FLAGS,
+	NFACCT_QUOTA,
 	__NFACCT_MAX
 };
 #define NFACCT_MAX (__NFACCT_MAX - 1)
diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
index c7b6d46..25afecf 100644
--- a/net/netfilter/nfnetlink_acct.c
+++ b/net/netfilter/nfnetlink_acct.c
@@ -32,18 +32,24 @@ static LIST_HEAD(nfnl_acct_list);
 struct nf_acct {
 	atomic64_t		pkts;
 	atomic64_t		bytes;
+	unsigned long		flags;
 	struct list_head	head;
 	atomic_t		refcnt;
 	char			name[NFACCT_NAME_MAX];
 	struct rcu_head		rcu_head;
+	char			data[0];
 };
 
+#define NFACCT_F_QUOTA (NFACCT_F_QUOTA_PKTS | NFACCT_F_QUOTA_BYTES)
+
 static int
 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;
 	char *acct_name;
+	unsigned int size = 0;
+	u32 flags = 0;
 
 	if (!tb[NFACCT_NAME])
 		return -EINVAL;
@@ -68,15 +74,39 @@ nfnl_acct_new(struct sock *nfnl, struct sk_buff *skb,
 			/* reset counters if you request a replacement. */
 			atomic64_set(&matching->pkts, 0);
 			atomic64_set(&matching->bytes, 0);
+			smp_mb__before_clear_bit();
+			/* reset overquota flag if quota is enabled. */
+			if ((matching->flags & NFACCT_F_QUOTA))
+				clear_bit(NFACCT_F_OVERQUOTA, &matching->flags);
 			return 0;
 		}
 		return -EBUSY;
 	}
 
 	nfacct = kzalloc(sizeof(struct nf_acct), GFP_KERNEL);
+	if (tb[NFACCT_FLAGS]) {
+		flags = ntohl(nla_get_be32(tb[NFACCT_FLAGS]));
+		if (flags & ~NFACCT_F_QUOTA)
+			return -EOPNOTSUPP;
+		if ((flags & NFACCT_F_QUOTA) == NFACCT_F_QUOTA)
+			return -EINVAL;
+		if (flags & NFACCT_F_OVERQUOTA)
+			return -EINVAL;
+
+		size += sizeof(u64);
+	}
+
+	nfacct = kzalloc(sizeof(struct nf_acct) + size, GFP_KERNEL);
 	if (nfacct == NULL)
 		return -ENOMEM;
 
+	if (flags & NFACCT_F_QUOTA) {
+		u64 *quota = (u64 *)nfacct->data;
+
+		*quota = be64_to_cpu(nla_get_be64(tb[NFACCT_QUOTA]));
+		nfacct->flags = flags;
+	}
+
 	strncpy(nfacct->name, nla_data(tb[NFACCT_NAME]), NFACCT_NAME_MAX);
 
 	if (tb[NFACCT_BYTES]) {
@@ -125,7 +155,13 @@ nfnl_acct_fill_info(struct sk_buff *skb, u32 portid, u32 seq, u32 type,
 	    nla_put_be64(skb, NFACCT_BYTES, cpu_to_be64(bytes)) ||
 	    nla_put_be32(skb, NFACCT_USE, htonl(atomic_read(&acct->refcnt))))
 		goto nla_put_failure;
+	if (acct->flags & NFACCT_F_QUOTA) {
+		u64 *quota = (u64 *)acct->data;
 
+		if (nla_put_be32(skb, NFACCT_FLAGS, htonl(acct->flags)) ||
+		    nla_put_be64(skb, NFACCT_QUOTA, cpu_to_be64(*quota)))
+			goto nla_put_failure;
+	}
 	nlmsg_end(skb, nlh);
 	return skb->len;
 
@@ -270,6 +306,8 @@ 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_FLAGS] = { .type = NLA_U32 },
+	[NFACCT_QUOTA] = { .type = NLA_U64 },
 };
 
 static const struct nfnl_callback nfnl_acct_cb[NFNL_MSG_ACCT_MAX] = {
@@ -336,6 +374,47 @@ void nfnl_acct_update(const struct sk_buff *skb, struct nf_acct *nfacct)
 }
 EXPORT_SYMBOL_GPL(nfnl_acct_update);
 
+static void nfnl_overquota_report(struct nf_acct *nfacct)
+{
+	int ret;
+	struct sk_buff *skb;
+
+	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+	if (skb == NULL)
+		return;
+
+	ret = nfnl_acct_fill_info(skb, 0, 0, NFNL_MSG_ACCT_OVERQUOTA, 0,
+				  nfacct);
+	if (ret <= 0) {
+		kfree_skb(skb);
+		return;
+	}
+	netlink_broadcast(init_net.nfnl, skb, 0, NFNLGRP_ACCT_QUOTA,
+			  GFP_ATOMIC);
+}
+
+bool nfnl_acct_overquota(const struct sk_buff *skb, struct nf_acct *nfacct)
+{
+	u64 *quota;
+	bool ret = false;
+
+	if (nfacct->flags & NFACCT_F_QUOTA_PKTS) {
+		quota = (u64 *)nfacct->data;
+		ret = atomic64_read(&nfacct->pkts) >= *quota &&
+		      !test_and_set_bit(NFACCT_F_OVERQUOTA, &nfacct->flags);
+	}
+	if (nfacct->flags & NFACCT_F_QUOTA_BYTES) {
+		quota = (u64 *)nfacct->data;
+		ret = atomic64_read(&nfacct->bytes) >= *quota &&
+		      !test_and_set_bit(NFACCT_F_OVERQUOTA, &nfacct->flags);
+	}
+	if (ret)
+		nfnl_overquota_report(nfacct);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(nfnl_acct_overquota);
+
 static int __init nfnl_acct_init(void)
 {
 	int ret;
-- 
1.8.3.2

[RFC PATCH 2/2] netfilter: xt_nfacct: Add quota to nfacct filter

[mathieu.poirier]

From: Mathieu Poirier <mathieu.poirier@linaro.org>

Fixing quota management in accounting framework, most notably:
- Clearing overquota bit in 'nfnl_acct_fill_info' when userspace
resets accounting statistics.
- Decoupling quota attainment verification and message dispatch
in 'nfnl_acct_overquota'.

With the above adding quota limits to xt_nfacct is trival.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 include/linux/netfilter/nfnetlink_acct.h |  3 ++-
 net/netfilter/nfnetlink_acct.c           | 36 ++++++++++++++++++++++----------
 net/netfilter/xt_nfacct.c                | 13 ++++++++++++
 3 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink_acct.h b/include/linux/netfilter/nfnetlink_acct.h
index b2e85e5..cfc210d 100644
--- a/include/linux/netfilter/nfnetlink_acct.h
+++ b/include/linux/netfilter/nfnetlink_acct.h
@@ -9,5 +9,6 @@ struct nf_acct;
 struct nf_acct *nfnl_acct_find_get(const char *filter_name);
 void nfnl_acct_put(struct nf_acct *acct);
 void nfnl_acct_update(const struct sk_buff *skb, struct nf_acct *nfacct);
-
+extern bool nfnl_acct_overquota(const struct sk_buff *skb,
+				struct nf_acct *nfacct);
 #endif /* _NFNL_ACCT_H */
diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
index 25afecf..e0b5147 100644
--- a/net/netfilter/nfnetlink_acct.c
+++ b/net/netfilter/nfnetlink_acct.c
@@ -147,6 +147,10 @@ nfnl_acct_fill_info(struct sk_buff *skb, u32 portid, u32 seq, u32 type,
 	if (type == NFNL_MSG_ACCT_GET_CTRZERO) {
 		pkts = atomic64_xchg(&acct->pkts, 0);
 		bytes = atomic64_xchg(&acct->bytes, 0);
+		if (acct->flags & NFACCT_F_QUOTA) {
+			smp_mb__before_clear_bit();
+			clear_bit(NFACCT_F_OVERQUOTA, &acct->flags);
+		}
 	} else {
 		pkts = atomic64_read(&acct->pkts);
 		bytes = atomic64_read(&acct->bytes);
@@ -393,23 +397,33 @@ static void nfnl_overquota_report(struct nf_acct *nfacct)
 			  GFP_ATOMIC);
 }
 
+/* Check 'skb' statistics against accounting object 'nfacct'.  Quotas limits
+ * are considered inclusive.  A message is sent to userspace if at or above
+ * a quota.
+ *
+ * Returns -EINVAL if nfacct object doesn't have a quota, true if above quota
+ * and false is below or at quota.
+ */
 bool nfnl_acct_overquota(const struct sk_buff *skb, struct nf_acct *nfacct)
 {
+	u64 now;
 	u64 *quota;
 	bool ret = false;
 
-	if (nfacct->flags & NFACCT_F_QUOTA_PKTS) {
-		quota = (u64 *)nfacct->data;
-		ret = atomic64_read(&nfacct->pkts) >= *quota &&
-		      !test_and_set_bit(NFACCT_F_OVERQUOTA, &nfacct->flags);
-	}
-	if (nfacct->flags & NFACCT_F_QUOTA_BYTES) {
-		quota = (u64 *)nfacct->data;
-		ret = atomic64_read(&nfacct->bytes) >= *quota &&
-		      !test_and_set_bit(NFACCT_F_OVERQUOTA, &nfacct->flags);
-	}
-	if (ret)
+	/* no place here if we don't have a quota */
+	if (!(nfacct->flags & NFACCT_F_QUOTA))
+		return -EINVAL;
+
+	quota = (u64 *)nfacct->data;
+	now = (nfacct->flags & NFACCT_F_QUOTA_PKTS) ?
+	       atomic64_read(&nfacct->pkts) : atomic64_read(&nfacct->bytes);
+
+	ret = now > *quota;
+
+	if (now >= *quota &&
+	    !test_and_set_bit(NFACCT_F_OVERQUOTA, &nfacct->flags)) {
 		nfnl_overquota_report(nfacct);
+	}
 
 	return ret;
 }
diff --git a/net/netfilter/xt_nfacct.c b/net/netfilter/xt_nfacct.c
index b3be0ef..bbe1491 100644
--- a/net/netfilter/xt_nfacct.c
+++ b/net/netfilter/xt_nfacct.c
@@ -21,10 +21,23 @@ MODULE_ALIAS("ip6t_nfacct");
 
 static bool nfacct_mt(const struct sk_buff *skb, struct xt_action_param *par)
 {
+	bool overquota;
 	const struct xt_nfacct_match_info *info = par->targinfo;
 
 	nfnl_acct_update(skb, info->nfacct);
 
+	overquota = nfnl_acct_overquota(skb, info->nfacct);
+
+	/* The only time packets are allowed through is when:
+	 * 1) A quota has been specivied.
+	 * 2) That quota hasn't been reached yet.
+	 */
+	if (overquota == false)
+		return false;
+
+	 /* Return true if a quota hasn't been specified or if
+	  * quota has been attained.
+	  */
 	return true;
 }
 
-- 
1.8.3.2

Re: [RFC PATCH 2/2] netfilter: xt_nfacct: Add quota to nfacct filter

[Pablo Neira Ayuso]

On Sun, Mar 30, 2014 at 03:10:44PM -0600, mathieu.poirier@linaro.org wrote:
> From: Mathieu Poirier <mathieu.poirier@linaro.org>
> 
> Fixing quota management in accounting framework, most notably:
> - Clearing overquota bit in 'nfnl_acct_fill_info' when userspace
> resets accounting statistics.
> - Decoupling quota attainment verification and message dispatch
> in 'nfnl_acct_overquota'.

Please, merge this to 1/2. I don't really mind about the one showing
up as author in the patch. You can add in the commit log that this
patch is a joint work between you and me so I'm also credited for the
changes.

Another comment below.

> With the above adding quota limits to xt_nfacct is trival.
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>  include/linux/netfilter/nfnetlink_acct.h |  3 ++-
>  net/netfilter/nfnetlink_acct.c           | 36 ++++++++++++++++++++++----------
>  net/netfilter/xt_nfacct.c                | 13 ++++++++++++
>  3 files changed, 40 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/netfilter/nfnetlink_acct.h b/include/linux/netfilter/nfnetlink_acct.h
> index b2e85e5..cfc210d 100644
> --- a/include/linux/netfilter/nfnetlink_acct.h
> +++ b/include/linux/netfilter/nfnetlink_acct.h
> @@ -9,5 +9,6 @@ struct nf_acct;
>  struct nf_acct *nfnl_acct_find_get(const char *filter_name);
>  void nfnl_acct_put(struct nf_acct *acct);
>  void nfnl_acct_update(const struct sk_buff *skb, struct nf_acct *nfacct);
> -
> +extern bool nfnl_acct_overquota(const struct sk_buff *skb,
> +				struct nf_acct *nfacct);
>  #endif /* _NFNL_ACCT_H */
> diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
> index 25afecf..e0b5147 100644
> --- a/net/netfilter/nfnetlink_acct.c
> +++ b/net/netfilter/nfnetlink_acct.c
> @@ -147,6 +147,10 @@ nfnl_acct_fill_info(struct sk_buff *skb, u32 portid, u32 seq, u32 type,
>  	if (type == NFNL_MSG_ACCT_GET_CTRZERO) {
>  		pkts = atomic64_xchg(&acct->pkts, 0);
>  		bytes = atomic64_xchg(&acct->bytes, 0);
> +		if (acct->flags & NFACCT_F_QUOTA) {
> +			smp_mb__before_clear_bit();
> +			clear_bit(NFACCT_F_OVERQUOTA, &acct->flags);
> +		}
>  	} else {
>  		pkts = atomic64_read(&acct->pkts);
>  		bytes = atomic64_read(&acct->bytes);
> @@ -393,23 +397,33 @@ static void nfnl_overquota_report(struct nf_acct *nfacct)
>  			  GFP_ATOMIC);
>  }
>  
> +/* Check 'skb' statistics against accounting object 'nfacct'.  Quotas limits
> + * are considered inclusive.  A message is sent to userspace if at or above
> + * a quota.
> + *
> + * Returns -EINVAL if nfacct object doesn't have a quota, true if above quota
> + * and false is below or at quota.
> + */
>  bool nfnl_acct_overquota(const struct sk_buff *skb, struct nf_acct *nfacct)
>  {
> +	u64 now;
>  	u64 *quota;
>  	bool ret = false;
>  
> -	if (nfacct->flags & NFACCT_F_QUOTA_PKTS) {
> -		quota = (u64 *)nfacct->data;
> -		ret = atomic64_read(&nfacct->pkts) >= *quota &&
> -		      !test_and_set_bit(NFACCT_F_OVERQUOTA, &nfacct->flags);
> -	}
> -	if (nfacct->flags & NFACCT_F_QUOTA_BYTES) {
> -		quota = (u64 *)nfacct->data;
> -		ret = atomic64_read(&nfacct->bytes) >= *quota &&
> -		      !test_and_set_bit(NFACCT_F_OVERQUOTA, &nfacct->flags);
> -	}
> -	if (ret)
> +	/* no place here if we don't have a quota */
> +	if (!(nfacct->flags & NFACCT_F_QUOTA))
> +		return -EINVAL;

This function returns boolean, we have to change it. You can probably
use this return values:

-1 no quota
0 underquota
1 overquota

You can just define some enum, ie.

enum {
        NFACCT_NO_QUOTA         = -1,
        NFACCT_UNDERQUOTA,
        NFACCT_OVERQUOTA,
};

to include/linux/netfilter/nfnetlink_acct.h.

I guess with that we can also remove the comments everywhere, as it
will be quite clear with the enums.

Merge window will be close soon, please also post the userspace
changes for the library and the utility asap.

Thanks.

> +
> +	quota = (u64 *)nfacct->data;
> +	now = (nfacct->flags & NFACCT_F_QUOTA_PKTS) ?
> +	       atomic64_read(&nfacct->pkts) : atomic64_read(&nfacct->bytes);
> +
> +	ret = now > *quota;
> +
> +	if (now >= *quota &&
> +	    !test_and_set_bit(NFACCT_F_OVERQUOTA, &nfacct->flags)) {
>  		nfnl_overquota_report(nfacct);
> +	}
>  
>  	return ret;
>  }
> diff --git a/net/netfilter/xt_nfacct.c b/net/netfilter/xt_nfacct.c
> index b3be0ef..bbe1491 100644
> --- a/net/netfilter/xt_nfacct.c
> +++ b/net/netfilter/xt_nfacct.c
> @@ -21,10 +21,23 @@ MODULE_ALIAS("ip6t_nfacct");
>  
>  static bool nfacct_mt(const struct sk_buff *skb, struct xt_action_param *par)
>  {
> +	bool overquota;
>  	const struct xt_nfacct_match_info *info = par->targinfo;
>  
>  	nfnl_acct_update(skb, info->nfacct);
>  
> +	overquota = nfnl_acct_overquota(skb, info->nfacct);
> +
> +	/* The only time packets are allowed through is when:
> +	 * 1) A quota has been specivied.
> +	 * 2) That quota hasn't been reached yet.
> +	 */
> +	if (overquota == false)
> +		return false;
> +
> +	 /* Return true if a quota hasn't been specified or if
> +	  * quota has been attained.
> +	  */
>  	return true;
>  }
>  
> -- 
> 1.8.3.2
> 

Re: [RFC PATCH 2/2] netfilter: xt_nfacct: Add quota to nfacct filter

[Mathieu Poirier]

On 31 March 2014 06:13, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Sun, Mar 30, 2014 at 03:10:44PM -0600, mathieu.poirier@linaro.org wrote:
>> From: Mathieu Poirier <mathieu.poirier@linaro.org>
>>
>> Fixing quota management in accounting framework, most notably:
>> - Clearing overquota bit in 'nfnl_acct_fill_info' when userspace
>> resets accounting statistics.
>> - Decoupling quota attainment verification and message dispatch
>> in 'nfnl_acct_overquota'.
>
> Please, merge this to 1/2. I don't really mind about the one showing
> up as author in the patch. You can add in the commit log that this
> patch is a joint work between you and me so I'm also credited for the
> changes.

Very well.

>
> Another comment below.
>
>> With the above adding quota limits to xt_nfacct is trival.
>>
>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>> ---
>>  include/linux/netfilter/nfnetlink_acct.h |  3 ++-
>>  net/netfilter/nfnetlink_acct.c           | 36 ++++++++++++++++++++++----------
>>  net/netfilter/xt_nfacct.c                | 13 ++++++++++++
>>  3 files changed, 40 insertions(+), 12 deletions(-)
>>
>> diff --git a/include/linux/netfilter/nfnetlink_acct.h b/include/linux/netfilter/nfnetlink_acct.h
>> index b2e85e5..cfc210d 100644
>> --- a/include/linux/netfilter/nfnetlink_acct.h
>> +++ b/include/linux/netfilter/nfnetlink_acct.h
>> @@ -9,5 +9,6 @@ struct nf_acct;
>>  struct nf_acct *nfnl_acct_find_get(const char *filter_name);
>>  void nfnl_acct_put(struct nf_acct *acct);
>>  void nfnl_acct_update(const struct sk_buff *skb, struct nf_acct *nfacct);
>> -
>> +extern bool nfnl_acct_overquota(const struct sk_buff *skb,
>> +                             struct nf_acct *nfacct);
>>  #endif /* _NFNL_ACCT_H */
>> diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
>> index 25afecf..e0b5147 100644
>> --- a/net/netfilter/nfnetlink_acct.c
>> +++ b/net/netfilter/nfnetlink_acct.c
>> @@ -147,6 +147,10 @@ nfnl_acct_fill_info(struct sk_buff *skb, u32 portid, u32 seq, u32 type,
>>       if (type == NFNL_MSG_ACCT_GET_CTRZERO) {
>>               pkts = atomic64_xchg(&acct->pkts, 0);
>>               bytes = atomic64_xchg(&acct->bytes, 0);
>> +             if (acct->flags & NFACCT_F_QUOTA) {
>> +                     smp_mb__before_clear_bit();
>> +                     clear_bit(NFACCT_F_OVERQUOTA, &acct->flags);
>> +             }
>>       } else {
>>               pkts = atomic64_read(&acct->pkts);
>>               bytes = atomic64_read(&acct->bytes);
>> @@ -393,23 +397,33 @@ static void nfnl_overquota_report(struct nf_acct *nfacct)
>>                         GFP_ATOMIC);
>>  }
>>
>> +/* Check 'skb' statistics against accounting object 'nfacct'.  Quotas limits
>> + * are considered inclusive.  A message is sent to userspace if at or above
>> + * a quota.
>> + *
>> + * Returns -EINVAL if nfacct object doesn't have a quota, true if above quota
>> + * and false is below or at quota.
>> + */
>>  bool nfnl_acct_overquota(const struct sk_buff *skb, struct nf_acct *nfacct)
>>  {
>> +     u64 now;
>>       u64 *quota;
>>       bool ret = false;
>>
>> -     if (nfacct->flags & NFACCT_F_QUOTA_PKTS) {
>> -             quota = (u64 *)nfacct->data;
>> -             ret = atomic64_read(&nfacct->pkts) >= *quota &&
>> -                   !test_and_set_bit(NFACCT_F_OVERQUOTA, &nfacct->flags);
>> -     }
>> -     if (nfacct->flags & NFACCT_F_QUOTA_BYTES) {
>> -             quota = (u64 *)nfacct->data;
>> -             ret = atomic64_read(&nfacct->bytes) >= *quota &&
>> -                   !test_and_set_bit(NFACCT_F_OVERQUOTA, &nfacct->flags);
>> -     }
>> -     if (ret)
>> +     /* no place here if we don't have a quota */
>> +     if (!(nfacct->flags & NFACCT_F_QUOTA))
>> +             return -EINVAL;
>
> This function returns boolean, we have to change it. You can probably
> use this return values:
>
> -1 no quota
> 0 underquota
> 1 overquota

Ok.

>
> You can just define some enum, ie.
>
> enum {
>         NFACCT_NO_QUOTA         = -1,
>         NFACCT_UNDERQUOTA,
>         NFACCT_OVERQUOTA,
> };
>
> to include/linux/netfilter/nfnetlink_acct.h.
>
> I guess with that we can also remove the comments everywhere, as it
> will be quite clear with the enums.
>
> Merge window will be close soon, please also post the userspace
> changes for the library and the utility asap.

At this time I only have the userspace modifications in the nfacct
port I did for Android.  That one links to libnl2.0 and won't be of
interest to you.  I can get you the kernel work in time for the merge
window but modifications to the "real" nfacct may not make it.  I'd
love to see the work going in the 3.15 merge window but can't offer
guarantees.

Would you consider queuing the kernel patch in the merge window
without the userspace?  Note that I would understand if you woudn't
accept that....

>
> Thanks.
>
>> +
>> +     quota = (u64 *)nfacct->data;
>> +     now = (nfacct->flags & NFACCT_F_QUOTA_PKTS) ?
>> +            atomic64_read(&nfacct->pkts) : atomic64_read(&nfacct->bytes);
>> +
>> +     ret = now > *quota;
>> +
>> +     if (now >= *quota &&
>> +         !test_and_set_bit(NFACCT_F_OVERQUOTA, &nfacct->flags)) {
>>               nfnl_overquota_report(nfacct);
>> +     }
>>
>>       return ret;
>>  }
>> diff --git a/net/netfilter/xt_nfacct.c b/net/netfilter/xt_nfacct.c
>> index b3be0ef..bbe1491 100644
>> --- a/net/netfilter/xt_nfacct.c
>> +++ b/net/netfilter/xt_nfacct.c
>> @@ -21,10 +21,23 @@ MODULE_ALIAS("ip6t_nfacct");
>>
>>  static bool nfacct_mt(const struct sk_buff *skb, struct xt_action_param *par)
>>  {
>> +     bool overquota;
>>       const struct xt_nfacct_match_info *info = par->targinfo;
>>
>>       nfnl_acct_update(skb, info->nfacct);
>>
>> +     overquota = nfnl_acct_overquota(skb, info->nfacct);
>> +
>> +     /* The only time packets are allowed through is when:
>> +      * 1) A quota has been specivied.
>> +      * 2) That quota hasn't been reached yet.
>> +      */
>> +     if (overquota == false)
>> +             return false;
>> +
>> +      /* Return true if a quota hasn't been specified or if
>> +       * quota has been attained.
>> +       */
>>       return true;
>>  }
>>
>> --
>> 1.8.3.2
>>

Re: [RFC PATCH 2/2] netfilter: xt_nfacct: Add quota to nfacct filter

[Pablo Neira Ayuso]

On Mon, Mar 31, 2014 at 07:50:06AM -0600, Mathieu Poirier wrote:
[...]
> >
> > You can just define some enum, ie.
> >
> > enum {
> >         NFACCT_NO_QUOTA         = -1,
> >         NFACCT_UNDERQUOTA,
> >         NFACCT_OVERQUOTA,
> > };
> >
> > to include/linux/netfilter/nfnetlink_acct.h.
> >
> > I guess with that we can also remove the comments everywhere, as it
> > will be quite clear with the enums.
> >
> > Merge window will be close soon, please also post the userspace
> > changes for the library and the utility asap.
> 
> At this time I only have the userspace modifications in the nfacct
> port I did for Android.  That one links to libnl2.0 and won't be of
> interest to you.  I can get you the kernel work in time for the merge
> window but modifications to the "real" nfacct may not make it.  I'd
> love to see the work going in the 3.15 merge window but can't offer
> guarantees.
> 
> Would you consider queuing the kernel patch in the merge window
> without the userspace?  Note that I would understand if you woudn't
> accept that....

You have to send me the library changes for libnetfilter_acct at
least and the tested kernel patch asap.