[PATCH] ipv4/route.c: respect prefsrc for local routes

Subsystems: networking [general], networking [ipv4/ipv6], the rest

STALE5771d

9 messages, 6 authors, 2011-01-04 · open the first message on its own page

[PATCH] ipv4/route.c: respect prefsrc for local routes

From: Joel Sing <hidden>
Date: 2011-01-04 06:24:48 

The preferred source address is currently ignored for local routes,
which results in all local connections having a src address that is the
same as the local dst address. Fix this by respecting the preferred source
address when it is provided for local routes.

This bug can be demonstrated as follows:

 # ifconfig dummy0 192.168.0.1
 # ip route show table local | grep local.*dummy0
 local 192.168.0.1 dev dummy0  proto kernel  scope host  src 192.168.0.1
 # ip route change table local local 192.168.0.1 dev dummy0 \
     proto kernel scope host src 127.0.0.1
 # ip route show table local | grep local.*dummy0
 local 192.168.0.1 dev dummy0  proto kernel  scope host  src 127.0.0.1

We now establish a local connection and verify the source IP
address selection:

 # nc -l 192.168.0.1 3128 &
 # nc 192.168.0.1 3128 &
 # netstat -ant | grep 192.168.0.1:3128.*EST
 tcp        0      0 192.168.0.1:3128        192.168.0.1:33228 ESTABLISHED
 tcp        0      0 192.168.0.1:33228       192.168.0.1:3128  ESTABLISHED

Signed-off-by: Joel Sing <redacted>
---
 net/ipv4/route.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index df948b0..93bfd95 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2649,8 +2649,12 @@ static int ip_route_output_slow(struct net *net, struct rtable **rp,
 	}
 
 	if (res.type == RTN_LOCAL) {
-		if (!fl.fl4_src)
-			fl.fl4_src = fl.fl4_dst;
+		if (!fl.fl4_src) {
+			if (res.fi->fib_prefsrc)
+				fl.fl4_src = res.fi->fib_prefsrc;
+			else
+				fl.fl4_src = fl.fl4_dst;
+		}
 		dev_out = net->loopback_dev;
 		fl.oif = dev_out->ifindex;
 		res.fi = NULL;
-- 
1.7.3.1

Re: [PATCH] ipv4/route.c: respect prefsrc for local routes

From: Eric Dumazet <hidden>
Date: 2011-01-04 07:24:45 

Le mardi 04 janvier 2011 à 17:24 +1100, Joel Sing a écrit :
quoted hunk
The preferred source address is currently ignored for local routes,
which results in all local connections having a src address that is the
same as the local dst address. Fix this by respecting the preferred source
address when it is provided for local routes.

This bug can be demonstrated as follows:

 # ifconfig dummy0 192.168.0.1
 # ip route show table local | grep local.*dummy0
 local 192.168.0.1 dev dummy0  proto kernel  scope host  src 192.168.0.1
 # ip route change table local local 192.168.0.1 dev dummy0 \
     proto kernel scope host src 127.0.0.1
 # ip route show table local | grep local.*dummy0
 local 192.168.0.1 dev dummy0  proto kernel  scope host  src 127.0.0.1

We now establish a local connection and verify the source IP
address selection:

 # nc -l 192.168.0.1 3128 &
 # nc 192.168.0.1 3128 &
 # netstat -ant | grep 192.168.0.1:3128.*EST
 tcp        0      0 192.168.0.1:3128        192.168.0.1:33228 ESTABLISHED
 tcp        0      0 192.168.0.1:33228       192.168.0.1:3128  ESTABLISHED

Signed-off-by: Joel Sing <redacted>
---
 net/ipv4/route.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index df948b0..93bfd95 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2649,8 +2649,12 @@ static int ip_route_output_slow(struct net *net, struct rtable **rp,
 	}
 
 	if (res.type == RTN_LOCAL) {
-		if (!fl.fl4_src)
-			fl.fl4_src = fl.fl4_dst;
+		if (!fl.fl4_src) {
+			if (res.fi->fib_prefsrc)
+				fl.fl4_src = res.fi->fib_prefsrc;
+			else
+				fl.fl4_src = fl.fl4_dst;
+		}
 		dev_out = net->loopback_dev;
 		fl.oif = dev_out->ifindex;
 		res.fi = NULL;
Please use FIB_RES_PREFSRC(res)

as we do a few lines after ;)

Thanks

Re: [PATCH] ipv4/route.c: respect prefsrc for local routes

From: Eric Dumazet <hidden>
Date: 2011-01-04 07:33:32 

Le mardi 04 janvier 2011 à 08:24 +0100, Eric Dumazet a écrit :

Please use FIB_RES_PREFSRC(res)
Hmm no, this is not applicable, but this could be shorter :

fl.fl4_src = res.fi->fib_prefsrc ? : fl.fl4_dst;

Re: [PATCH] ipv4/route.c: respect prefsrc for local routes

From: Changli Gao <hidden>
Date: 2011-01-04 08:07:29 

On Tue, Jan 4, 2011 at 3:33 PM, Eric Dumazet [off-list ref] wrote:
Le mardi 04 janvier 2011 à 08:24 +0100, Eric Dumazet a écrit :
quoted
Please use FIB_RES_PREFSRC(res)
Hmm no, this is not applicable, but this could be shorter :

fl.fl4_src = res.fi->fib_prefsrc ? : fl.fl4_dst;

I think Joe may object the use of "? :"

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

Re: [PATCH] ipv4/route.c: respect prefsrc for local routes

From: Eric Dumazet <hidden>
Date: 2011-01-04 08:38:40 

Le mardi 04 janvier 2011 à 16:07 +0800, Changli Gao a écrit :
On Tue, Jan 4, 2011 at 3:33 PM, Eric Dumazet [off-list ref] wrote:
quoted
Le mardi 04 janvier 2011 à 08:24 +0100, Eric Dumazet a écrit :
quoted
Please use FIB_RES_PREFSRC(res)
Hmm no, this is not applicable, but this could be shorter :

fl.fl4_src = res.fi->fib_prefsrc ? : fl.fl4_dst;

I think Joe may object the use of "? :"
Ternary operator is standard C idiom, used in networking stuff, for
example in FIB_RES_PREFSRC() ;)

