On Mon, 2013-07-29 at 14:15 -0700, John-Mark Gurney wrote:
> Hans Petter Selasky wrote this message on Mon, Jul 29, 2013 at 19:58 +0200:
> > The aligned will make sure that the structure gets padded properly to the
> > size specified. Only on ARM/MIPS etc, structures get automatically aligned
> > according to the element in the structure requiring the greatest alignment.
> > I've test-compiled the USB WLAN drivers, and the aligned makes a
> > difference. The problem is that the radiotap header skews some following
> > elements, so that they are no longer aligned. The radiotap header itself is
> > packed, and this is not a problem.
>
> Ouch, has anyone looked at the code that caused this?
>
> in ieee80211_radiotap.c, it looks like the original fault was in either
> set_channel, or set_xchannel, and both do (the equivalent of):
> struct {
> uint16_t freq;
> uint16_t flags;
> } *rc = p;
>
> rc->freq = htole16(c->ic_freq);
> rc->flags = htole16(c->ic_flags);
>
If there's any chance the pointer isn't aligned in code like this, then
htole16() is the wrong function, it should be using le16enc() such as
le16enc(&rc->freq, c->ic_freq);
le16enc(&rc->flags, c->ic_flags);
With any luck, an x86 compiler can optimize the inline code for the
endian enc/dec functions to take advantage of the platform's ability to
do unaligned accesses. But that's a side issue to code correctness --
portable code has to get these things right even when it's inefficient.
-- Ian
> And then there is complicated code that calculates offsets, etc, in
> radiotap_offset.. What we probably really need is to mark the above
> as __packed or equiv so that we don't assume that the passed in pointer
> is aligned...
>
> The whole management of this radiochan and radiotap_offset is pretty
> nasty... If marking the structures __packed works, it's probably
> because two bugs are offsetting, and magicly making things align
> again...
>
> > -----Original message-----
> > > From:Warner Losh <i...@bsdimp.com <mailto:i...@bsdimp.com> >
> > > Sent: Monday 29th July 2013 17:04
> > > To: Adrian Chadd <adr...@freebsd.org <mailto:adr...@freebsd.org> >
> > > Cc: Hans Petter Selasky <hans.petter.sela...@bitfrost.no
> > > <mailto:hans.petter.sela...@bitfrost.no> >; freebsd-arm
> > > <freebsd-...@freebsd.org <mailto:freebsd-...@freebsd.org> >;
> > > freebsd-wireless@freebsd.org <mailto:freebsd-wireless@freebsd.org>
> > > Subject: Re: My WLI-UC-GNM up crash
> > >
> > > Aren't structures already aligned to 4 bytes when placed inside other
> > > structures (unless marked __packed)?
> > >
> > > Warner
> > >
> > > On Jul 28, 2013, at 11:50 AM, Adrian Chadd wrote:
> > >
> > > > As long as that results in the radiotap structures being 4 or 8 byte
> > > > padded when it's embedded in the softc - then yes, indeed.
> > > >
> > > > Xiao, can you try?
> > > >
> > > >
> > > > -adrian
> > > >
> > > > On 28 July 2013 03:35, Hans Petter Selasky <h...@bitfrost.no
> > > > <mailto:h...@bitfrost.no> > wrote:
> > > >> Hi,
> > > >>
> > > >> Can you try the attached patch?
> > > >>
> > > >> --HPS
> > > > _______________________________________________
> > > > freebsd-...@freebsd.org <mailto:freebsd-...@freebsd.org> mailing list
> > > > http://lists.freebsd.org/mailman/listinfo/freebsd-arm
> > > > <http://lists.freebsd.org/mailman/listinfo/freebsd-arm>
> > > > To unsubscribe, send any mail to "freebsd-arm-unsubscr...@freebsd.org
> > > > <mailto:freebsd-arm-unsubscr...@freebsd.org> "
> > >
> > >
> >
> > _______________________________________________
> > freebsd-...@freebsd.org mailing list
> > http://lists.freebsd.org/mailman/listinfo/freebsd-arm
> > To unsubscribe, send any mail to "freebsd-arm-unsubscr...@freebsd.org"
>
_______________________________________________
freebsd-wireless@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-wireless
To unsubscribe, send any mail to "freebsd-wireless-unsubscr...@freebsd.org"
