Re: Panic in ipfw

Julian Elischer wrote:
> Robert Watson wrote:
>> On Tue, 3 Jul 2007, Ian FREISLICH wrote:
>>
>>> "Andrey V. Elsukov" wrote:
>>>>
>>>>> I got this panic yesterday on a fairly busy firewall.  I have some 
>>>>> private patches to ip_fw2.c and to the em driver (see the earlier 
>>>>> "em0 hijacking traffic to port 623" thread).  I don't think this 
>>>>> panic is a result of those changes.
>>>>
>>>>> It occurred round about the time an address was added to an interface.
>>>>
>>>> I have a patch that can help you (i guess..). Can you test this patch?
>>>>
>>>> http://butcher.heavennet.ru/patches/kernel/inaddr_locking/
>>>
>>> Thanks.  Wow, that looks like it touches a lot more than just ipfw. 
>>> It took about 1.5 years of production at 2.3 billion backets a day to 
>>> trigger this condition twice.  It's going to be difficult to tell if 
>>> this patch fixes the problem.
>>
>> This, FYI, is actually the reason why the locking isn't there now -- 
>> when prioritizing things to make MPSAFE, performance work, etc, 
>> address/ifnet lists, with the exception of multicast address lists, it 
>> was clear that they were basically static data structures.  Andrey's 
>> patch addresses problems I've wanted to work on for several years, and 
>> I'm very pleased he's working on it. It's only part of the solution to 
>> the long-term problem there -- we have a number of other 
>> synchronization issues for data structures that are nearly almost 
>> always static, and some life cycle issues with ifnet registration.  To 
>> date, I think this is really the first bug report I've seen that I 
>> could authoritatively point the finger at missing synchronization at 
>> the ifnet/ifaddr layer as the source.
>>
>> My hope is that these will be addressed in FreeBSD 8.x, especially 
>> with the upcoming read-mostly locks, which will have almost zero cost 
>> to acquire for read protection, exactly what we'd like to see for 
>> these code paths.  Some of these changes may be mergeable to the 7.x 
>> branch, but will need lots of time to "burn in", as the risks of such 
>> changes are non-trivial.  I did some initial work to properly lock 
>> down the ifaddr address lists for netinet, and found they required 
>> moderate rearrangement of the address management ioctl paths, which 
>> are pretty complex without being rerranged :-).
> 
> I think in this case the answer is to have an ABI to do the "me" lookup 
> that ipfw can call.
> The "This_is_me()" call would be part of the interface system and would 
> work on a cached set of addresses held especially for this purpose 
> (probably hashed).
> The cache would be replaced atomically by the interface module when it 
> became out
> of date.  Part of this is because the current way of looking up "me" is 
> very slow
> and if it is done on every packet, can be a real source of (on systems 
> with lots of vlans
> or aliases) cache flushing and cpu usage.
> 

damn
didn't mean to send this yet.

I was rewritng it and hit the wrong key..

ignore the part about being slow.. I was not taling sense and forgot about the current hash
methods.


Basically doing a lot of locking on every packet is bad.
I'd prefer to have the per-packet work done on a read-on only
copy, in much the same way that Robert and I have been discussing for 
the firewall code itself.





>>
>> Robert N M Watson
>> Computer Laboratory
>> University of Cambridge
> 
>