GNATS-1110: in_cksum_hdr error in PC386 BSP

[Thomas Doerfler (nt)]

Till,

congratulations!

It seems it is really necessary to review ALL "asm" inserts for memory
barrier requirements.
wkr,
Thomas.

Till Straumann schrieb:
> I believe I caught this one.
> Thanks to Danilliu for reporting this and his/(her?)
> perseverance.
> 
> It is definitely a toolchain/optimization issue:
> 
> The BSD code does
> 
> header->checksum = 0;
> header->checksum = ip_cksum_hdr(&header);
> 
> The inline ip_cksum_hdr() routine has inline assembly
> and gcc doesn't consider that the assembly actually
> could read 'header->checksum' (which it does) so it thinks the
> header->checksum = 0 assignment is unnecessary
> and optimizes it away :-(
> 
> Confirmed by disassembly of Danilliu's broken
> and a working binary.
> 
> [ As soon as any 'real' subroutine (like a printf)
> is called from ip_cksum_hdr() gcc realizes that
> the header->checksum=0 assigment could have
> side-effects and the problem goes away].
> 
> A memory barrier could resolve this.
> 
> 
> -- Till
> 
> 
> Ralf Corsepius wrote:
> 
>> On Wed, 2006-06-28 at 09:54 -0500, Eric Norum wrote:
>>  
>>
>>> On Jun 28, 2006, at 9:39 AM, Ralf Corsepius wrote:
>>>
>>>   
>>>
>>>> On Wed, 2006-06-28 at 08:47 -0500, Eric Norum wrote:
>>>>
>>>>
>>>>     
>>>>
>>>>> The checksum routines are very heavily used so it's really
>>>>> important  that they be as fast as possible.  Do you have an
>>>>> estimate of the
>>>>>
>>>>> relative times of the old and new versions of this code?
>>>>>       
>>>>
>>>> heavily used and broken don't align smoothly.
>>>>     
>>>
>>> Yes, I think that they might in this case.
>>> 1) This problem may have just shown up as a result of the change from
>>> gcc-3.x to gcc-4.x.
>>> 2) Joel has reported for some time that qemu has been broken.
>>>   
>>
>> Somebody might want to test on real hardware.
>>
>>  
>>
>>>> Either these routines have been heavily used, then they must have
>>>> been
>>>> working, or not - Then I don't understand why nobody has tripped
>>>> over
>>>> this issue in the years this code is in place.
>>>>     
>>>
>>> Because in previous years they were using a different tool chain.
>>>   
>>
>> It's always a comfortable excuse to accuse the toolchain :-)
>>
>> Did you check the asm being generated?
>>
>>  
>>
>>>> IMO, the PR is way to vague to give any trust in it.
>>>>     
>>>
>>> Others have reported problems with networking on the pc386.
>>>   
>>
>> This doesn't mean much, esp. because most networking on little endian
>> targets always is a magnitude difficult than on big endian ones.
>>
>>  
>>
>>>  This is the first time I've seen a patch though.
>>>   
>>
>> I presume, explicitly testing this checksum code on real hardware with
>> somebody being familiar to i386s isn't much of a problem.
>>
>> Ralf
>>
>>
>>
>>  
>>


-- 
--------------------------------------------
IMD Ingenieurbuero fuer Microcomputertechnik
Thomas Doerfler           Herbststrasse 8
D-82178 Puchheim          Germany
email:    Thomas.Doerfler at imd-systems.de
PGP public key available at:
     http://www.imd-systems.de/pgpkey_en.html