ARM network trouble after recent mbuf changes

[Warner Losh]

On Aug 27, 2013, at 12:58 AM, Michael Tuexen wrote:

> On Aug 27, 2013, at 8:53 AM, Andre Oppermann <andre at FreeBSD.org> wrote:
> 
>> On 27.08.2013 08:30, Michael Tuexen wrote:
>>> On Aug 27, 2013, at 7:53 AM, Hans Petter Selasky <hps at bitfrost.no> wrote:
>>> 
>>>> On 08/27/13 00:38, Michael Tuexen wrote:
>>>>> I did some tests with a small program. Having in struct pkthdr 64 bit entities
>>>>> results in a 64 bit alignment when used in struct mbuf. Using __packed
>>>>> for struct mbuf, removes the padding.
>>>> 
>>>> 
>>>> Hi,
>>>> 
>>>> Maybe you could use __aligned(8) instead, and account for the extra padding on all platforms? Packed has other disadvantages on ARM platforms when accessing the structures, like that non-aligned access is possible, and that it is sometimes slower than aligned access.
>>> Isn't there a performance penalty when accessing 64-bit entities not being 64-bit
>>> aligned? If that is the case, wouldn't it make sense to add a 4 byte padding to
>>> struct m_hdr for ILP32? Then the problem should go away...
>> 
>> Either that or define MLEN and MHLEN in a way that actually reflects the true
>> size of what they are representing.  The latter is the true bug.
> Correct. There is the hidden assumption that there is no padding. Maybe you can
> put that in a comment...

Maybe we can have it as a CTASSERT. Better than any comment. CTASSERTS are free and catch this kind of thing...

> We could also have some code (maybe under INVARIANTS) where we check that
> an struct mbuf has the size MSIZE and panic if not.
> This would make things clearer if the problem happens again.
>> 
>>> We could also get rid of the 64 bit alignment by not having 64-bit entities in
>>> struct pkthdr. Removing sixtyfour should be easy. However, we now have also
>>> uint64_t csum_flags.
>> 
>> Yes, the 64 bit fields are to be used to store packet associated information
>> on its way through the stack.  Reducing it to 32 bits would somewhat defeat
>> their purpose.
> I completely agree...
>> 
>> -- 
>> Andre
>> 
>> 
> 
> _______________________________________________
> freebsd-arm at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-arm
> To unsubscribe, send any mail to "freebsd-arm-unsubscribe at freebsd.org"