Herbert Xu wrote:

>I understand the inconsistency and agree that it should be fixed.
>However, I think the way you did it has created a new inconsistency.
>
>Tunnel mode SAs are not always used to carry subnets.  It can also
>be used for host-to-host configurations where the aim is to protect
>the IP header.  Therefore it would be inconsistent to look up a
>new route for host-to-host tunnel mode SAs.
>
>Perhaps we can simply expand the check to include local as well,
>i.e.,
>
>	if (local != fl->fl4_src || remote != fl->fl4_dst) {
>
>What do you think?
>  
>

I don't think this solves the inconsistency. By reuseing routes in tunnel
mode we allow routing by different criteria when the inner packet is headed
for the remote gateway. Your suggestion limits this a bit further, but we
can still have a situation where all packets going through a tunnel take
one path, except when the inner packet is heading for the remote gateway
itself. I think it is logically correct to reroute all packets in tunnel
mode, if we want to allow full policy routing for tunnel mode packets we
should hand tos and fwmark to xfrm_dst_lookup(). I don't think we should
do this currently because of a different problem. __xfrm4_find_bundle()
uses a different key than routing for looking up cached bundles. When the
original route was reused it is used even if fwmark/tos don't match. Fixing
this is easy, but it causes alot more cached bundles to exist. My last patch
limits this situation to transport mode because we always choose a new route
in tunnel mode based only on src/dst. Please see the attached patch for a
possible fix (ugly and compile tested only). If we want to do this for 
tunnel
mode we probably need a hash for the cached bundles first, which has its
own set of problems.

Regards
Patrick