On Fri, Dec 23, 2011 at 9:42 PM, <pablo@xxxxxxxxxxxxx> wrote:
> +
> +static LIST_HEAD(nfnl_acct_list);
You suppose that there won't be many accounting instants. It isn't
scalable, as we have to iterate the list to find a special item. How
about a hash table?
> +
> +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) {
You should use the RCU variant here.
> + 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) {
> + atomic64_set(&cur->pkts, 0);
> + atomic64_set(&cur->bytes, 0);
The get and zero operations should be done in an atomic context,
otherwise counters added between them will be lost.
--
Regards,
Changli Gao(xiaosuo@xxxxxxxxx)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
