FreeBSD bugs ;)

[Alexander V. Chernikov]

Ondrej Zajicek wrote:
> On Sun, Jan 22, 2012 at 05:41:40PM +0400, Alexander V. Chernikov wrote:
>> Ondrej Zajicek wrote:
>>> On Thu, Jan 19, 2012 at 03:34:30PM +0400, Alexander V. Chernikov wrote:
>>>>>> So you removed it? Or renamed?
>>>>> I  removed  it,  so another interface took it's interface id, but bird
>>>>> kept associating previous name (vlan's) with this id. - renaming fails
>>>>> in principally the same way: bird keeps previous name ;)
>>>>>
>>>>>> It is supposed to work (it is handled as removing and readding the interface),
>>>>>> but probably not tested.
>>>>> Well,  I was supposed to be awfully rich by now, but it wasn't tested,
>>>>> hence it didn't work :>
>>>> Attached patch seems to work on FreeBSD.
>>>> For OpenBSD rtsock events seems to be the same (and interface renaming
>>>> is not supported there at all).
>>>> I don't have any NetBSD instance to test on, unfortunately
>>>>
>>>> Patch is not finished: interface renaming has to be handled in
>>>> appropriate protocols. In OSPF, for example, it is a bit tricky due to
>>>> the fact that interface can belong to several areas
>>> I guess that seamless renaming is not really important, so it could be
>>> handled just by IF_CHANGE_TOO_MUCH (i.e. virtual iface down and up),
>>> that would work for any protocol. (or, because ifaces in BIRD are
>>> generally keyed by name, just remove the old one before creating the new
>>> one) BTW, AFAIK Linux does not allow iface renaming unless the iface is
>>> down.
>> I've already got some rename-handling code in OSPF but it is not fully
>> tested.. Maybe it is really better not to bother about keeping interface
>> UP.
> 
> BTW, i already have some minor fixes in iface scanning code in GIT i
I saw, RSS is a useful thing :)
> discovered yesterday. I would merge your patch and my comments to
> fix these bugs.
Okay, so you'll finish this and we skip refcount stuff for now?
> 
>>> struct_iface should not be freed when interface disappears. There are
>>> users for that (most recently link-local sticky neighbors, but AFAIK
>>> there are others i don't remember now). Perhaps these structures should
>> So the right way is to change such users behavior ? We definitely know
>> interface is destroyed by OS and we can't predict if it ever comes up
>> again and name / iface index / addresses are the same.
> 
> This is just technical issue whether a struct iface has to be removed
> immediately (synchronously) when an iface disappears, or a bit later
> when it would be removed anyway. For example, OSPF would probably hold
> some references for these structures until next recalculation. I guess
> that it is much simpler and convenient to add reference counting to
> struct iface than to modify all these users. It is true that currently
> we just never free these structs, which is bad but not particularly
> problematic. But fixing this is a different issue. So we could probably
> fix renaming/creating/removing ifaces now and i will add these reference
> counting later.
> 
>> Moreover, imagine
>> bird running on some kind of PPTP server with several thousand interface
>> UP which are destroyed/created with different interface indexes, names
>> and addreses with the rate of several interfaces per second.
> 
> Well, BIRD would have probably a problem in such settings, because there
> are too many parts of the code that would run in O(n^2) time (where n is
> a number of ifaces).
Another thing to fix / think of (at least for me)
> 
>> For NEF_STICKY we can either set iface pointer to NULL (and teach
>> neighbor-api consumers to check this, which is fair) or simply get some
>> gloval static "DUMMY" interface structure and change interface pointer
>> to it.
> 
> This is an issue for link-local sticky neighbors, which obviously needs
> a way to specify an interface. We could use explicit string, but that
> would make it two times more complicated. It is a kind of sticky iface
> entry.

> 
>>> be reference counted, but keeping them with IF_SHUTDOWN is OK. One
>>> thing that was a cause for a bug related to renaming is that shutdown
>>> ifaces kept their iface index and are returned by if_find_by_index().
>>> Simple fix for that would probably make destroying and creating ifaces
>>> work.
>>> RTM_IFANNOUNCE seems useful. Am i understand that correctly that any
>>> iface changes are notified using RTM_IFINFO, just create and destroy of
>>> ifaces are notified using RTM_IFANNOUNCE? And also that when newly
>> Yes. This is stated in route(4):
>>      #define RTM_IFINFO      0xe    /* iface going up/down etc. */
>>      #define RTM_IFANNOUNCE  0x11   /* iface arrival/departure */
>>
>> To be more precise:
>> ifconfig vlan999 create inet 1.2.3.4/29
> 
> Thanks
> 
> BTW, i also noticed that 'ifconfig re0 down' on BSD would first trigger
> RTM_DELADDR (for an address of iface), then RTM_IFINFO (for iface down),
> but in the next scan, the address is still reported. That seems
> a bit inconsistent to me, but not a big issue (interfaces disappears in
> BIRD and then reappears, but as the iface is down, it is not propagated
> anywhere).

This is interesting. The code that does this also sends such messages
for IPv4 address family only which is inconsistent twice. Thanks for
pointing.

> 
>> triggers the following:
>>
>> got message of size 24 on Sun Jan 22 17:11:43 2012
>> RTM_IFANNOUNCE: interface arrival/departure: len 24, if# 15, what: arrival
>>
>> got message of size 184 on Sun Jan 22 17:11:43 2012
>> RTM_DELETE: Delete Route: len 184, pid: 0, seq 0, errno 0,
>> flags:<UP,GATEWAY,STATIC>
>> locks:  inits:
>> sockaddrs: <DST,GATEWAY,NETMASK>
>>  default default default
>>
> ...
>