netdev - Re: [PATCH] xfrm: cache bundle lookup results in flow cache

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4BA5CC16.9040606@iki.fi>
Date:	Sun, 21 Mar 2010 09:34:46 +0200
From:	Timo Teräs <timo.teras@....fi>
To:	Herbert Xu <herbert@...dor.apana.org.au>
CC:	netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH] xfrm: cache bundle lookup results in flow cache

Herbert Xu wrote:
> On Sat, Mar 20, 2010 at 06:26:00PM +0200, Timo Teräs wrote:
>> So should go ahead and:
>> 1. modify flow cache to be more generic (have virtual put and get
>>   for each object; and remove the atomic_t pointer)
>> 2. modify flow cache to have slow and fast resolvers so we can
>>   copy with the current sleeping requirement
> 
> I don't think we need either of these.  To support the sleep
> requirement, just return -EAGAIN from the resolver when the
> template can't be resolved.  Then the caller of flow_cache_lookup
> can sleep as it does now.  It simply has to repeat the flow
> cache lookup afterwards.

Ok, we can do that to skip 2. But I think 1 would be still useful.
It'd probably be good to actually have flow_cache_ops pointer in
each entry instead of the atomic_t pointer.

The reasoning:
- we can then have type-based checks that the reference count
  is valid (e.g. policy's refcount must not go to zero, it's bug,
  and we can call dst_release which warns if refcount goes to
  negative); imho it's hack to call atomic_dec instead of the
  real type's xxx_put
- the flow cache needs to somehow know if the entry is stale so
  it'll try to refresh it atomically; e.g. if there's no
  check for 'stale', the lookup returns stale xfrm_dst. we'd
  then need new api to update the stale entry, or flush it out
  and repeat the lookup. the virtual get could check for it being
  stale (if so release the entry) and then return null for the
  generic code to call the resolver atomically
- for paranoia we can actually check the type of the object in
  cache via the ops (if needed)

>> 3. cache bundles instead of policies for outgoing stuff
>> 4. kill find_bundle and just instantiate new ones if we get cache
>>   miss
>> 5. put all bundles to global hlist (since only place that walks
>>   through them is gc, and stale bundle can be dst_free'd right
>>   away); use genid's for policy to flush old bundles
>> 6. dst_free and unlink bundle immediately if it's found to be stale
> 
> Sounds good.

Okay. Sounds like a plan. I'll work on this next week.
I'll also try to make it a series of patches instead of the big
hunk I sent initially.

- Timo

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html