Re: [PATCH 2/2] netfilter: conntrack: optional reliable conntrack event delivery

[Pablo Neira Ayuso <pablo@netfilter.org>]

Patrick McHardy wrote:
> Eric Dumazet wrote:
>> Pablo Neira Ayuso a écrit :
>>> OK :). Eric, could you tell what if this patch is OK? It's based on the
>>> RCU version.
>>>
>> Sure, this patch is fine !
>>
>> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
> 
> Thanks Eric. Pablo, please send me those patch as soon as possible,
> I plan to send my tree upstream today. Thanks!

I'll do in a couple of minutes: I have another issue that you're going
to notice in the new patchset, I put it here before posting them if you
have time to look at it. I'd like to know if you're OK with this.

events = xchg(&e->cache, 0);
[...]
-               notify->fcn(events, &item);
+               ret = notify->fcn(events, &item);
+               if (likely(ret == 0))
+                       delivered = 1;
+       }
+       if (unlikely(!delivered)) {
+               unsigned int old = 0;
+retry:
+               /* restore undelivered events to the cache */
+               ret = cmpxchg(&e->cache, old, events);
+               /* ... but we've got new events during delivery */
+               if (unlikely(ret != old)) {
+                       old = ret;
+                       events |= ret;
+                       goto retry;
+               }
        }

 out_unlock:

To avoid races between the cache clearing and event delivery:

1) I retrieve the event cache and clear it with xchg.
2) Then, if I fail to deliver the event, I restore the cache. However,
if the event cache is not zero anymore, then some new events have been
cached during the delivery, I use cmpxchg to conditionally restore the
events without losing the new events.

Can you see any problem with this optimistic approach? I know, it can
potentially try to restore the cache, but this is very unlikely to happen?

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html