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: <20100319093210.GA23895@gondor.apana.org.au>
Date:	Fri, 19 Mar 2010 17:32:10 +0800
From:	Herbert Xu <herbert@...dor.apana.org.au>
To:	Timo Teräs <timo.teras@....fi>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH] xfrm: cache bundle lookup results in flow cache

On Fri, Mar 19, 2010 at 11:12:21AM +0200, Timo Teräs wrote:
>
>> That would be better but it's still hacky.  Proper reference
>> counting like we had before would be my preference.
>
> Well, the cache entry is still referenced only very shortly,
> I don't see why keeping bh disabled why doing it is considered
> a hack. Refcounting the cache entries is trickier. Though,
> it could be used to optimize the update process: we could safely
> update it instead of doing now lookup later.

Well we had a nicely type-agnostic cache which is self-contained,
but your patch is bleeding generic code into xfrm_policy.c, that's
why I felt it to be hacky :)

Anyway I see how your scheme works now as far as object life
is concerned, and I agree that it is safe.

However, I wonder if we could do it while still leaving all the
object life-cycle management stuff (and the BH disabling bits)
in flow.c

The crux of the issue is that you now have two objects to track
instead of one.  As the direction is a key in the lookup, we're
really only worried about the outbound case here.

So how about going back to what I suggested earlier, and keeping
a back-pointer from xfrm_dst to the policy? Of course xfrm_dst
would also hold a ref count on the policy.  You'd only have to
do it for the top-level xfrm_dst.

It does mean that you'll need to write a different resolver for
outbound vs. inbound/forward, but that makes sense because we
only use bundles for outbound policies.

What do you think?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
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