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]

Date:	Sun, 21 Mar 2010 10:31:23 +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

Timo Teräs wrote:
> 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)

- could cache bundle OR policy for outgoing stuff. it's useful
  to cache the policy in case we need to sleep, or if it's a
  policy forbidding traffic. in those cases there's no bundle
  to cache at all. alternatively we can make dummy bundles that
  are marked invalid and are just used to keep a reference to
  the policy.

Oh, this also implies that the resolver function should be
changed to get the old stale object so it can re-use it to
get the policy object instead of searching it all over again.

- 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