[U-Boot] IP_t should be a "packed" struct

[Ben Warren <biggerbadderben@gmail.com>]

Jerry Van Baren wrote:
> Ben Warren wrote:
>> Luigi 'Comio' Mantellini wrote:
>>> Hi ML,
>>>
>>> I'm working on a mips target and I used qemu_mips target to simulate 
>>> my target (that I hope to have in the next week...)
>>>
>>> Following my activities I noticed that IP_t structure is no defined 
>>> with attribute "packed". I noticed this issue because using a 
>>> self-made toolchain (gcc4.2.4+binutils2.8+uclibc0.9.30) the compiler 
>>> has aligned all bytes to 32bit boundary. This is not ok, because the 
>>> packets IP_t can be non aligned (see the /net/net.c PingSend 
>>> function, for an example).
>>>
>>>   
>> Why is your compiler aligning all bytes to 32-bit boundary?  Seems 
>> like an awful waste of space.  This struct should pack itself nicely, 
>> and does on the small sample of toolchains I've tried (gcc 4.3.2 
>> x86_64 and gcc 4.0.0 ppc_4xx).
>
> The compiler is optimizing for speed and/or execution size at the 
> expense of larger data structures either by command (e.g. a -O option) 
> or as part of the compiler writer's choice.  CPUs almost always 
> execute code significantly faster when the data is properly aligned.  
> Many CPUs require software to deal with the misalignment which costs 
> code space and execution time.
>
> Since the compiler wasn't instructed that the IP headers needed to be 
> packed, it is within the compiler's right to not pack them.
>
Sure.  In this case, though, the optimization's not necessarily going to 
gain anything since we never use a raw struct IP_t, only pointers that 
overlay other char arrays through casting.   Of course there's no point 
discussing this much further here, but I suspect that this packing 
problem will exist in many more places than the protocol headers.
> [snip]
>
> Best regards,
> gvb
>
regards,
Ben