On Fri, Feb 03, 2017 at 11:16:16AM +0200, Julian Anastasov wrote:
> 
>       Hello,
> 
> On Fri, 3 Feb 2017, Steffen Klassert wrote:
> > 
> > I thought about this (completely untested) one:
> > 
> > static void xfrm_confirm_neigh(const struct dst_entry *dst, const void
> > *daddr)
> > 
> > {
> >     const struct dst_entry *dst = dst->child;
> 
>       When starting and dst arg is first xform, the above
> assignment skips it. May be both lines should be swapped.

Yes, that's better :)

> 
> >     const struct xfrm_state *xfrm = dst->xfrm;
> > 
> >     if (xfrm)
> >             daddr = &xfrm->id.daddr;
> > 
> >     dst->ops->confirm_neigh(dst, daddr);
> > }
> > 
> > Only the last dst_entry in this call chain (path) sould
> > not have dst->xfrm set. So it finally calls path->ops->confirm_neigh
> > with the daddr of the last transformation. But your version
> > should do the same.
> 
>       Above can be fixed but it is risky for the stack
> usage when using recursion. In practice, there should not be
> many xforms, though. Also, is id.daddr valid for transports?

Yes, it is needed for the lookup. But id.daddr ist the same
as daddr of the packet on transport mode.

> 
> > >   This should work as long as path and last tunnel are
> > > from same family.
> > 
> > Yes, the outer mode of the last transformation has the same
> > family as path.
> > 
> > > Also, after checking xfrm_dst_lookup() I'm not
> > > sure using just &xfrm->id.daddr is enough. Should we consider
> > > more places for daddr value?
> > 
> > Yes, indeed. We should do it like xfrm_dst_lookup() does it.
> 
>       OK, I'll get logic from there. Should I use loop or
> recursion?

I don't have a strong opinion on that. Both should work,
choose whatever you prefer.

Reply via email to