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.