Re: [PATCH] Accounting rework: ct_extend + 64bit counters

[Patrick McHardy <kaber@trash.net>]

Krzysztof Oledzki wrote:
> On Tue, 3 Jun 2008, Patrick McHardy wrote:
> 
>>>  +    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.

You can add it to unconfirmed conntracks though.

> 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?

We should move towards getting rid of the default value,
having this depend on a config option must only be a temporary
solution. So we'd still need a target to enable it manually
and some kind of warning.

>>> +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?

Yes.