* [PATCH nf-next,v2] netfilter: nft_counter: convert it to use per-cpu counters
@ 2015-06-19 9:36 Pablo Neira Ayuso
2015-06-19 9:39 ` Patrick McHardy
0 siblings, 1 reply; 7+ messages in thread
From: Pablo Neira Ayuso @ 2015-06-19 9:36 UTC (permalink / raw)
To: netfilter-devel; +Cc: kaber, fw, eric.dumazet
This patch converts the existing seqlock to per-cpu counters.
Suggested-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nft_counter.c | 93 ++++++++++++++++++++++++++++++-------------
1 file changed, 66 insertions(+), 27 deletions(-)
diff --git a/net/netfilter/nft_counter.c b/net/netfilter/nft_counter.c
index 1759123..ee80cb9 100644
--- a/net/netfilter/nft_counter.c
+++ b/net/netfilter/nft_counter.c
@@ -18,40 +18,56 @@
#include <net/netfilter/nf_tables.h>
struct nft_counter {
- seqlock_t lock;
u64 bytes;
u64 packets;
};
+struct nft_counter_percpu {
+ struct nft_counter counter;
+ struct u64_stats_sync syncp;
+};
+
+struct nft_counter_percpu_priv {
+ struct nft_counter_percpu __percpu *counter;
+};
+
static void nft_counter_eval(const struct nft_expr *expr,
struct nft_regs *regs,
const struct nft_pktinfo *pkt)
{
- struct nft_counter *priv = nft_expr_priv(expr);
+ struct nft_counter_percpu_priv *priv = nft_expr_priv(expr);
+ struct nft_counter_percpu *this_cpu;
- write_seqlock_bh(&priv->lock);
- priv->bytes += pkt->skb->len;
- priv->packets++;
- write_sequnlock_bh(&priv->lock);
+ this_cpu = this_cpu_ptr(priv->counter);
+ this_cpu->counter.bytes += pkt->skb->len;
+ this_cpu->counter.packets++;
}
static int nft_counter_dump(struct sk_buff *skb, const struct nft_expr *expr)
{
- struct nft_counter *priv = nft_expr_priv(expr);
+ struct nft_counter_percpu_priv *priv = nft_expr_priv(expr);
+ struct nft_counter_percpu *cpu_stats;
+ struct nft_counter total;
+ u64 bytes, packets;
unsigned int seq;
- u64 bytes;
- u64 packets;
-
- do {
- seq = read_seqbegin(&priv->lock);
- bytes = priv->bytes;
- packets = priv->packets;
- } while (read_seqretry(&priv->lock, seq));
-
- if (nla_put_be64(skb, NFTA_COUNTER_BYTES, cpu_to_be64(bytes)))
- goto nla_put_failure;
- if (nla_put_be64(skb, NFTA_COUNTER_PACKETS, cpu_to_be64(packets)))
+ int cpu;
+
+ memset(&total, 0, sizeof(total));
+ for_each_possible_cpu(cpu) {
+ cpu_stats = per_cpu_ptr(priv->counter, cpu);
+ do {
+ bytes = cpu_stats->counter.bytes;
+ packets = cpu_stats->counter.packets;
+ } while (u64_stats_fetch_retry_irq(&cpu_stats->syncp, seq));
+
+ total.packets += packets;
+ total.bytes += bytes;
+ }
+
+ if (nla_put_be64(skb, NFTA_COUNTER_BYTES, cpu_to_be64(total.bytes)) ||
+ nla_put_be64(skb, NFTA_COUNTER_PACKETS, cpu_to_be64(total.packets)))
goto nla_put_failure;
+
return 0;
nla_put_failure:
@@ -67,23 +83,46 @@ static int nft_counter_init(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nlattr * const tb[])
{
- struct nft_counter *priv = nft_expr_priv(expr);
-
- if (tb[NFTA_COUNTER_PACKETS])
- priv->packets = be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_PACKETS]));
- if (tb[NFTA_COUNTER_BYTES])
- priv->bytes = be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_BYTES]));
+ struct nft_counter_percpu_priv *priv = nft_expr_priv(expr);
+ struct nft_counter_percpu __percpu *cpu_stats;
+ struct nft_counter_percpu *this_cpu;
+
+ cpu_stats = netdev_alloc_pcpu_stats(struct nft_counter_percpu);
+ if (cpu_stats == NULL)
+ return ENOMEM;
+
+ preempt_disable();
+ this_cpu = this_cpu_ptr(cpu_stats);
+ if (tb[NFTA_COUNTER_PACKETS]) {
+ this_cpu->counter.packets =
+ be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_PACKETS]));
+ }
+ if (tb[NFTA_COUNTER_BYTES]) {
+ this_cpu->counter.bytes =
+ be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_BYTES]));
+ }
+ preempt_enable();
+
+ priv->counter = cpu_stats;
- seqlock_init(&priv->lock);
return 0;
}
+static void nft_counter_destroy(const struct nft_ctx *ctx,
+ const struct nft_expr *expr)
+{
+ struct nft_counter_percpu_priv *priv = nft_expr_priv(expr);
+
+ free_percpu(priv->counter);
+}
+
static struct nft_expr_type nft_counter_type;
static const struct nft_expr_ops nft_counter_ops = {
.type = &nft_counter_type,
- .size = NFT_EXPR_SIZE(sizeof(struct nft_counter)),
+ .size = NFT_EXPR_SIZE(sizeof(struct nft_counter_percpu_priv)),
.eval = nft_counter_eval,
.init = nft_counter_init,
+ .destroy = nft_counter_destroy,
.dump = nft_counter_dump,
};
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH nf-next,v2] netfilter: nft_counter: convert it to use per-cpu counters
2015-06-19 9:36 [PATCH nf-next,v2] netfilter: nft_counter: convert it to use per-cpu counters Pablo Neira Ayuso
@ 2015-06-19 9:39 ` Patrick McHardy
2015-06-19 9:49 ` Pablo Neira Ayuso
0 siblings, 1 reply; 7+ messages in thread
From: Patrick McHardy @ 2015-06-19 9:39 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, fw, eric.dumazet
On 19.06, Pablo Neira Ayuso wrote:
> static void nft_counter_eval(const struct nft_expr *expr,
> struct nft_regs *regs,
> const struct nft_pktinfo *pkt)
> {
> - struct nft_counter *priv = nft_expr_priv(expr);
> + struct nft_counter_percpu_priv *priv = nft_expr_priv(expr);
> + struct nft_counter_percpu *this_cpu;
>
> - write_seqlock_bh(&priv->lock);
> - priv->bytes += pkt->skb->len;
> - priv->packets++;
> - write_sequnlock_bh(&priv->lock);
> + this_cpu = this_cpu_ptr(priv->counter);
> + this_cpu->counter.bytes += pkt->skb->len;
> + this_cpu->counter.packets++;
u64_state_update_begin()/end()?
> @@ -67,23 +83,46 @@ static int nft_counter_init(const struct nft_ctx *ctx,
> const struct nft_expr *expr,
> const struct nlattr * const tb[])
> {
> - struct nft_counter *priv = nft_expr_priv(expr);
> -
> - if (tb[NFTA_COUNTER_PACKETS])
> - priv->packets = be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_PACKETS]));
> - if (tb[NFTA_COUNTER_BYTES])
> - priv->bytes = be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_BYTES]));
> + struct nft_counter_percpu_priv *priv = nft_expr_priv(expr);
> + struct nft_counter_percpu __percpu *cpu_stats;
> + struct nft_counter_percpu *this_cpu;
> +
> + cpu_stats = netdev_alloc_pcpu_stats(struct nft_counter_percpu);
> + if (cpu_stats == NULL)
> + return ENOMEM;
> +
> + preempt_disable();
Seems unnecessary, the stats are not active durign ->init().
> + this_cpu = this_cpu_ptr(cpu_stats);
> + if (tb[NFTA_COUNTER_PACKETS]) {
> + this_cpu->counter.packets =
> + be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_PACKETS]));
> + }
> + if (tb[NFTA_COUNTER_BYTES]) {
> + this_cpu->counter.bytes =
> + be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_BYTES]));
> + }
> + preempt_enable();
> +
> + priv->counter = cpu_stats;
>
> - seqlock_init(&priv->lock);
> return 0;
> }
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH nf-next,v2] netfilter: nft_counter: convert it to use per-cpu counters
2015-06-19 9:39 ` Patrick McHardy
@ 2015-06-19 9:49 ` Pablo Neira Ayuso
2015-06-19 9:50 ` Patrick McHardy
2015-06-19 13:43 ` Marcelo Ricardo Leitner
0 siblings, 2 replies; 7+ messages in thread
From: Pablo Neira Ayuso @ 2015-06-19 9:49 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel, fw, eric.dumazet
On Fri, Jun 19, 2015 at 11:39:37AM +0200, Patrick McHardy wrote:
> On 19.06, Pablo Neira Ayuso wrote:
> > static void nft_counter_eval(const struct nft_expr *expr,
> > struct nft_regs *regs,
> > const struct nft_pktinfo *pkt)
> > {
> > - struct nft_counter *priv = nft_expr_priv(expr);
> > + struct nft_counter_percpu_priv *priv = nft_expr_priv(expr);
> > + struct nft_counter_percpu *this_cpu;
> >
> > - write_seqlock_bh(&priv->lock);
> > - priv->bytes += pkt->skb->len;
> > - priv->packets++;
> > - write_sequnlock_bh(&priv->lock);
> > + this_cpu = this_cpu_ptr(priv->counter);
> > + this_cpu->counter.bytes += pkt->skb->len;
> > + this_cpu->counter.packets++;
>
> u64_state_update_begin()/end()?
Right, will fix that, thanks!
> > @@ -67,23 +83,46 @@ static int nft_counter_init(const struct nft_ctx *ctx,
> > const struct nft_expr *expr,
> > const struct nlattr * const tb[])
> > {
> > - struct nft_counter *priv = nft_expr_priv(expr);
> > -
> > - if (tb[NFTA_COUNTER_PACKETS])
> > - priv->packets = be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_PACKETS]));
> > - if (tb[NFTA_COUNTER_BYTES])
> > - priv->bytes = be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_BYTES]));
> > + struct nft_counter_percpu_priv *priv = nft_expr_priv(expr);
> > + struct nft_counter_percpu __percpu *cpu_stats;
> > + struct nft_counter_percpu *this_cpu;
> > +
> > + cpu_stats = netdev_alloc_pcpu_stats(struct nft_counter_percpu);
> > + if (cpu_stats == NULL)
> > + return ENOMEM;
> > +
> > + preempt_disable();
>
> Seems unnecessary, the stats are not active durign ->init().
I know, I need to find a better way to silent a lockdep preempt splat
that I'm getting here because of illegal use of this_cpu_ptr() with
preemption enabled.
> > + this_cpu = this_cpu_ptr(cpu_stats);
> > + if (tb[NFTA_COUNTER_PACKETS]) {
> > + this_cpu->counter.packets =
> > + be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_PACKETS]));
> > + }
> > + if (tb[NFTA_COUNTER_BYTES]) {
> > + this_cpu->counter.bytes =
> > + be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_BYTES]));
> > + }
> > + preempt_enable();
> > +
> > + priv->counter = cpu_stats;
> >
> > - seqlock_init(&priv->lock);
> > return 0;
> > }
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH nf-next,v2] netfilter: nft_counter: convert it to use per-cpu counters
2015-06-19 9:49 ` Pablo Neira Ayuso
@ 2015-06-19 9:50 ` Patrick McHardy
2015-06-19 9:59 ` Pablo Neira Ayuso
2015-06-19 13:43 ` Marcelo Ricardo Leitner
1 sibling, 1 reply; 7+ messages in thread
From: Patrick McHardy @ 2015-06-19 9:50 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, fw, eric.dumazet
On 19.06, Pablo Neira Ayuso wrote:
> On Fri, Jun 19, 2015 at 11:39:37AM +0200, Patrick McHardy wrote:
> > > + struct nft_counter_percpu_priv *priv = nft_expr_priv(expr);
> > > + struct nft_counter_percpu __percpu *cpu_stats;
> > > + struct nft_counter_percpu *this_cpu;
> > > +
> > > + cpu_stats = netdev_alloc_pcpu_stats(struct nft_counter_percpu);
> > > + if (cpu_stats == NULL)
> > > + return ENOMEM;
> > > +
> > > + preempt_disable();
> >
> > Seems unnecessary, the stats are not active durign ->init().
>
> I know, I need to find a better way to silent a lockdep preempt splat
> that I'm getting here because of illegal use of this_cpu_ptr() with
> preemption enabled.
Ok guess it doesn't really hurt.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH nf-next,v2] netfilter: nft_counter: convert it to use per-cpu counters
2015-06-19 9:50 ` Patrick McHardy
@ 2015-06-19 9:59 ` Pablo Neira Ayuso
2015-06-19 10:00 ` Patrick McHardy
0 siblings, 1 reply; 7+ messages in thread
From: Pablo Neira Ayuso @ 2015-06-19 9:59 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel, fw, eric.dumazet
On Fri, Jun 19, 2015 at 11:50:12AM +0200, Patrick McHardy wrote:
> On 19.06, Pablo Neira Ayuso wrote:
> > On Fri, Jun 19, 2015 at 11:39:37AM +0200, Patrick McHardy wrote:
> > > > + struct nft_counter_percpu_priv *priv = nft_expr_priv(expr);
> > > > + struct nft_counter_percpu __percpu *cpu_stats;
> > > > + struct nft_counter_percpu *this_cpu;
> > > > +
> > > > + cpu_stats = netdev_alloc_pcpu_stats(struct nft_counter_percpu);
> > > > + if (cpu_stats == NULL)
> > > > + return ENOMEM;
> > > > +
> > > > + preempt_disable();
> > >
> > > Seems unnecessary, the stats are not active durign ->init().
> >
> > I know, I need to find a better way to silent a lockdep preempt splat
> > that I'm getting here because of illegal use of this_cpu_ptr() with
> > preemption enabled.
>
> Ok guess it doesn't really hurt.
I'm going to leave it in place, OK?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH nf-next,v2] netfilter: nft_counter: convert it to use per-cpu counters
2015-06-19 9:59 ` Pablo Neira Ayuso
@ 2015-06-19 10:00 ` Patrick McHardy
0 siblings, 0 replies; 7+ messages in thread
From: Patrick McHardy @ 2015-06-19 10:00 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, fw, eric.dumazet
On 19.06, Pablo Neira Ayuso wrote:
> On Fri, Jun 19, 2015 at 11:50:12AM +0200, Patrick McHardy wrote:
> > On 19.06, Pablo Neira Ayuso wrote:
> > > On Fri, Jun 19, 2015 at 11:39:37AM +0200, Patrick McHardy wrote:
> > > > > + struct nft_counter_percpu_priv *priv = nft_expr_priv(expr);
> > > > > + struct nft_counter_percpu __percpu *cpu_stats;
> > > > > + struct nft_counter_percpu *this_cpu;
> > > > > +
> > > > > + cpu_stats = netdev_alloc_pcpu_stats(struct nft_counter_percpu);
> > > > > + if (cpu_stats == NULL)
> > > > > + return ENOMEM;
> > > > > +
> > > > > + preempt_disable();
> > > >
> > > > Seems unnecessary, the stats are not active durign ->init().
> > >
> > > I know, I need to find a better way to silent a lockdep preempt splat
> > > that I'm getting here because of illegal use of this_cpu_ptr() with
> > > preemption enabled.
> >
> > Ok guess it doesn't really hurt.
>
> I'm going to leave it in place, OK?
Yeah, I don't see an alternative right now.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH nf-next,v2] netfilter: nft_counter: convert it to use per-cpu counters
2015-06-19 9:49 ` Pablo Neira Ayuso
2015-06-19 9:50 ` Patrick McHardy
@ 2015-06-19 13:43 ` Marcelo Ricardo Leitner
1 sibling, 0 replies; 7+ messages in thread
From: Marcelo Ricardo Leitner @ 2015-06-19 13:43 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Patrick McHardy, netfilter-devel, fw, eric.dumazet
On Fri, Jun 19, 2015 at 11:49:36AM +0200, Pablo Neira Ayuso wrote:
> On Fri, Jun 19, 2015 at 11:39:37AM +0200, Patrick McHardy wrote:
> > On 19.06, Pablo Neira Ayuso wrote:
> > > static void nft_counter_eval(const struct nft_expr *expr,
> > > struct nft_regs *regs,
> > > const struct nft_pktinfo *pkt)
> > > {
> > > - struct nft_counter *priv = nft_expr_priv(expr);
> > > + struct nft_counter_percpu_priv *priv = nft_expr_priv(expr);
> > > + struct nft_counter_percpu *this_cpu;
> > >
> > > - write_seqlock_bh(&priv->lock);
> > > - priv->bytes += pkt->skb->len;
> > > - priv->packets++;
> > > - write_sequnlock_bh(&priv->lock);
> > > + this_cpu = this_cpu_ptr(priv->counter);
> > > + this_cpu->counter.bytes += pkt->skb->len;
> > > + this_cpu->counter.packets++;
> >
> > u64_state_update_begin()/end()?
>
> Right, will fix that, thanks!
>
> > > @@ -67,23 +83,46 @@ static int nft_counter_init(const struct nft_ctx *ctx,
> > > const struct nft_expr *expr,
> > > const struct nlattr * const tb[])
> > > {
> > > - struct nft_counter *priv = nft_expr_priv(expr);
> > > -
> > > - if (tb[NFTA_COUNTER_PACKETS])
> > > - priv->packets = be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_PACKETS]));
> > > - if (tb[NFTA_COUNTER_BYTES])
> > > - priv->bytes = be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_BYTES]));
> > > + struct nft_counter_percpu_priv *priv = nft_expr_priv(expr);
> > > + struct nft_counter_percpu __percpu *cpu_stats;
> > > + struct nft_counter_percpu *this_cpu;
> > > +
> > > + cpu_stats = netdev_alloc_pcpu_stats(struct nft_counter_percpu);
> > > + if (cpu_stats == NULL)
> > > + return ENOMEM;
> > > +
> > > + preempt_disable();
> >
> > Seems unnecessary, the stats are not active durign ->init().
>
> I know, I need to find a better way to silent a lockdep preempt splat
> that I'm getting here because of illegal use of this_cpu_ptr() with
> preemption enabled.
If preemption is safe, maybe you can use raw_cpu_ptr() instead?
Marcelo
> > > + this_cpu = this_cpu_ptr(cpu_stats);
> > > + if (tb[NFTA_COUNTER_PACKETS]) {
> > > + this_cpu->counter.packets =
> > > + be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_PACKETS]));
> > > + }
> > > + if (tb[NFTA_COUNTER_BYTES]) {
> > > + this_cpu->counter.bytes =
> > > + be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_BYTES]));
> > > + }
> > > + preempt_enable();
> > > +
> > > + priv->counter = cpu_stats;
> > >
> > > - seqlock_init(&priv->lock);
> > > return 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
>
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-06-19 13:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-19 9:36 [PATCH nf-next,v2] netfilter: nft_counter: convert it to use per-cpu counters Pablo Neira Ayuso
2015-06-19 9:39 ` Patrick McHardy
2015-06-19 9:49 ` Pablo Neira Ayuso
2015-06-19 9:50 ` Patrick McHardy
2015-06-19 9:59 ` Pablo Neira Ayuso
2015-06-19 10:00 ` Patrick McHardy
2015-06-19 13:43 ` Marcelo Ricardo Leitner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).