Subject: Re: [3/4] [IPSEC] Add route element to xfrm_dst

From: Patrick McHardy <[email-masked]>
Date: Mon, 14 Mar 2005 12:52:16 +0100

Herbert Xu wrote:
> This patch adds a pointer to the route corresponding to the specific
> flow over the SA of an xfrm_dst that's being used.
> 
> It also sets the next pointer of each xfrm_dst to the one above it.
> This allows to traverse the list upwards from the bottom.

Looking at this again, I noticed a problem:

> +		if (remote != fl_tunnel.fl4_dst) {
> +			fl_tunnel.fl4_src = local;
> +			fl_tunnel.fl4_dst = remote;
> +			err = xfrm_dst_lookup((struct xfrm_dst **)&rt,
> +					      &fl_tunnel, AF_INET);
> +			if (err)
> +				goto error;
> +		} else
> +			dst_hold(&rt->u.dst);
>  	}
> +
>  	dst_prev->child = &rt->u.dst;
> +	dst->path = &rt->u.dst;
> +
> +	*dst_p = dst;
> +	dst = dst_prev;
> +
> +	dst_prev = *dst_p;
>  	i = 0;
> -	for (dst_prev = dst; dst_prev != &rt->u.dst; dst_prev = dst_prev->child) {
> +	for (; dst_prev != &rt->u.dst; dst_prev = dst_prev->child) {

Since the tunnel dst is not necessarily the last in the bundle anymore,
we might miss to initialize some dsts, for example with ipcomp/tunnel +
esp/transport. If we have nested tunnels we'll fiddle with entries in
the routing cache.

Regards
Patrick