[RFC] Event Cache per-cpu

[RFC] Event Cache per-cpu

[Pablo Neira]

Hi all,

I've been discussing this issue with Patrick during last week. I've 
found two problems with the current approach of a event cache per cpu. 
The problem is related to 1) preemption, 2) the fact that 
ip_conntrack_in can be called from process and softirq context.

1) If preemption is enable, a calling process walking on ip_conntrack_in 
could be preempted by other process which has more priority on the same CPU.

2) A calling process walking on ip_conntrack can be preempted by a 
softirq which could walk the same piece of bits on the same CPU.

In both cases, the event cache will be corrupted. In resume, I see three 
possibilities:

a) per-cpu event cache. Won't work, even if we disable preemption in 
ip_conntrack_in, we can still have problems with softirqs preempting a 
calling process.

b) per conntrack cache. It won't work, in a SMP environment, two packets 
of the same connection can be handled by two different CPU's.

c) per packet cache. I don't see any possible race with this approach at 
the moment, but I'll need to use the nfcache field in skbuff. I prefer 
this than adding a new field to a skbuff, I think that Davem won't like 
that.

There's 15 bits in nfcache available now (because IPVS guys are using 
one in private). My last patch has 11 events.

enum ip_conntrack_events
{
       IPCT_NEW,                       /* New conntrack                */
       IPCT_RELATED,                   /* Expected connection          */
       IPCT_DESTROY,                   /* Destroyed conntrack          */
       IPCT_STATUS,                    /* Status has changed           */
       IPCT_REFRESH,                   /* Timer has been refreshed     */
       IPCT_PROTOINFO,                 /* Update of protocol info      */
       IPCT_PROTOINFO_VOLATILE,        /* Volatile protocol info       */
       IPCT_HELPER,                    /* New helper for conntrack     */
       IPCT_HELPINFO,                  /* Update of helper info        */
       IPCT_HELPINFO_VOLATILE,         /* Volatile helper info         */
       IPCT_NATINFO,                   /* NAT info                     */
};

Patrick suggests that we could run out of bits soon if events go in 
nfcache, that's true. To fix that, could we remove this stuff in 
netfilter_ipv4.h, nobody is using it ?

#define NFC_IP_SRC              0x0001
/* Dest IP address. */
#define NFC_IP_DST              0x0002
...

Please, comments welcome.

Pablo

Re: [RFC] Event Cache per-cpu

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 2173 bytes --]

On Mon, Nov 08, 2004 at 08:29:38PM +0100, Pablo Neira wrote:
> Hi all,
> c) per packet cache. I don't see any possible race with this approach at 
> the moment, but I'll need to use the nfcache field in skbuff. I prefer 
> this than adding a new field to a skbuff, I think that Davem won't like 
> that.

seems fine to me.

> There's 15 bits in nfcache available now (because IPVS guys are using 
> one in private). My last patch has 11 events.

who authorized IPVS people to use that bit ? It's ours, and we fought
hard for it  ;)  [just kidding, but I didn't know that before].

> enum ip_conntrack_events
> {
>       IPCT_NEW,                       /* New conntrack                */
>       IPCT_RELATED,                   /* Expected connection          */
>       IPCT_DESTROY,                   /* Destroyed conntrack          */
>       IPCT_STATUS,                    /* Status has changed           */
>       IPCT_REFRESH,                   /* Timer has been refreshed     */
>       IPCT_PROTOINFO,                 /* Update of protocol info      */
>       IPCT_PROTOINFO_VOLATILE,        /* Volatile protocol info       */
>       IPCT_HELPER,                    /* New helper for conntrack     */
>       IPCT_HELPINFO,                  /* Update of helper info        */
>       IPCT_HELPINFO_VOLATILE,         /* Volatile helper info         */
>       IPCT_NATINFO,                   /* NAT info                     */
> };
> 
> Patrick suggests that we could run out of bits soon if events go in 
> nfcache, that's true. To fix that, could we remove this stuff in 
> netfilter_ipv4.h, nobody is using it ?

No, I think this can be removed, nobody has ever used nfcache the way it
was originally intended.

> Please, comments welcome.
> Pablo

-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [RFC] Event Cache per-cpu

[Pablo Neira]

Harald Welte wrote:

>On Mon, Nov 08, 2004 at 08:29:38PM +0100, Pablo Neira wrote:
>  
>
>>Hi all,
>>c) per packet cache. I don't see any possible race with this approach at 
>>the moment, but I'll need to use the nfcache field in skbuff. I prefer 
>>this than adding a new field to a skbuff, I think that Davem won't like 
>>that.
>>    
>>
>
>seems fine to me.
>  
>

tony the tiger says grrrreat :)

>>There's 15 bits in nfcache available now (because IPVS guys are using 
>>one in private). My last patch has 11 events.
>>    
>>
>
>who authorized IPVS people to use that bit ? It's ours, and we fought
>hard for it  ;)  [just kidding, but I didn't know that before].
>  
>

I'll sync this between IPVS and netfilter. Make them move that to 
netfilter.h if they really need it. We'll see, later.

>>enum ip_conntrack_events
>>{
>>      IPCT_NEW,                       /* New conntrack                */
>>      IPCT_RELATED,                   /* Expected connection          */
>>      IPCT_DESTROY,                   /* Destroyed conntrack          */
>>      IPCT_STATUS,                    /* Status has changed           */
>>      IPCT_REFRESH,                   /* Timer has been refreshed     */
>>      IPCT_PROTOINFO,                 /* Update of protocol info      */
>>      IPCT_PROTOINFO_VOLATILE,        /* Volatile protocol info       */
>>      IPCT_HELPER,                    /* New helper for conntrack     */
>>      IPCT_HELPINFO,                  /* Update of helper info        */
>>      IPCT_HELPINFO_VOLATILE,         /* Volatile helper info         */
>>      IPCT_NATINFO,                   /* NAT info                     */
>>};
>>
>>Patrick suggests that we could run out of bits soon if events go in 
>>nfcache, that's true. To fix that, could we remove this stuff in 
>>netfilter_ipv4.h, nobody is using it ?
>>    
>>
>
>No, I think this can be removed, nobody has ever used nfcache the way it
>was originally intended.
>  
>

Ok, expect a new patch in next days.

Pablo