[4/4,v3] ipv4: mark nexthop as dead when it's subnet becomes unreachable

Removing ip address and it's subnet route using fib_del_ifaddr() does
not purge routes with nexthop in such subnet.

This could be easily reproduced with the following config:

  ip link add dev dummy1 type dummy
  ip link set up dev dummy1
  ip -4 addr add 10.0.10.1/24 dev dummy1
  ip -4 addr add 10.0.20.1/24 dev dummy1

  ip -4 route add 172.16.0.0/12 proto static via 10.0.10.5
  ip -4 route show exact 172.16.0.0/12
  172.16.0.0/12 via 10.0.10.5 dev dummy1  proto static

  ip -4 addr del 10.0.10.1/24 dev dummy1

  ip -4 route show exact 172.16.0.0/12
  172.16.0.0/12 via 10.0.10.5 dev dummy1  proto static

Add interface address (ifa) parameter to fib_sync_down_dev()
and use it to match nexthop against it's subnet.

Use fib_sync_down_dev() in fib_del_ifaddr() among with fib_sync_down_addr()
to mark as dead routes with nexthop in ifa.

  v3. Fix NH marking as dead when NH gateway subnet is still on
      interface (e.g. 10.0.10.1/24 and 10.0.30.1/16 and NH is 10.0.10.5).
      Thanks to Julian Anastasov.

  v2. Fix NH marking as dead when NH created with onlink option.

Signed-off-by: Sergey Popovich <popovich_sergei@mail.ru>
---
 include/net/ip_fib.h     |  3 ++-
 net/ipv4/fib_frontend.c  |  5 +++--
 net/ipv4/fib_semantics.c | 29 +++++++++++++++++++++++++++--
 3 files changed, 32 insertions(+), 5 deletions(-)

Hello,

On Fri, 24 Jan 2014, Sergey Popovich wrote:

> -int fib_sync_down_dev(struct net_device *dev, int force)
> +static inline bool fib_sync_down_gw(struct fib_nh *nh,
> +				    struct in_ifaddr *ifr)
> +{
> +	if (!ifr)
> +		return true;
> +
> +	if (nh->nh_flags & RTNH_F_ONLINK)
> +		return false;
> +
> +	if (!inet_ifa_match(nh->nh_gw, ifr))
> +		return false;
> +

	You need to walk subnets here, not IPs, so
for_primary_ifa() instead of for_ifa() will save some
cycles. But for me such change still looks expensive
and does not fix the root of the problem:

- You fix the problem from IP address point of view.
The actual problem is that subnet is removed, i.e.
it is the route removal that is making GWs unreachable.
I can ip route delete some link route and cause GWs
to become unreachable.

- not sure that walking the ifa_list is a fast operation

- sadly, the NHs can not survive the secondary address
promotion as done in __inet_del_ifa().

	You can have additional optimization in
fib_del_ifaddr() while calling fib_sync_down_dev():
do nothing if secondary address is deleted because its
subnet (primary address) should be present. For example:

	if (fib_sync_down_addr(dev_net(dev), ifa->ifa_local) |
	    (!(ifa->ifa_flags & IFA_F_SECONDARY) &&
	     fib_sync_down_dev(dev, ifa, 0)))

> +	for_ifa(ifr->ifa_dev) {

	Below ifa == ifr check will not be needed when
for_primary_ifa() is used or when fib_sync_down_dev() is
called only for primary IPs. We can see some ifr in the
list only if it is secondary IP deleted during the promotion
process. Without promotion, the primary/secondary ifa is
unlinked before the NETDEV_DOWN notification and we do
not see it here.

> +		if (unlikely(ifa == ifr))
> +			continue;
> +		if (inet_ifa_match(nh->nh_gw, ifa))
> +			return false;
> +	} endfor_ifa(ifr->ifa_dev);
> +
> +	return true;

Regards

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