Re: [PATCH] Accounting rework: ct_extend + 64bit counters (ver. 2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Looks pretty good, just one or two minor things below. What would
be nice though is to reduce patch size a bit, two suggestions for
this are:

- move seq_print_acct() changes to seperate cleanup patch
- keep counters as packets/bytes struct to reduce rename
  noise in connlimit
- maybe (hardly worth it) pull out the netlink.h change

Krzysztof Oledzki wrote:
+int nf_conntrack_acct_init(void)
+{
+	int ret;
+
+#ifdef CONFIG_NF_CT_ACCT
+	printk(KERN_ERR "CONFIG_NF_CT_ACCT is deprecated and will be removed soon.\n");
+#endif

This should point out what people are supposed to do
(use the new runtime option). KERN_WARN or NOTICE
seems more appropriate.

+
+	ret = nf_ct_extend_register(&acct_extend);
+	if (ret < 0) {
+		printk(KERN_ERR "nf_conntrack_acct: Unable to register extension\n");
+		return ret;
+	}
+
+#ifdef CONFIG_SYSCTL
+	acct_sysctl_header = register_sysctl_paths(nf_net_netfilter_sysctl_path, acct_sysctl_table);
+#endif

Please stay under the 80 character limit. Also should check whether
registration succeeded.

 struct hlist_head *nf_ct_alloc_hashtable(unsigned int *sizep, int *vmalloced)
@@ -1165,6 +1174,10 @@ int __init nf_conntrack_init(void)
 	if (ret < 0)
 		goto out_fini_expect;
+ ret = nf_conntrack_acct_init();
+	if (ret < 0)
+		goto out_fini_acct;
+
 	/* For use by REJECT target */
 	rcu_assign_pointer(ip_ct_attach, nf_conntrack_attach);
 	rcu_assign_pointer(nf_ct_destroy, destroy_conntrack);
@@ -1177,6 +1190,7 @@ int __init nf_conntrack_init(void)
return ret; +out_fini_acct:

nf_conntrack_helper_fini(); ?

 out_fini_expect:
 	nf_conntrack_expect_fini();
 out_fini_proto:
--
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

[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux