Date:	Thu, 10 Mar 2011 01:32:18 +0200 (EET)
From:	Julian Anastasov <ja@....bg>
To:	David Miller <davem@...emloft.net>
cc:	netdev@...r.kernel.org
Subject: Re: [PATCH] ipv4: Cache source address in nexthop entries.


 	Hello,

On Wed, 9 Mar 2011, David Miller wrote:

> From: Julian Anastasov <ja@....bg>
> Date: Wed, 9 Mar 2011 02:49:49 +0200 (EET)
>
>> 	But we still need to propagate the address events
>> to nexthops on all devices. Even if it is slow, I see it in
>> this way (not tested):
>
> Ok, I think it is even more involved than what you propose.
>
> We have to potentially re-run inet_select_addr() on all nexthops, even
> those pointing to devices other than the one being modified, because
> when an interface loses it's last IP address we have to look for a
> source address to use on other active interfaces.

 	Yes. The common case is addresses to be scope
global. That means they can influence any nexthop on
any device. Only when address has scope >= LINK we can
use version that walks single table row for the same device.
In the default case we need to walk all table rows.

 	Additionally, it is possible same address to exist
on different devices, possibly with different scope. That is
why reacting only on "last_ip" indication when deleting IP
is not enough. May be while we delete scope-link address on the
concerned device, there can be same address as scope-link
on another device but its scope is not enough to keep
it as nh_saddr, so the right thing is to get different
IP as saddr. That is why in my version of fib_update_nh_saddrs
I don't care about "last_ip" indication. I think, my
version of fib_update_nh_saddrs is ok for IP add/del but
as I said, device UP events should recalc nh_saddr too.
In my version the logic is:

- add IP:
 	- check all NHs on all devices
 	- goal: if the added IP-scope is suitable for NH
 	it can replace other less-priority IPs, where the
 	priority depends on factors such as device, placement in
 	device list and scope
 	- if NH has same nh_saddr then nothing to do
 	- IP scope and DEV should be allowed for the nh_cfg_scope
 	to change the nh_saddr

- del IP:
 	- check all NHs on all devices
 	- goal: if the removed IP-scope was nh_saddr somewhere
 	then we should recalc NH, even when IP is not the
 	last address with same value
 	- if NH has different nh_saddr then nothing to do
 	- IP scope and DEV should be allowed for the nh_cfg_scope
 	to change the nh_saddr because may be this IP-scope-DEV
 	was used as nh_saddr

> Probably it makes sense to recompute this at use time instead of
> walking all of this stuff at every event.

 	Yes, it looks less dangerous.

> So device address operations increment a generation ID and
> FIB_RES_PREFSRC() checks that ID against one stored in the nexthop.

 	Yes, may be it is better this generation ID to be global
value for netns, because the global addresses should cause
refresh for all nexthops on all devices. The ID should be
matched with corresponding value in NH. Another option is
to use per-device gen_id instead of netns gen_id but in this
case scope-global address event should increment gen_id for all
devices while the device_UP event will increment only
gen_id for the concerned DEV.

 	I.e. there are 2 variants:

1. global netns gen_id:
 	- add/del IP, device UP => netns->gen_id ++

2. dev->gen_id:
 	- add/del IP with scope < LINK => increment
 	dev->gen_id for all devices
 	- add/del IP with scope >= LINK => increment
 	dev->gen_id only for concerned device
 	- device UP: increment dev->gen_id only for
 	concerned device

Regards

--
Julian Anastasov <ja@....bg>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists