On Tue, 3 Jun 2008, Patrick McHardy wrote:
Krzysztof Oledzki wrote:
[NETFILTER] Accounting rework: ct_extend + 64bit counters
Initially netfilter has had 64bit counters for conntrack-based accounting,
but
it was changed in 2.6.14 to save memory. Unfortunately in-kernel 64bit
counters are
still required, for example for "connbytes" extension. However, 64bit
counters
waste a lot of memory and it was not possible to enable/disable it runtime.
This patch:
- reimplements accounting with respect to the extension infrastructure,
- makes one global version of seq_print_acct() instead of two
seq_print_counters(),
- makes it possible to enable it at boot time (for SYSFS=n),
- makes it possible to enable/disable it at runtime by sysctl or sysfs,
- extents counters from 32bit to 64bit,
- enables accounting code unconditionally (no more CONFIG_NF_CT_ACCT),
- keeps accounting disabled by default,
- removes buggy IPCT_COUNTER_FILLING event handling.
If accounting is enabled newly created connections get additional acct
extent.
Old connections are not changed as it is not possible to add a ct_extend
area
to confirmed conntrack. Accounting is performed for all connections with
acct extent regardless of a current state of
net.netfilter.nf_conntrack_acct.
Patch against 2.6.26-rc4, it would be nice if it can be included
in 2.6.27.
Thanks for working on this. Some minor comments below:
Thanks,
Signed-off-by: Krzysztof Piotr Oledzki <ole@xxxxxx>
diff -Nur linux-2.6.26-rc4-net/Documentation/kernel-parameters.txt
linux-2.6.26-rc4-64bitc/Documentation/kernel-parameters.txt
--- linux-2.6.26-rc4-net/Documentation/kernel-parameters.txt 2008-05-26
20:08:11.000000000 +0200
+++ linux-2.6.26-rc4-64bitc/Documentation/kernel-parameters.txt
2008-06-02 18:28:30.000000000 +0200
@@ -1234,6 +1234,11 @@
This usage is only documented in each driver source
file if at all.
+ nf_conntrack.acct=
+ [NETFILTER] Enable connection tracking flow
accounting
+ 0 to disable accounting (default)
+ 1 to enable accounting
Changing the default will probably result in surprises.
How about we make a config option (CONFIG_NF_ACCT_COMPAT)
that makes it default to 1 and print a warning that this
option is going to be removed/the default changed. Then
we add a target to manually enable accounting on a per-connection
base and kill off the compat option after a couple of
month.
As far as I know there is now way to enable accounting on a per-connection
base with a target as it is not possible to ad ct_extend to confirmed
conntracks. However, I think we may still use CONFIG_NF_CT_ACCT but only
to set a default value of this (nf_ct_acct) variable, is that acceptable?
diff -Nur
linux-2.6.26-rc4-net/include/linux/netfilter/nf_conntrack_common.h
linux-2.6.26-rc4-64bitc/include/linux/netfilter/nf_conntrack_common.h
--- linux-2.6.26-rc4-net/include/linux/netfilter/nf_conntrack_common.h
2008-05-26 20:08:11.000000000 +0200
+++ linux-2.6.26-rc4-64bitc/include/linux/netfilter/nf_conntrack_common.h
2008-06-02 17:19:05.000000000 +0200
@@ -122,10 +122,6 @@
IPCT_NATINFO_BIT = 10,
IPCT_NATINFO = (1 << IPCT_NATINFO_BIT),
- /* Counter highest bit has been set */
- IPCT_COUNTER_FILLING_BIT = 11,
- IPCT_COUNTER_FILLING = (1 << IPCT_COUNTER_FILLING_BIT),
-
This is exposed to userspace and needs to stay to avoid breaking
compilation.
OK.
+unsigned int
+seq_print_acct(struct seq_file *s, const struct nf_conn *ct, int dir)
+{
+ struct nf_conn_acct *acct;
+
+ acct = nf_conn_acct_find(ct);
+ if (!acct)
+ return 0;
+
+ return seq_printf(s, "packets=%llu bytes=%llu ",
+ acct->packets[dir],
+ acct->bytes[dir]);
Will probably cause warnings on 64bit.
OK, so we need here something like "(unsigned long long)", right?
Best regards,
Krzysztof Olędzki
