Subject: Re: [PATCH RFC 1/2] flow: virtualize get and entry deletion methods

From: Timo Teräs <[email-masked]>
Date: Mon, 29 Mar 2010 12:00:38 +0300

Herbert Xu wrote:
> On Thu, Mar 25, 2010 at 12:26:11PM -0700, David Miller wrote:
>> Maybe we can make the dead state check safe to do asynchronously
>> somehow?  I wonder if the policy layer is overdue for an RCU
>> conversion or similar.
> 
> In fact, now that I read this again, we don't even need to grab
> the lock to perform the deadness check.  This is because the
> existing never did it anyway.  The liveliness is guaranteed by
> the policy destruction code doing a synchronous cache flush.
> 
> Timo, what was the reason for getting rid of the synchronous
> flush again?

No, just having the flush call is not enough to guarantee
liveliness. The flushing happens in delayed work, and the flows
might be in use before the flush has been finished or even
started.

I was also hoping to move the "delayed" deletion part to
flow cache core so the code would be shared with policies and
bundles.

As stated before, we don't really need lock for the 'dead' check.
It's only written once, and actually read without lock in some
other places too. And all the other places that do take the lock,
release it right away making the resulting dead check result
"old" anyway. 

Looks to me that the whole policy locking is not up-to-date
anymore. Apparently the only place that actually needs it is
the migration code (which just happens to be broke anyway since
bundle creation does not take read lock). But it could be
relatively easily changed to replace policy with new
templates... and the whole policy stuff converted to RCU.

However, now that I've almost done the code ready. I'm thinking
if this is such a good idea or not. I was hoping to have bundles
always in the flow cache, but it's not sufficient. In case we
have socket bound policy that results in a bundle, we might need
create bundles outside the flow cache.

It turns out the generic flow cache might not be so easily
done that could host bundles and policies. Or at least the
shared code would not be as much as hoped for. Given that it
makes also sense to store other objects for outgoing stuff
(we might need reference to multiple policies if matching
a sub and main policy), it might be a consideration to make
the flow cache specialized to contain those objects. Or do
we have other possible users for the flow cache?

- Timo