Re: [PATCH] ipv4: Cache source address in nexthop entries.

 	Hello,

On Tue, 8 Mar 2011, David Miller wrote:

	change_nexthops(fi) {
+		nexthop_nh->nh_cfg_scope = cfg->fc_scope;
		nexthop_nh->nh_saddr = inet_select_addr(nexthop_nh->nh_dev,
							nexthop_nh->nh_gw,
-							nexthop_nh->nh_scope);
+							nexthop_nh->nh_cfg_scope);
	} endfor_nexthops(fi)

 	I see another problem with nh_saddr. fib_info
is reused when NHs and some parameters are equal but the
scope is not part of the fib_info and it is wrong to cache
route scope in NH or even in fib_info. The problem is
that we can expose in nh_saddr addresses with wrong
scope for the route. It happens when two routes with
different route scope reuse same fib_info.

 	First thought is that the field fa_scope can be
moved into fib_scope in struct fib_info, not to use
nh_cfg_scope. Then fib_find_info can differentiate the
entries by fib_scope. It means, fib_info will not be reused
if fib_scope is different. For FIB_RES_PREFSRC I assume we
can access fib_info from nh->nh_parent safely when
refreshing nh_saddr.

 	Here is a script that demonstrates the wrong
exposing:

#! /bin/sh -x

dev=eth1

ip link set $dev up
ip addr flush $dev
ip route flush dev $dev

# address that we want to show only to site
ip addr add 1.2.3.4/24 brd + dev $dev scope site

# first route learns 1.2.3.4 as nh_saddr, it is suitable for site
ip route add 5.6.7.0/24 via 1.2.3.6 dev $dev scope site

# our global primary address for universe, bigger subnet
ip addr add 1.2.3.5/23 brd + dev $dev scope global

# second route reuses same fib_info, exposes site address
# 1.2.3.4 to the 4.3.2.0 world
ip route add 4.3.2.0/24 via 1.2.3.6 dev $dev scope global

# show results:

# we must see src 1.2.3.4:
ip route get 5.6.7.8

# we must see src 1.2.3.5. Old kernels show 1.2.3.5, but we
# show 1.2.3.4:
ip route get 4.3.2.1

Regards

--
Julian Anastasov [off-list ref]