This could be properly done using another macro in include/net/ip_fib.h
to centralize this ternary op in one point :

#define __FIB_RES_PREFSRC(res, default) ((res).fi->fib_prefsrc ? : default)
#define FIB_RES_PREFSRC(res) __FIB_RES_PREFSRC(res, default, __fib_res_prefsrc(&res))

Re: [PATCH] ipv4/route.c: respect prefsrc for local routes

From: Eric Dumazet <hidden>
Date: 2011-01-04 08:40:24 

Le mardi 04 janvier 2011 à 09:38 +0100, Eric Dumazet a écrit :

This could be properly done using another macro in include/net/ip_fib.h
to centralize this ternary op in one point :

#define __FIB_RES_PREFSRC(res, default) ((res).fi->fib_prefsrc ? : default)
#define FIB_RES_PREFSRC(res) __FIB_RES_PREFSRC(res, default, __fib_res_prefsrc(&res)
I meant

#define FIB_RES_PREFSRC(res) __FIB_RES_PREFSRC(res, __fib_res_prefsrc(&res))

Re: [PATCH] ipv4/route.c: respect prefsrc for local routes

From: Ben Hutchings <hidden>
Date: 2011-01-04 14:16:34 

On Tue, 2011-01-04 at 09:38 +0100, Eric Dumazet wrote:
Le mardi 04 janvier 2011 à 16:07 +0800, Changli Gao a écrit :
quoted
On Tue, Jan 4, 2011 at 3:33 PM, Eric Dumazet [off-list ref] wrote:
quoted
Le mardi 04 janvier 2011 à 08:24 +0100, Eric Dumazet a écrit :
quoted
Please use FIB_RES_PREFSRC(res)
Hmm no, this is not applicable, but this could be shorter :

fl.fl4_src = res.fi->fib_prefsrc ? : fl.fl4_dst;

I think Joe may object the use of "? :"
Ternary operator is standard C idiom, used in networking stuff, for
example in FIB_RES_PREFSRC() ;)
[...]

However, the option to omit the second operand is a GNU extension.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH] ipv4/route.c: respect prefsrc for local routes

From: Joe Perches <joe@perches.com>
Date: 2011-01-04 15:32:11 

On Tue, 2011-01-04 at 14:16 +0000, Ben Hutchings wrote:
On Tue, 2011-01-04 at 09:38 +0100, Eric Dumazet wrote:
quoted
Le mardi 04 janvier 2011 à 16:07 +0800, Changli Gao a écrit :
quoted
On Tue, Jan 4, 2011 at 3:33 PM, Eric Dumazet [off-list ref] wrote:
quoted
Le mardi 04 janvier 2011 à 08:24 +0100, Eric Dumazet a écrit :
quoted
Please use FIB_RES_PREFSRC(res)
Hmm no, this is not applicable, but this could be shorter :
fl.fl4_src = res.fi->fib_prefsrc ? : fl.fl4_dst;
I think Joe may object the use of "? :"
Ternary operator is standard C idiom, used in networking stuff, for
example in FIB_RES_PREFSRC() ;)
However, the option to omit the second operand is a GNU extension.
I don't object to using GNU extensions.

The ?: extension is used in most every major subsystem.

$ grep -rP --include=*.[ch] "\?\s*:" * | wc -l
515

(with some false positives)

Re: [PATCH] ipv4/route.c: respect prefsrc for local routes

From: David Miller <davem@davemloft.net>
Date: 2011-01-04 19:35:19 

From: Joel Sing <redacted>
Date: Tue,  4 Jan 2011 17:24:20 +1100

The preferred source address is currently ignored for local routes,
which results in all local connections having a src address that is the
same as the local dst address. Fix this by respecting the preferred source
address when it is provided for local routes.

This bug can be demonstrated as follows:

 # ifconfig dummy0 192.168.0.1
 # ip route show table local | grep local.*dummy0
 local 192.168.0.1 dev dummy0  proto kernel  scope host  src 192.168.0.1
 # ip route change table local local 192.168.0.1 dev dummy0 \
     proto kernel scope host src 127.0.0.1
 # ip route show table local | grep local.*dummy0
 local 192.168.0.1 dev dummy0  proto kernel  scope host  src 127.0.0.1

We now establish a local connection and verify the source IP
address selection:

 # nc -l 192.168.0.1 3128 &
 # nc 192.168.0.1 3128 &
 # netstat -ant | grep 192.168.0.1:3128.*EST
 tcp        0      0 192.168.0.1:3128        192.168.0.1:33228 ESTABLISHED
 tcp        0      0 192.168.0.1:33228       192.168.0.1:3128  ESTABLISHED

Signed-off-by: Joel Sing <redacted>
Applied to net-2.6, thanks Joel.

If you guys want to mess with ternary operators and new macros,
please do that in net-next-2.6 the next time I merge or similar.

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help