ARM network trouble after recent mbuf changes

[Andre Oppermann]

On 27.08.2013 16:57, Ian Lepore wrote:
> On Tue, 2013-08-27 at 16:23 +0200, Andre Oppermann wrote:
>> On 27.08.2013 15:29, Michael Tuexen wrote:
>>> On Aug 27, 2013, at 1:05 PM, Andre Oppermann <andre at FreeBSD.org> wrote:
>>>> Thanks.  I've changed the test accordingly.
>>>>
>>>> While doing the CTASSERTs to prevent such an incident in the future I stumbled
>>>> across a bit of evil name space pollution in mbuf.h.  It is impossible to take
>>>> sizeof(struct m_ext) because "m_ext" is redefined to point into struct mbuf.
>>>>
>>>> In addition to the alignment fix I've solved the namespace issues with m_ext
>>>> and the stupidly named struct pkthdr as well and properly prefixed them.  The
>>>> fallout from LINT was zero (as it should be).
>>>>
>>>> http://people.freebsd.org/~andre/m_hdr-alignment-20130827.diff
>>>>
>>>> Please test.
>>   >
>>> Done. r254954 with your patch applied runs fine on a RPi.
>>
>> Does the CTASSERT trigger if the padding in m_hdr is not there?
>>
>
> Yes:
>
>    --- uipc_mbuf.o ---
>    /local/build/staging/freebsd/bb/src/sys/kern/uipc_mbuf.c:91:1: error: static_assert failed "compile-time assertion failed"
>    CTASSERT(sizeof(struct mbuf) == MSIZE);
>
>
> Since the MLEN and MHLEN macros are used to size the data arrays based
> on the assumption that they begin at a given offset within the struct,
> these asserts more directly express that:
>
>    CTASSERT(MSIZE - offsetof(struct mbuf, m_dat) == MLEN);
>    CTASSERT(MSIZE - offsetof(struct mbuf, m_pktdat) == MHLEN);
>
> With these added you get all 3 asserts and somewhat more of a clue about
> why things are wrong (other than just sizeof mbuf being wrong):

Excellent.  I exchanged them for the struct mbuf member asserts.

>    --- uipc_mbuf.o ---
>    /local/build/staging/freebsd/bb/src/sys/kern/uipc_mbuf.c:91:1: error: static_assert failed "compile-time assertion failed"
>    CTASSERT(sizeof(struct mbuf) == MSIZE);
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    /local/build/staging/freebsd/bb/src/sys/sys/systm.h:100:21: note: expanded from macro 'CTASSERT'
>    #define CTASSERT(x)     _Static_assert(x, "compile-time assertion failed")
>                            ^
>    /local/build/staging/freebsd/bb/src/sys/kern/uipc_mbuf.c:97:1: error: static_assert failed "compile-time assertion failed"
>    CTASSERT(MSIZE - offsetof(struct mbuf, m_dat) == MLEN);
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    /local/build/staging/freebsd/bb/src/sys/sys/systm.h:100:21: note: expanded from macro 'CTASSERT'
>    #define CTASSERT(x)     _Static_assert(x, "compile-time assertion failed")
>                            ^              ~
>    /local/build/staging/freebsd/bb/src/sys/kern/uipc_mbuf.c:98:1: error: static_assert failed "compile-time assertion failed"
>    CTASSERT(MSIZE - offsetof(struct mbuf, m_pktdat) == MHLEN);
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    /local/build/staging/freebsd/bb/src/sys/sys/systm.h:100:21: note: expanded from macro 'CTASSERT'
>    #define CTASSERT(x)     _Static_assert(x, "compile-time assertion failed")
>                            ^              ~
>
>
> IMO, it would be great if MLEN and MHLEN could be #define'd in terms of
> offsetof() expressions, but the compiler is unhappy about asking for
> offsetof an incomplete struct, even though it has all the info it needs
> at the point the expression is encountered.

Yeah, I couldn't find a way either to solve this nasty circular dependency.

-- 
Andre