From owner-freebsd-arm@FreeBSD.ORG Tue Aug 27 12:04:46 2013 Return-Path: Delivered-To: freebsd-arm@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id D522F6F7; Tue, 27 Aug 2013 12:04:46 +0000 (UTC) (envelope-from tuexen@freebsd.org) Received: from mail-n.franken.de (drew.ipv6.franken.de [IPv6:2001:638:a02:a001:20e:cff:fe4a:feaa]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 67ADC226D; Tue, 27 Aug 2013 12:04:46 +0000 (UTC) Received: from [192.168.1.200] (p508F3337.dip0.t-ipconnect.de [80.143.51.55]) (Authenticated sender: macmic) by mail-n.franken.de (Postfix) with ESMTP id 910BC1C0C0692; Tue, 27 Aug 2013 14:04:44 +0200 (CEST) Content-Type: text/plain; charset=iso-8859-1 Mime-Version: 1.0 (Mac OS X Mail 6.5 \(1508\)) Subject: Re: ARM network trouble after recent mbuf changes From: Michael Tuexen In-Reply-To: <521C87FF.8010100@freebsd.org> Date: Tue, 27 Aug 2013 14:04:43 +0200 Content-Transfer-Encoding: quoted-printable Message-Id: <30F80BE6-9580-4166-BD12-09AABD65CCE5@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> To: Andre Oppermann X-Mailer: Apple Mail (2.1508) Cc: freebsd-arm X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Porting FreeBSD to the StrongARM Processor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Aug 2013 12:04:47 -0000 On Aug 27, 2013, at 1:05 PM, Andre Oppermann wrote: > On 27.08.2013 11:38, Michael Tuexen wrote: >> On Aug 27, 2013, at 8:53 AM, Andre Oppermann = wrote: >>=20 >>> On 27.08.2013 00:22, Thomas Skibo wrote: >>>> On 8/26/13 2:11 PM, Andre Oppermann wrote: >>>>>=20 >>>>> Can you try this patch see check if it makes a difference on the = bitfield? >>>>=20 >>>> Actually, this works for me. But, I'm worried that somewhere else = something is going to trip over a >>>> struct pkthdr not being 64-bit aligned. There are several 64-bit = fields in there. >>>=20 >>> The problem is the disconnect between the definition of MLEN and = MHLEN and >>> the effective size/padding of struct mbuf. That's the true bug. >>>=20 >>> On LP64 all is fine. On i386 it turns out to be fine too because = doesn't >>> care. >>>=20 >>> MLEN and MHLEN are incorrectly derived. In fact they should be = derived from >>> stuct mbuf where this padding would be taking into account. However = the way >>> it is structured right now it that would create a circular = dependency. >>>=20 >>> Please try the patch below to confirm. If it fixes your problem for = now >>> I'm going to commit as an immediate fix while searching for a better = long >>> term stable solution. >>>=20 >>> -- >>> Andre >>>=20 >>> Index: sys/mbuf.h >>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >>> --- sys/mbuf.h (revision 254953) >>> +++ sys/mbuf.h (working copy) >>> @@ -94,6 +94,9 @@ >>> int32_t mh_len; /* amount of data in this = mbuf */ >>> uint32_t mh_type:8, /* type of data in this mbuf = */ >>> mh_flags:24; /* flags; see below */ >>> +#if defined(__ILP32__) >>> + uint32_t mh_pad; /* pad to 64 bit alignment = */ >>> +#endif >>> }; > > >> OK. It doesn't work. The reason is, that __ILP32__ is not defined... = At >> lease I don't see it anywhere in the BSD stack. So I'm currently = rebuilding >> with #if !defined(__LP64__) instead. I'll let you know... >=20 > Thanks. I've changed the test accordingly. Great. My testing of the !defined(__LP64__) stuff shows that it works. >=20 >=20 > 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. >=20 > 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). >=20 > http://people.freebsd.org/~andre/m_hdr-alignment-20130827.diff >=20 > Please test. I'll do and let you know. It takes a couple of hours. The patch looks good... Best regards Michael >=20 > --=20 > Andre >=20 >