[RFC PATCH 1/2] ipv6: select oif corresponding to source address

[RFC PATCH 1/2] ipv6: select oif corresponding to source address

[Emmanuel Thierry]

When selecting the next hop, prefer the interface to which the
source address is associated. This preference fixes problems for
IPv6 hosts in multi-interfaces setup.

In the case where a host has:
* multiple links, each providing internet connectivity
* both links advertising prefix and default route via Router
Advertisements

The current route selection process completly ignores whether or
not the next hop is consistent with the source address. This may
lead to packets being sent with a specific source address on the
wrong link, then dropped by a router enforcing reverse path
filtering.

This fix pre-selects the output interface corresponding to the
source address bound to the socket. The fl6->flowi6_oif
attribute gives to the route selection algorithm an hint about
what interface to choose in case of a tie between several routes
of equal preferences (equal netmask, equal metrics and equal
RFC 4191 preference values).

Signed-off-by: Emmanuel Thierry <emmanuel.thierry@telecom-bretagne.eu>
---
 net/ipv6/ip6_output.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 91fb4e8..1a05395 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -848,8 +848,14 @@ static int ip6_dst_lookup_tail(struct sock *sk,
 #endif
 	int err;
 
-	if (*dst == NULL)
+	if (*dst == NULL) {
+		struct inet6_ifaddr *ifp;
+		if (!ipv6_addr_any(&fl6->saddr)) {
+			ifp = ipv6_get_ifaddr(net, &fl6->saddr, NULL, 1);
+			fl6->flowi6_oif = ifp->idev->dev->ifindex;
+		}
 		*dst = ip6_route_output(net, sk, fl6);
+	}
 
 	if ((err = (*dst)->error))
 		goto out_err_release;
-- 
1.7.9.5

[RFC PATCH 2/2] ipv6: don't use routing cache for inexact matchings

[Emmanuel Thierry]

On route selection, make the cache to be used only when the flow
output interface corresponds to the next hop interface. This
fixes problems for IPv6 hosts in multi-interfaces setup.

When a packet is sent through an interface, the RTF_CACHE entry
is created based on a specific flow information (e.g. with
fl6->flowi6_oif set to a specific value). If another packet is
sent with a different flow information than the first one, the
RTF_CACHE entry is used regardless of flow information, even if
this information would make the route selection algorithm to
select a different route.

By ignoring RTF_CACHE entries for which rt6_check_dev() fails,
we ensure consistency of the route selection algorithm.

Signed-off-by: Emmanuel Thierry <emmanuel.thierry@telecom-bretagne.eu>
---
 net/ipv6/route.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index f54e3a1..65ce3d9 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -595,7 +595,8 @@ static int rt6_score_route(struct rt6_info *rt, int oif,
 	int m;
 
 	m = rt6_check_dev(rt, oif);
-	if (!m && (strict & RT6_LOOKUP_F_IFACE))
+	if (!m &&
+	    ((strict & RT6_LOOKUP_F_IFACE) || (rt->rt6i_flags & RTF_CACHE)))
 		return RT6_NUD_FAIL_HARD;
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	m |= IPV6_DECODE_PREF(IPV6_EXTRACT_PREF(rt->rt6i_flags)) << 2;
-- 
1.7.9.5

Re: [RFC PATCH 1/2] ipv6: select oif corresponding to source address

[Emmanuel Thierry]

Hello,

These two rfc patches fix problems stated in "ipv6: strange routing behaviors on a multi-interfaces setup" on 2013-09-25:

> 
> I'm working on multi-interfaces setups on IPv6. I found several disturbing route behaviors which sounds like bugs to me.
> 
> Both eth1 and eth2 interfaces receive RAs from distinct routers and autoconfigure:
> * their slaac address
> * their default route, both with the same priority
> Under these conditions, the following happen depending on the expiration time of each route.
> 
> 
> 1/ A ping with a specified source address and interface may go through the wrong interface.
> 
> # ping6  -c 1 -I "<slaac_eth1>%eth1" <dest>
> … uses the right interface (eth1) with the right source address (<slaac_eth1>).
> 
> # ping6  -c 1 -I "<slaac_eth2>%eth2" <dest>
> … uses the wrong interface (eth1) with the right source address (<slaac_eth2>).
> 
> The ping6 utility performed a bind() on <slaac_ethx> with scope id set to <ifindex_ethx>.
> If we flush the routing cache between each ping, the routing is done as expected.
> 
> As i could observe in the kernel. When ip6_pol_route() is called, oif is equal to <ifindex_eth2> but the flag RT6_LOOKUP_F_IFACE is not set. This makes routes through other interfaces to still be valid.
> Shouldn't we set the RT6_LOOKUP_F_IFACE flag when a scope id is specified ?
> 
> 
> 2/ A ping from a specified source address may go through the wrong interface.
> 
> # ping6  -I "<slaac_eth2>" <dest>
> … may use eth1 with <slaac_eth2>.
> 
> The ping6 utility performed a bind() on <slaac_eth2>
> This is a derivative from the first one, with the significative difference that it also happens if the routing cache is empty. The most recent default route is chosen regardless of the source address.
> 
> Shouldn't we look in a first try for routes on the device corresponding to the source address, and in a second try for others ?
> 
> 
> 3/ A ping from a specified interface may go through the wrong interface with the wrong source address.
> 
> # ping6  -I "eth2" <dest>
> … may use eth1 with <slaac_eth1>.
> 
> The ping6 performs a setsockopt(IPV6_PKTINFO) with <ifindex_ethx>, then a connect() to the destination. In this case, source address selection is concerned, but also routing since source address selection depends on routing.
> 
> 
> I experienced these problems on a 3.11.1 kernel but they look to be quite recurrent in the past versions as well. 
> 

Best regards
Emmanuel Thierry

Re: [RFC PATCH 1/2] ipv6: select oif corresponding to source address

[Hannes Frederic Sowa]

On Fri, Nov 01, 2013 at 06:17:36PM +0100, Emmanuel Thierry wrote:
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -848,8 +848,14 @@ static int ip6_dst_lookup_tail(struct sock *sk,
>  #endif
>  	int err;
>  
> -	if (*dst == NULL)
> +	if (*dst == NULL) {
> +		struct inet6_ifaddr *ifp;
> +		if (!ipv6_addr_any(&fl6->saddr)) {
> +			ifp = ipv6_get_ifaddr(net, &fl6->saddr, NULL, 1);
> +			fl6->flowi6_oif = ifp->idev->dev->ifindex;

Cool that you are working on this! :)

Small feedback that might help while you play with this:

You have to be careful here, ipv6_get_ifaddr increments the reference
count of the ifp, so you have to drop it again: in6_ifa_put(ifp);

For such short periods holding a reference does not always make sense. You
could check ipv6_get_lladdr/__ipv6_get_lladdr how you could do this
without touching the reference counters if you put this section in
a rcu_read_lock. This especially makes sense if this is a often used
code path.

> +		}
>  		*dst = ip6_route_output(net, sk, fl6);
> +	}
>  
>  	if ((err = (*dst)->error))
>  		goto out_err_release;

I'll have to think about this carefully.

Thanks,

  Hannes

Re: [RFC PATCH 1/2] ipv6: select oif corresponding to source address

[Sergei Shtylyov]

Hello.

On 11/01/2013 08:17 PM, Emmanuel Thierry wrote:

> When selecting the next hop, prefer the interface to which the
> source address is associated. This preference fixes problems for
> IPv6 hosts in multi-interfaces setup.

> In the case where a host has:
> * multiple links, each providing internet connectivity
> * both links advertising prefix and default route via Router
> Advertisements

> The current route selection process completly ignores whether or
> not the next hop is consistent with the source address. This may
> lead to packets being sent with a specific source address on the
> wrong link, then dropped by a router enforcing reverse path
> filtering.

> This fix pre-selects the output interface corresponding to the
> source address bound to the socket. The fl6->flowi6_oif
> attribute gives to the route selection algorithm an hint about
> what interface to choose in case of a tie between several routes
> of equal preferences (equal netmask, equal metrics and equal
> RFC 4191 preference values).

> Signed-off-by: Emmanuel Thierry <emmanuel.thierry@telecom-bretagne.eu>
> ---
>   net/ipv6/ip6_output.c |    8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)

> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 91fb4e8..1a05395 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -848,8 +848,14 @@ static int ip6_dst_lookup_tail(struct sock *sk,
>   #endif
>   	int err;
>
> -	if (*dst == NULL)
> +	if (*dst == NULL) {
> +		struct inet6_ifaddr *ifp;

    Empty line needed between declaration and other code.

> +		if (!ipv6_addr_any(&fl6->saddr)) {
> +			ifp = ipv6_get_ifaddr(net, &fl6->saddr, NULL, 1);
> +			fl6->flowi6_oif = ifp->idev->dev->ifindex;
> +		}
>   		*dst = ip6_route_output(net, sk, fl6);
> +	}
>
>   	if ((err = (*dst)->error))
>   		goto out_err_release;

WBR, Sergei