Re: [PATCHv4 net-next] xfrm: Namespacify xfrm_policy_sk_bundles

[Timo Teras <timo.teras@iki.fi>]

On Wed, 25 Dec 2013 14:40:36 +0800
Fan Du <fan.du@windriver.com> wrote:

> ccing Timo
> 
> On 2013年12月24日 18:35, Steffen Klassert wrote:
> > On Fri, Dec 20, 2013 at 11:34:41AM +0800, Fan Du wrote:
> >>
> >> Subject: [PATCHv4 net-next] xfrm: Namespacify
> >> xfrm_policy_sk_bundles
> >>
> >> xfrm_policy_sk_bundles, protected by
> >> net->xfrm.xfrm_policy_sk_bundle_lock should be put into netns xfrm
> >> structure, otherwise xfrm_policy_sk_bundles can be corrupted from
> >> different net namespace.
> >
> > I'm ok with this patch, but I wonder where we use these cached
> > socket bundles. After a quick look I see where we add and where we
> > delete them, but I can't see how we use these cached bundles.
> 
> Interesting
> 
> The per socket bundles is introduced by Timo in commit 80c802f3
> ("xfrm: cache bundles instead of policies for outgoing flows")

Those existed even before. I just did systematic transformation of the
caching code to work on bundle level instead of policy level.

> But one fundamental question is why not use existing flow cache
> for per socket bundles as well? then no need to create such per
> sock xdst for every packet, and also share the same flow cache
> flush mechanism.

It was needed when the flow cache cached policies. They explicitly
needed to check the socket for per-socket policy. So it made no sense
to have anything socket related in the cache.

> My first impression is it can be done this way, I'm going to head
> this way unless turn out otherwise.

I think it could converted. You'll still need to look up the per-socket
policies. But if caching the bundles in flow cache simplifies overall
code it sounds like a good thing to do.

- Timo