On Mon, Jul 11, 2005 at 11:30:27PM +0200, Matthias Drochner wrote:
> 
> dyoung@pobox.com said:
> > > IEEE80211_RADIOTAP_F_FCS flag
> > > [...]
> > > should be within the outer header
> > Outer header?
> 
> Well, the header before the variable parts, where one can
> just reference an element without decoding more than
> necessary.
> As said, thare would be room in the it_pad field without
> breaking an API, but as I see it adding another element
> wouldn't do much damage either.
> 
> > > In order to extract the ieee802_11 frame type from a ..._radiotap
> > > frame, one needs start offset and length of the payload.
> > > [...]
> > It shouldn't, and it doesn't.  See the it_len member.
> 
> OK, the fixed 64-byte it_len should work with all payload start
> alignments if the hardware is not too strange, but there is
> no way to express a payload length which is smaller than
> the raw mbuf length. The latter would be needed to pass just
> the 80211 packet along, without radiotap stuff.
> 
> > >    (btw, the DATAPAD field flag should be there too)
> > I don't understand.
> 
> According to the comment in ieee80211_radiotap.h, the
> IEEE80211_RADIOTAP_F_DATAPAD field would tell that there
> is a padding between the 802.11 header and the payload.
> Again, it should be easy to just extract the payload without
> decoding everything else. Not strictly necessary, and forgivable
> if there are historical reasons, but I wouldn't do it that
> way for a new design.
> 
> > There are sensible alignment rules.  All >8-bit fields are aligned on
> > their natural boundary.
> 
> Hmm -- the order of elements in the structure depends on
> the driver/hardware's capabilities, and the actual definitions
> all have a __packed__ attached.
> And the dissector in "ethereal" really counts bytes, without
> considering alignment.

As I say, there are rules.  On any given radio, there are only two
structs, one for tx, one for rx.

If ethereal really counts bytes, w/o considering alignment, then they
seriously botched their implementation, and you should file a PR against
ethereal.

I am going to check in some changes to the manual page shortly, and
update the install sets so that it is installed.  Read the manual page,
then read ieee80211_radiotap.h, then read the reference implementation
in tcpdump.  If the description in the manual page does not agree with
the header file and the reference implementation, then send a PR.

The actual definitions *are* __packed__, but the definitions are
*explicitly* padded to natural boundaries---or else they're broken like
ethereal is broken.

I use __packed__ to avoid gratuitous padding by the compiler: IIUC,
it would be perfectly ok, as far as the C standard is concerned, if
the C compiler for a 64-bit machine aligned *every* struct member on a
64-bit boundary.  Correct me if I'm wrong.

There is some question in my mind whether compilers like gcc generate
efficient code for __packed__ structs, even if members are naturally
aligned.  Perhaps there is a more appropriate compiler directive?

In sum, I am not convinced that there is a design flaw in radiotap.
The documentation may not be true to my intent.  Implementations like
ethereal's may be badly broken.

Dave

-- 
David Young             OJC Technologies
dyoung@ojctech.com      Urbana, IL * (217) 278-3933