On Tue, 24 Jun 2008, Patrick McHardy wrote:
A few minor nits - a patch on top to fix them is fine, I'll
just fold them together.
I prefer to send a new patch if that is OK for you.
<CUT>
+int nf_conntrack_acct_init(void);
+void nf_conntrack_acct_fini(void);
extern for function declarations please.
Ack.
+static inline
+struct ip_conntrack_counter *nf_conn_acct_find(const struct nf_conn *ct)
+{
+ return nf_ct_ext_find(ct, NF_CT_EXT_ACCT);
+}
+
+struct ip_conntrack_counter *nf_ct_acct_ext_add(struct nf_conn *ct, gfp_t gfp);
+
Also using ip_conntrack in the
struct name doesn't make much sense, please use nf_conntrack as prefix.
Or maybe for consistency with the other extensions, nf_conn.
Fine, I'll rename it to nf_conn_counter. Please note that the
ip_conntrack_counter comes from nf_conntrack_common.h and is already used
in several places. Isn't it a pointless rename?
+static unsigned int nf_ct_acct __read_mostly = NF_CT_ACCT_DEFAULT;
Perhaps this should be bool instead?
Yep.
+struct ip_conntrack_counter *nf_ct_acct_ext_add(struct nf_conn *ct, gfp_t
gfp)
+{
+ struct ip_conntrack_counter *acct;
+
+ if (!nf_ct_acct)
+ return NULL;
+
+ acct = nf_ct_ext_add(ct, NF_CT_EXT_ACCT, gfp);
+ if (!acct)
+ pr_debug("failed to add accounting extension area");
+
+ return acct;
+};
Missing EXPORT_SYMBOL_GPL for ctnetlink:
ERROR: "nf_ct_acct_ext_add" [net/netfilter/nf_conntrack_netlink.ko]
undefined!
Perhaps this should be inlined or at least the nf_ct_acct
check so we can avoid the unnecessary function call for
disabled accounting?
Indeed, this function is quite short, I'll inline it.
+#ifdef CONFIG_NF_CT_ACCT
+ printk(KERN_WARNING "CONFIG_NF_CT_ACCT is deprecated and will be
removed soon. Plase use\n");
+ printk(KERN_WARNING "nf_conntrack.acct=1 kernel paramater, acct=1
nf_conntrack module option or\n");
+ printk(KERN_WARNING "sysctl net.netfilter.nf_conntrack_acct=1 to
enable it.\n");
Since sysctls are deprecated (or are they still?), better to just
mention the kernel and module options.
No, sysctls are not deprecated and allow to enable/disable accouting
at runtime - kernel/module options allow to enable/disable accouting at
boot time. We only deprecate CONFIG_NF_CT_ACCT.
diff --git a/net/netfilter/nf_conntrack_netlink.c
b/net/netfilter/nf_conntrack_netlink.c
index ab655f6..165e7f3 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -229,9 +234,6 @@ ctnetlink_dump_counters(struct sk_buff *skb, const
struct nf_conn *ct,
nla_put_failure:
return -1;
}
-#else
-#define ctnetlink_dump_counters(a, b, c) (0)
-#endif
#ifdef CONFIG_NF_CONNTRACK_MARK
static inline int
@@ -389,8 +391,8 @@ ctnetlink_fill_info(struct sk_buff *skb, u32 pid, u32
seq,
if (ctnetlink_dump_status(skb, ct) < 0 ||
ctnetlink_dump_timeout(skb, ct) < 0 ||
- ctnetlink_dump_counters(skb, ct, IP_CT_DIR_ORIGINAL) < 0 ||
- ctnetlink_dump_counters(skb, ct, IP_CT_DIR_REPLY) < 0 ||
+ ctnetlink_dump_acct(skb, ct, IP_CT_DIR_ORIGINAL) < 0 ||
+ ctnetlink_dump_acct(skb, ct, IP_CT_DIR_REPLY) < 0 ||
This rename seems pointless. I also like the old name better.
Fine.
Best regards,
Krzysztof Olędzki
