[PATCH 7/13] [RFC] [IPV6] Move source address selection into route lookup.

[PATCH 7/13] [RFC] [IPV6] Move source address selection into route lookup.

[Ville Nuorvala]

This patch moves the normal source address selection from
ip6_dst_lookup() into ip6_pol_route_output(), but shouldn't
change the routing or source address selection behavior in
any way.

Signed-off-by: Ville Nuorvala <vnuorval@tcs.hut.fi>
---
 net/ipv6/ip6_output.c |    6 ------
 net/ipv6/route.c      |   37 ++++++++++++++++++++++---------------
 2 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 6671691..0019007 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -855,12 +855,6 @@ static int ip6_dst_lookup_tail(struct so
 	if ((err = (*dst)->error))
 		goto out_err_release;

-	if (ipv6_addr_any(&fl->fl6_src)) {
-		err = ipv6_get_saddr(*dst, &fl->fl6_dst, &fl->fl6_src);
-		if (err)
-			goto out_err_release;
-	}
-
 	return 0;

 out_err_release:
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index aa96be8..b7b8148 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -536,7 +536,7 @@ struct rt6_info *rt6_lookup(struct in6_a
 	int flags = strict ? RT6_LOOKUP_F_IFACE : 0;

 	if (saddr) {
-		memcpy(&fl.fl6_src, saddr, sizeof(*saddr));
+		ipv6_addr_copy(&fl.fl6_src, saddr);
 		flags |= RT6_LOOKUP_F_HAS_SADDR;
 	}

@@ -629,13 +629,11 @@ static struct rt6_info *ip6_pol_route_in
 {
 	struct fib6_node *fn;
 	struct rt6_info *rt, *nrt;
-	int strict = 0;
+	int strict = flags & RT6_LOOKUP_F_IFACE;
 	int attempts = 3;
 	int err;
 	int reachable = RT6_LOOKUP_F_REACHABLE;

-	strict |= flags & RT6_LOOKUP_F_IFACE;
-
 relookup:
 	read_lock_bh(&table->tb6_lock);

@@ -726,22 +724,22 @@ static struct rt6_info *ip6_pol_route_ou
 {
 	struct fib6_node *fn;
 	struct rt6_info *rt, *nrt;
-	int strict = 0;
-	int attempts = 3;
-	int err;
+	int has_saddr = flags & RT6_LOOKUP_F_HAS_SADDR;
+	int strict = flags & RT6_LOOKUP_F_IFACE;
 	int reachable = RT6_LOOKUP_F_REACHABLE;
+	int attempts = 3;
+	struct in6_addr saddr;

-	strict |= flags & RT6_LOOKUP_F_IFACE;
-
+	ipv6_addr_copy(&saddr, &fl->fl6_src);
 relookup:
 	read_lock_bh(&table->tb6_lock);

 restart_2:
-	fn = fib6_lookup(&table->tb6_root, &fl->fl6_dst, &fl->fl6_src);
+	fn = fib6_lookup(&table->tb6_root, &fl->fl6_dst, &saddr);

 restart:
 	rt = rt6_select(&fn->leaf, fl->oif, strict | reachable);
-	BACKTRACK(&fl->fl6_src);
+	BACKTRACK(&saddr);
 	if (rt == &ip6_null_entry ||
 	    rt->rt6i_flags & RTF_CACHE)
 		goto out;
@@ -749,6 +747,13 @@ restart:
 	dst_hold(&rt->u.dst);
 	read_unlock_bh(&table->tb6_lock);

+	if (!has_saddr) {
+		/* policy rule doesn't restrict source address */
+		if (ipv6_get_saddr(&rt->u.dst, &fl->fl6_dst, &saddr))
+			goto no_saddr;
+		has_saddr = RT6_LOOKUP_F_HAS_SADDR;
+		ipv6_addr_copy(&fl->fl6_src, &saddr);
+	}
 	if (!rt->rt6i_nexthop && !(rt->rt6i_flags & RTF_NONEXTHOP))
 		nrt = rt6_alloc_cow(rt, &fl->fl6_dst, &fl->fl6_src);
 	else {
@@ -764,8 +769,7 @@ #endif

 	dst_hold(&rt->u.dst);
 	if (nrt) {
-		err = ip6_ins_rt(nrt);
-		if (!err)
+		if (!ip6_ins_rt(nrt))
 			goto out2;
 	}

@@ -778,7 +782,6 @@ #endif
 	 */
 	dst_release(&rt->u.dst);
 	goto relookup;
-
 out:
 	if (reachable) {
 		reachable = 0;
@@ -790,6 +793,10 @@ out2:
 	rt->u.dst.lastuse = jiffies;
 	rt->u.dst.__use++;
 	return rt;
+no_saddr:
+	rt = &ip6_null_entry;
+	dst_hold(&rt->u.dst);
+	goto out2;
 }

 struct dst_entry * ip6_route_output(struct sock *sk, struct flowi *fl)
@@ -2044,7 +2051,7 @@ #endif
 		NLA_PUT_U32(skb, RTA_IIF, iif);
 	else if (dst) {
 		struct in6_addr saddr_buf;
-		if (ipv6_get_saddr(&rt->u.dst, dst, &saddr_buf) == 0)
+		if (!ipv6_get_saddr(&rt->u.dst, dst, &saddr_buf))
 			NLA_PUT(skb, RTA_PREFSRC, 16, &saddr_buf);
 	}

-- 
1.4.2.3

Re: [PATCH 7/13] [RFC] [IPV6] Move source address selection into route lookup.

[David Miller]

From: Ville Nuorvala <vnuorval@tcs.hut.fi>
Date: Tue, 17 Oct 2006 03:13:17 +0300

> This patch moves the normal source address selection from
> ip6_dst_lookup() into ip6_pol_route_output(), but shouldn't
> change the routing or source address selection behavior in
> any way.
> 
> Signed-off-by: Ville Nuorvala <vnuorval@tcs.hut.fi>

Although this conversion is very clean and the next patch
is very logic, I'm going to hold on all patches from 7 onward
so there is some time for some discussion of the RFC'ness
of them :-)

Thanks.

Re: [PATCH 7/13] [RFC] [IPV6] Move source address selection into route lookup.

[Jean-Mickael Guerin]

David Miller wrote:
> From: Ville Nuorvala <vnuorval@tcs.hut.fi>
> Date: Tue, 17 Oct 2006 03:13:17 +0300
>
>> This patch moves the normal source address selection from
>> ip6_dst_lookup() into ip6_pol_route_output(), but shouldn't
>> change the routing or source address selection behavior in
>> any way.
>>
>> Signed-off-by: Ville Nuorvala <vnuorval@tcs.hut.fi>
>
> Although this conversion is very clean and the next patch
> is very logic, I'm going to hold on all patches from 7 onward
> so there is some time for some discussion of the RFC'ness
> of them :-)
>
With regard to RFC'ness, passing self tests TAHI "Ready Logo 2" with 
SUBTREE
turned on could be a valuable feedback.

Jean-Mickael

Re: [PATCH 7/13] [RFC] [IPV6] Move source address selection into route lookup.

[YOSHIFUJI Hideaki / 吉藤英明]

In article <45348DD4.4000600@6wind.com> (at Tue, 17 Oct 2006 10:01:24 +0200), Jean-Mickael Guerin <jean-mickael.guerin@6wind.com> says:

> > Although this conversion is very clean and the next patch
> > is very logic, I'm going to hold on all patches from 7 onward
> > so there is some time for some discussion of the RFC'ness
> > of them :-)
> >
> With regard to RFC'ness, passing self tests TAHI "Ready Logo 2" with 
> SUBTREE
> turned on could be a valuable feedback.

Have you done it?

We've been trying to pass that, but we failed to compile;
we probably used wrong tree...

--yoshfuji

Re: [PATCH 7/13] [RFC] [IPV6] Move source address selection into route lookup.

[Mitsuru Chinen]

YOSHIFUJI Hideaki / 吉藤英明 wrote:
> In article <45348DD4.4000600@6wind.com> (at Tue, 17 Oct 2006 10:01:24 +0200), Jean-Mickael Guerin <jean-mickael.guerin@6wind.com> says:
> 
>>> Although this conversion is very clean and the next patch
>>> is very logic, I'm going to hold on all patches from 7 onward
>>> so there is some time for some discussion of the RFC'ness
>>> of them :-)
>>>
>> With regard to RFC'ness, passing self tests TAHI "Ready Logo 2" with 
>> SUBTREE
>> turned on could be a valuable feedback.
> 
> Have you done it?
> 
> We've been trying to pass that, but we failed to compile;
> we probably used wrong tree...

Excuse me. I overlooked an error message from patch command.
When I appiled the patch manually, we could compile them.

We will double-check these patches with TAHI, either.
And then, we will report the results.

Best Regards,
-- 
Mitsuru Chinen <mitch@jp.ibm.com>

Re: (usagi-core 31424) Re: [PATCH 7/13] [RFC] [IPV6] Move source address selection into route lookup.

[Ville Nuorvala]

On 10/19/06 05:27, Mitsuru Chinen wrote:

Hello Mitsuru-san,

> At the first Echo Request, its source address is a global address.
> 
> | # ping6 -n -c 1 -i 1 -s 1452 -p 00 -w 2 -I eth1 FF1E: :1:2
> | PATTERN: 0x00
> | PING FF1E::1:2(ff1e::1:2) from 3ffe:501:ffff:100:2d0:b7ff:fe9a:455b eth1: 1452 data bytes
> |
> | --- FF1E::1:2 ping statistics ---
> | 3 packets transmitted, 0 received, 100% loss, time 1999ms
> 
> However, after receiving a Too Big message, the source address
> of Echo Request is changed to :: at the failure case.

thank you for the thorough test report! I'll try to track down the reason.

Regards,
Ville

Re: (usagi-core 31424) Re: [PATCH 7/13] [RFC] [IPV6] Move source address selection into route lookup.

[Ville Nuorvala]

Mitsuru-san,

could you apply patch #12 and rerun the test? I think this particular
problem is caused by the routing cache entry not having a valid source
address in the first place. As ip6_dst_lookup() doesn't do the source
address lookup anymore, it is critical that we only store cache entries
with complete address data for both destination and source.

These two patches apparently shouldn't have been split in the first
place. Sorry!

Regards,
Ville

Re: (usagi-core 31424) Re: [PATCH 7/13] [RFC] [IPV6] Move source address selection into route lookup.

[Mitsuru Chinen]

Hello Ville,

Ville Nuorvala wrote:
> Mitsuru-san,
> 
> could you apply patch #12 and rerun the test? I think this particular
> problem is caused by the routing cache entry not having a valid source
> address in the first place. As ip6_dst_lookup() doesn't do the source
> address lookup anymore, it is critical that we only store cache entries
> with complete address data for both destination and source.
> 
> These two patches apparently shouldn't have been split in the first
> place. Sorry!

I'm afraid patch #12 doesn't work for this issue.

Although I applied patch #12, I still got fails.
The test log is the same as the last one.
When I applied all patches, I got fails, either.

Best Regards,
-- 
Mitsuru Chinen <mitch@jp.ibm.com>

Re: (usagi-core 31424) Re: [PATCH 7/13] [RFC] [IPV6] Move source address selection into route lookup.

[Ville Nuorvala]

On 10/19/06 05:27, Mitsuru Chinen wrote:

Hello again Mitsuru-san,

I haven't yet had time to follow up on this issue and I would in fact
need some help to proceed.

> http://testlab.linux-ipv6.org/tahi-autorun.2/net-2.6_20061018/

The host testlab.linux-ipv6.org doesn't seem to be visible to the
outside world so could you post the results somewhere where I could take
a closer look at the results?

Could you actually also send me the configuration files for the TAHI
test, so I could try to set up an identical test environment?

Thanks,
Ville

Re: (usagi-core 31424) Re: [PATCH 7/13] [RFC] [IPV6] Move source address selection into route lookup.

[YOSHIFUJI Hideaki / 吉藤英明]

In article <4549E8B3.4070601@tcs.hut.fi> (at Thu, 02 Nov 2006 14:46:43 +0200), Ville Nuorvala <vnuorval@tcs.hut.fi> says:

> > http://testlab.linux-ipv6.org/tahi-autorun.2/net-2.6_20061018/
> 
> The host testlab.linux-ipv6.org doesn't seem to be visible to the
> outside world so could you post the results somewhere where I could take
> a closer look at the results?

It is visible world-wide, assuming you have IPv6 connection.

> Could you actually also send me the configuration files for the TAHI
> test, so I could try to set up an identical test environment?

Nothing special.

--yoshfuji

Re: !! SPAM Suspect : SPAM-URL-DBL !! Re: (usagi-core 31424) Re: [PATCH 7/13] [RFC] [IPV6] Move source address selection into route lookup.

[Jean-Mickael Guerin]

>> The host testlab.linux-ipv6.org doesn't seem to be visible to the
>> outside world so could you post the results somewhere where I could take
>> a closer look at the results?
>
> It is visible world-wide, assuming you have IPv6 connection.
>
With IPv4-only connection, one can try to append ".ipv4.sixxs.org":

http://testlab.linux-ipv6.org.ipv4.sixxs.org/tahi-autorun.2/net-2.6_20061018/

Jean-Mickael

Re: (usagi-core 31424) Re: [PATCH 7/13] [RFC] [IPV6] Move source address selection into route lookup.

[Jean-Mickael Guerin]

[ reposted, with better subject ]
>>> http://testlab.linux-ipv6.org/tahi-autorun.2/net-2.6_20061018/
>> The host testlab.linux-ipv6.org doesn't seem to be visible to the
>> outside world so could you post the results somewhere where I could take
>> a closer look at the results?
>
> It is visible world-wide, assuming you have IPv6 connection.
With IPv4-only connection, one can try to append ".ipv4.sixxs.org":

http://testlab.linux-ipv6.org.ipv4.sixxs.org/tahi-autorun.2/net-2.6_20061018/ 


Jean-Mickael