ARM network trouble after recent mbuf changes

[Michael Tuexen]

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

> On 27.08.2013 11:38, Michael Tuexen wrote:
>> On Aug 27, 2013, at 8:53 AM, Andre Oppermann <andre at FreeBSD.org> wrote:
>> 
>>> On 27.08.2013 00:22, Thomas Skibo wrote:
>>>> On 8/26/13 2:11 PM, Andre Oppermann wrote:
>>>>> 
>>>>> Can you try this patch see check if it makes a difference on the bitfield?
>>>> 
>>>> 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.
>>> 
>>> 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.
>>> 
>>> On LP64 all is fine.  On i386 it turns out to be fine too because doesn't
>>> care.
>>> 
>>> 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.
>>> 
>>> 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.
>>> 
>>> --
>>> Andre
>>> 
>>> Index: sys/mbuf.h
>>> ===================================================================
>>> --- 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...
> 
> Thanks.  I've changed the test accordingly.
Great. My testing of the !defined(__LP64__) stuff shows that it works.
> 
> 
> 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.
I'll do and let you know. It takes a couple of hours.
The patch looks good...

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