Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 27 Aug 2013 15:29:37 +0200
From:      Michael Tuexen <tuexen@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:  <D2FB4EEE-DE17-43CB-9E77-7BCA7D1B9661@freebsd.org>
In-Reply-To: <521C87FF.8010100@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>

next in thread | previous in thread | raw e-mail | index | archive | help

On Aug 27, 2013, at 1:05 PM, Andre Oppermann <andre@FreeBSD.org> wrote:

> On 27.08.2013 11:38, Michael Tuexen wrote:
>> On Aug 27, 2013, at 8:53 AM, Andre Oppermann <andre@FreeBSD.org> =
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.
>=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.
Done. r254954 with your patch applied runs fine on a RPi.

Best regards
Michael
>=20
> --=20
> Andre
>=20
> <m_hdr-alignment-20130827.diff>




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?D2FB4EEE-DE17-43CB-9E77-7BCA7D1B9661>