cvs commit: src/sys/net if_vlan.c

[Julian Elischer]

Yar Tikhiy wrote:

>On Thu, Jun 29, 2006 at 01:24:56PM +0400, Maxim Konovalov wrote:
>  
>
>>On Thu, 29 Jun 2006, 07:52-0000, Yar Tikhiy wrote:
>>
>>    
>>
>>>yar         2006-06-29 07:52:30 UTC
>>>
>>>  FreeBSD src repository
>>>
>>>  Modified files:
>>>    sys/net              if_vlan.c
>>>  Log:
>>>  Detach the interface first, do vlan_unconfig() then.
>>>  Previously, another thread could get a pointer to the
>>>  interface by scanning the system-wide list and sleep
>>>  on the global vlan mutex held by vlan_unconfig().
>>>  The interface was gone by the time the other thread
>>>  woke up.
>>>
>>>  In order to be able to call vlan_unconfig() on a detached
>>>  interface, remove the purely cosmetic bzero'ing of IF_LLADDR
>>>  from the function because a detached interface has no addresses.
>>>
>>>  Noticed by:     a stress-testing script by maxim
>>>  Reviewed by:    glebius
>>>      
>>>
>>Still no cookie :-)
>>
>>db> bt
>>Tracing pid 75800 tid 100098 td 0xc2b0e960
>>in_control(c2a1c67c,c02069f6,c40eece0,c2e66000,c2b0e960) at in_control+0x114
>>ifioctl(c2a1c67c,c02069f6,c40eece0,c2b0e960,0,...) at ifioctl+0xee
>>soo_ioctl(c27cb4c8,c02069f6,c40eece0,c2c04980,c2b0e960) at soo_ioctl+0x2db
>>ioctl(c2b0e960,d56a4d04) at ioctl+0x370
>>syscall(3b,3b,3b,bfbfe2c4,0,...) at syscall+0x27e
>>Xint0x80_syscall() at Xint0x80_syscall+0x1f
>>--- syscall (54, FreeBSD ELF32, ioctl), eip = 0x2817cb43, esp =
>>0xbfbfe28c, ebp = 0xbfbfe2d8 ---
>>
>>Let me know if you need more info.
>>    
>>
>
>I stress tested gif(4) in the same manner for kicks and got a very
>similar panic in in_control().  I suppose that my change eliminated
>a concurrency problem in vlan(4) and we began to feel the lack of
>refcounting at ifnet level.  Indeed, a thread can keep a pointer
>to an ifnet beyond its lifetime and panic the system on access to
>the dead ifnet.
>
>  
>

Unfortunatly, since mbufs point to ifnets it is almost impossible to 
"efficiently" refcount
ifnets.  Mbufs may persist almost indefinitly in a socket receive 
buffer, well after the given
receive interface has gone away. I submitted patches to full real 
referenc counting of ifnets
 in 1995 but it was already too cumbersom then, and since then it has 
gotten worse.   (due to SMP etc.)

The solution I guess is to make mbufs reference ifnets by some indirect 
method
that can be validity checked.  The same solution was used to fix a 
reference problem
with proc structures in 1992.  (the PID was stored in some places 
instead of struct proc *).

Basically, we assign a monatomically increasing  number (assuming no 
wrap) to every interface.
We keep that  number in a hash  with a pointer to the  ifnet.
We change the ifp in mbuf headers to hold that number, and always access 
it with
a function (ifnum2ifp(ifnum)) and if it returns NULL then we drop the 
packet as its receive interface
has gone away.   There is already an interface number, but it gets re-used.
(maybe that is not a problem?). You probably should still take out a 
refeence on the ifnet
(or a lock) if you decide to make use of the ifnet. (e.g. probably ipfw 
should
do something to stop it from going away while it is testing recv interface).

this of course introduces some overhead.. how much we can afford is 
another question.