Re: [PATCHv3 net-next 5/7] net: add confirm_neigh method to dst_ops

On Fri, Feb 03, 2017 at 11:16:16AM +0200, Julian Anastasov wrote:

	Hello,

On Fri, 3 Feb 2017, Steffen Klassert wrote:

quoted

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 :)

quoted

	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.

quoted

quoted

	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.

quoted

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html