Date: Tue, 27 Aug 2013 08:57:35 -0600 From: Ian Lepore <ian@FreeBSD.org> To: Andre Oppermann <andre@FreeBSD.org> Cc: freebsd-arm <freebsd-arm@FreeBSD.org> Subject: Re: ARM network trouble after recent mbuf changes Message-ID: <1377615455.1111.180.camel@revolution.hippie.lan> In-Reply-To: <521CB66B.9020806@freebsd.org> References: <1377550636.1111.156.camel@revolution.hippie.lan> <521BC472.7040804@freebsd.org> <521BD531.4090104@sbcglobal.net> <521C4CD9.4050308@freebsd.org> <0E0536B2-2B7F-4EED-9EFD-4B9E2C2D729A@freebsd.org> <521C87FF.8010100@freebsd.org> <D2FB4EEE-DE17-43CB-9E77-7BCA7D1B9661@freebsd.org> <521CB66B.9020806@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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): --- 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. -- Ian
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1377615455.1111.180.camel>