Re: connbytes & 64bit counters

[Patrick McHardy <kaber@trash.net>]

Krzysztof Oledzki wrote:
> OK, maybe something like this can be accepted - no ifdefs, 64 bit
> counters unconditionally for both ip_conntrack/nf_conntrack. If someone
> needs to save memory it is IMHO better to simply disable accounting at all.


Or ideally have a runtime option. Something to keep in mind when
redesigning the nf_conntrack allocation scheme.

> [NETFILTER] Reimplementation of 64bit counters for bytes/packets accounting
> 
> 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 with "connbytes" extension.
> 
> Signed-off-by: Krzysztof Piotr Oledzki <ole@ans.pl>
> 
> diff -Nur
> linux-2.6.19.1-orig/include/linux/netfilter/nf_conntrack_common.h
> linux-2.6.19.1-64bitconntrack/include/linux/netfilter/nf_conntrack_common.h
> --- linux-2.6.19.1-orig/include/linux/netfilter/nf_conntrack_common.h   
> 2006-12-11 20:32:53.000000000 +0100
> +++
> linux-2.6.19.1-64bitconntrack/include/linux/netfilter/nf_conntrack_common.h   
> 2006-12-22 21:15:10.000000000 +0100
> @@ -122,7 +122,7 @@
>      IPCT_NATINFO_BIT = 10,
>      IPCT_NATINFO = (1 << IPCT_NATINFO_BIT),
> 
> -    /* Counter highest bit has been set */
> +    /* Counter highest bit has been set - UNUSED */
>      IPCT_COUNTER_FILLING_BIT = 11,
>      IPCT_COUNTER_FILLING = (1 << IPCT_COUNTER_FILLING_BIT),


No need to keep this.

> @@ -139,8 +139,8 @@
>  #ifdef __KERNEL__
>  struct ip_conntrack_counter
>  {
> -    u_int32_t packets;
> -    u_int32_t bytes;
> +    u_int64_t packets;
> +    u_int64_t bytes;
>  };
> 
>  struct ip_conntrack_stat
> diff -Nur
> linux-2.6.19.1-orig/include/linux/netfilter/nfnetlink_conntrack.h
> linux-2.6.19.1-64bitconntrack/include/linux/netfilter/nfnetlink_conntrack.h
> --- linux-2.6.19.1-orig/include/linux/netfilter/nfnetlink_conntrack.h   
> 2006-12-11 20:32:53.000000000 +0100
> +++
> linux-2.6.19.1-64bitconntrack/include/linux/netfilter/nfnetlink_conntrack.h   
> 2006-12-22 21:15:26.000000000 +0100
> @@ -89,10 +89,10 @@
> 
>  enum ctattr_counters {
>      CTA_COUNTERS_UNSPEC,
> -    CTA_COUNTERS_PACKETS,        /* old 64bit counters */
> -    CTA_COUNTERS_BYTES,        /* old 64bit counters */
> -    CTA_COUNTERS32_PACKETS,
> -    CTA_COUNTERS32_BYTES,
> +    CTA_COUNTERS_PACKETS,        /* 64bit counters */
> +    CTA_COUNTERS_BYTES,        /* 64bit counters */
> +    CTA_COUNTERS32_PACKETS,        /* unused */
> +    CTA_COUNTERS32_BYTES,        /* unused */
>      __CTA_COUNTERS_MAX
>  };
>  #define CTA_COUNTERS_MAX (__CTA_COUNTERS_MAX - 1)
> diff -Nur linux-2.6.19.1-orig/net/ipv4/netfilter/ip_conntrack_core.c
> linux-2.6.19.1-64bitconntrack/net/ipv4/netfilter/ip_conntrack_core.c
> --- linux-2.6.19.1-orig/net/ipv4/netfilter/ip_conntrack_core.c   
> 2006-12-11 20:32:53.000000000 +0100
> +++
> linux-2.6.19.1-64bitconntrack/net/ipv4/netfilter/ip_conntrack_core.c   
> 2006-12-22 21:13:35.000000000 +0100
> @@ -1148,9 +1148,6 @@
>          ct->counters[CTINFO2DIR(ctinfo)].packets++;
>          ct->counters[CTINFO2DIR(ctinfo)].bytes +=
>                          ntohs(skb->nh.iph->tot_len);
> -        if ((ct->counters[CTINFO2DIR(ctinfo)].packets & 0x80000000)
> -            || (ct->counters[CTINFO2DIR(ctinfo)].bytes & 0x80000000))
> -            event |= IPCT_COUNTER_FILLING;


This was actually broken before, since the counters are not
reset (they just overflow) an event was generated for every
packet until the overflow once they reached 2^31. Anyway,
I'm not sure how ulogd2 uses these counters, Harald, is it
necessary to receive period updates?