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

[Sergey Popovich <popovich_sergei@mail.ru>]

В письме от 23 января 2014 12:06:30 пользователь Julian Anastasov написал:
> 	Hello,
> 
> On Tue, 21 Jan 2014, Sergey Popovich wrote:
> > +			if (nexthop_nh->nh_dev != dev ||
> > +			    nexthop_nh->nh_scope == scope ||
> > +			    (ifa && !inet_ifa_match(nexthop_nh->nh_gw, ifa)))
> 
> 	What if nh_gw is part from another smaller/larger subnet?
> For example, what if we still have 10.0.0.200/8 ? 10.0.10.5 is
> still reachable, i.e. fib_check_nh() would create such NH.


Please correct me if I dont understand something:

1. fib_sync_down_dev() is used when interface is going down
to remove entires with stray nexthops (including multipath routes).

2. It takes as its argument device on which event (DOWN for short) is received
and force argument to force fib info entry deletion (which is true when
fib_sync_down_dev() called from fib_disable_ip() with 2 on UNREGISTER event.

Case, that patch is tries to address happens when we have two
or more addresses on interface, and NH exists in one of such subnet.

With two or more address on iface, fib_disable_ip() is not called on single 
address removal, so fib_sync_down_dev() also not called, and we end with 
routes with stray nexthop.

There is no problem with single address and NH in its subnet, as 
fib_sync_down_dev() called from fib_disable_ip().

When deleting IP address, we have net_device where address deleted
and deleted ifa entry.

Only thing that I miss is RTNH_F_ONLINK NH flag, should be consulted
before marking nexthop as dead. I will fix this in v2.


> IMHO, marking NH by exact nh_gw looks more acceptable because
> the exact GW becomes unreachable. Otherwise, you will need
> fib_lookup() as in fib_check_nh() to check that NH becomes
> unreachable.

Not sure that I fully understand you.

When deleting address and removing its subnet, instead of removing route from 
the FIB, resolve new NH with fib_lookup() if possible, as this done in 
fib_check_nh(), and leave route with modified NH?

Well, sounds good, but what to do with multipath routes?
Is this correct at all?

Thanks revieving my patches.

> 
> 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

-- 
SP5474-RIPE
Sergey Popovich