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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



	Hello,

On Fri, 3 Feb 2017, Steffen Klassert wrote:

> On Thu, Feb 02, 2017 at 01:04:34AM +0200, Julian Anastasov wrote:
> > 
> > 	It may sounds good. But only dst->path->ops->confirm_neigh
> > points to real IPv4/IPv6 function. And also, I guess, the
> > family can change while walking the chain, so we should be
> > careful while providing the original daddr (which comes from
> > sendmsg). I had the idea to walk all xforms to get the latest
> > tunnel address but this can be slow. 
> 
> Is this a per packet call or is the information cached somewhere?

	It is for every packet that is sent with both
MSG_CONFIRM and MSG_PROBE, i.e. when nothing is sent
on the wire. It is used by patch 6 just for UDP, RAW, ICMP, L2TP.

> > Something like this?:
> > 
> > static void xfrm_confirm_neigh(const struct dst_entry *dst, const void 
> > *daddr)
> > {
> > 	const struct dst_entry *path = dst->path;
> > 
> > 	/* By default, daddr is from sendmsg() if we have no tunnels */
> > 	for (;dst != path; dst = dst->child) {
> > 		const struct xfrm_state *xfrm = dst->xfrm;
> > 
> > 		/* Use address from last tunnel	*/
> > 		if (xfrm->props.mode != XFRM_MODE_TRANSPORT)
> > 			daddr = &xfrm->id.daddr;
> > 	}
> > 	path->ops->confirm_neigh(path, daddr);
> > }
> 
> 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.

> 	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?

> > 	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?

Regards

--
Julian Anastasov <ja@xxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Networking Development]     [Linux OMAP]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux