[patch] interface routes

[Andre Oppermann]

On 07.03.2013 12:43, Alexander V. Chernikov wrote:
> On 07.03.2013 11:39, Andre Oppermann wrote:
>> On 07.03.2013 07:34, Alexander V. Chernikov wrote:
>>> Hello list!
>>>
>>> There is a known long-lived issue with interface routes
>>> addition/deletion:
>>>
>>> ifconfig iface inet 1.2.3.4/24 can fail if given prefix is already in
>>> kernel route table (for
>>> example, advertised by IGP like OSPF).
>>>
>>> Interface route can be deleted via route(8) or any route socket user
>>> (sometimes this happens with
>>> popular opensource daemons like bird/quagga).
>>>
>>> Problem is reported at least in kern/106722 and kern/155772.
>>
>> You patch is a welcome addition.
>>
>>> This can be fixed the following way:
>>> Immutable route flag (RTM_PINNED, added in 19995 with 'for future use'
>>> comment) is utilised to mark
>>> route 'immutable'.
>>> rtrequest1_fib refuses to delete routes with given flag unless
>>> RTM_PINNED is set in rti_flags.
>>
>> How do the routing daemons react to being unable to change/delete
>> such a route?
> routing daemons live long with the fact that there route socket cmds can
> fail (and the is route(8) utility which can do anything), so typically
> bird/quagga yells like
> 'bird: KRT: Error sending route 11.0.0.0/24 to kernel: File exists'
> and marks given route as not installed in internal RIB. Additionally,
> daemon will probably re-try to insert such routes on every periodic KRT
> rescan (tens of minutes).

OK. No problem then.

> Given that such sutiations usually happens for a very short time (e.g.
> physical link flaps) everything should become to normal state quickly.
>
>>
>> EADDRINUSE would likely be a more descriptive error instead of EPERM?
> Well, not sure if EADDRINUSE is very descriptive for _deleting_ route.
> "Yes, I know that it is in use so that's the reason I'm trying to delete
> it".

I'm thinking of distinguishing it from a permission denial, because of
insufficient rights (jail or something like that) vs. an explicitly
pinned route.  With EPERM you may look for the problem in the wrong
place.  E*INUSE is a common error for something can't be removed due
to it still being used by or for something else.  Which is the case
here and may be more appropriate.

>>> Every interface address manupulation is done via rtinit[1], so
>>> rtinit1() sets this flag (and behavior does not change here).
>>>
>>> Adding interface address is handled via atomically deleting old prefix
>>> and adding interface one.
>>
>> This brings up a long standing sore point of our routing code
>> which this patch makes more pronounced.  When an interface link
>> state is down I don't want the route to it to persist but to
>> become inactive so another path can be chosen.  This the very
>> point of running a routing daemon.  So on the link-down event
>> the installed interface routes should be removed from the routing
>> table.  The configured addresses though should persist and the
>> interface routes re-installed on a link-up event.  What's your
>> opinion on it?
 >
> This is exactly what is done in current code for IPv4:
> if_down calls if_unroute(), it cals prctlinput() for every interface
> address, and domain-dependent function like rip_ctlinput calls
> in_ifscrub() cleaning given interface route.
> However, address route (/32) still remains (but route daemons, at least
> bird, tends to ignore it since it is not listed as valid interface
> address/mask).

IF_DOWN and link state down are not the same thing.  When the cable
is unplugged the link state goes down but not the interface.

> This is not done for IPv6 and we should probably do the same.

Yes, they should be synchronized.

>> Other than these points I think your code is fine and can go
>> into the tree.

-- 
Andre