Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 3 Feb 2012 20:26:02 -0800
From:      Adrian Chadd <adrian@freebsd.org>
To:        freebsd-wireless@freebsd.org
Subject:   [net80211] support vendor bitmap entries; teach if_ath to export PHY error code in error frames
Message-ID:  <CAJ-VmonSrcSQmZeENgH1fagnn6DOMhX6rC-n9=dOtypTabcSLg@mail.gmail.com>

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

[-- Attachment #1 --]
Hi,

As part of my radar project, I'm going to try and teach the BSD/Linux
ath driver and radiotap API about vendor extensions and implement a
(reasonably) well documented way to expose what I need.

The driver already exports the whole PHY error frame payload via
radiotap and you can select which PHY error types are passed up by
setting bits in the dev.ath.X.monpass sysctl. The trouble is the PHY
error code isn't exposed so it currently isn't possible to know
whether its a data or a PHY error frame.

This patch is a WIP and implements what I need. In short:

* The net80211 radiotap code doesn't know about vendor extension
bitmap(s), so there's currently no way to take those into account when
calculating the channel state offset in the header. Since the radiotap
code directly writes into that (rather than have the driver do it per
frame), adding any vendor bitmaps does mess things up quite a bit.

* define a vendor attribute in if_athioctl.h - this should move to
ieee80211_radiotap.h at some point.

* Add fields to the ath RX radiotap struct, which includes the vendor
bitmap, vendor attribute and the vendor payload.

* Add a vendor payload, which includes the Atheros OUI and the RX
status + PHY error code.

With this, userland tools can be built to read the radar and spectral
scan PHY error frames in userland. It makes prototyping code much,
much easier.

What I'm going to do to this:

* Move the vendor attribute definition into ieee80211_radiotap.h;
* Finalise on the Atheros vendor attribute layout. It doesn't match
what is used in the reference driver but I'll use different vendor
bitmap/namespace bytes, so things won't clash.
* Document the new ieee80211_radiotap_attachv() and why it exists.

I'd appreciate some feedback.

Thanks!


Adrian

[-- Attachment #2 --]
Index: net80211/ieee80211_var.h
===================================================================
--- net80211/ieee80211_var.h	(revision 230855)
+++ net80211/ieee80211_var.h	(working copy)
@@ -700,6 +700,11 @@
 		uint32_t tx_radiotap,
 	    struct ieee80211_radiotap_header *rh, int rlen,
 		uint32_t rx_radiotap);
+void	ieee80211_radiotap_attachv(struct ieee80211com *,
+	    struct ieee80211_radiotap_header *th,
+	    int tlen, int n_tx_v, uint32_t tx_radiotap,
+	    struct ieee80211_radiotap_header *rh,
+	    int rlen, int n_rx_v, uint32_t rx_radiotap);
 void	ieee80211_radiotap_detach(struct ieee80211com *);
 void	ieee80211_radiotap_vattach(struct ieee80211vap *);
 void	ieee80211_radiotap_vdetach(struct ieee80211vap *);
Index: net80211/ieee80211_radiotap.c
===================================================================
--- net80211/ieee80211_radiotap.c	(revision 230855)
+++ net80211/ieee80211_radiotap.c	(working copy)
@@ -47,13 +47,24 @@
 
 #include <net80211/ieee80211_var.h>
 
-static int radiotap_offset(struct ieee80211_radiotap_header *, int);
+static int radiotap_offset(struct ieee80211_radiotap_header *, int, int);
 
 void
 ieee80211_radiotap_attach(struct ieee80211com *ic,
 	struct ieee80211_radiotap_header *th, int tlen, uint32_t tx_radiotap,
 	struct ieee80211_radiotap_header *rh, int rlen, uint32_t rx_radiotap)
 {
+	ieee80211_radiotap_attachv(ic, th, tlen, 0, tx_radiotap,
+	    rh, rlen, 0, rx_radiotap);
+}
+
+void
+ieee80211_radiotap_attachv(struct ieee80211com *ic,
+	struct ieee80211_radiotap_header *th,
+	int tlen, int n_tx_v, uint32_t tx_radiotap,
+	struct ieee80211_radiotap_header *rh,
+	int rlen, int n_rx_v, uint32_t rx_radiotap)
+{
 #define	B(_v)	(1<<(_v))
 	int off;
 
@@ -63,9 +74,9 @@
 	/* calculate offset to channel data */
 	off = -1;
 	if (tx_radiotap & B(IEEE80211_RADIOTAP_CHANNEL))
-		off = radiotap_offset(th, IEEE80211_RADIOTAP_CHANNEL);
+		off = radiotap_offset(th, n_tx_v, IEEE80211_RADIOTAP_CHANNEL);
 	else if (tx_radiotap & B(IEEE80211_RADIOTAP_XCHANNEL))
-		off = radiotap_offset(th, IEEE80211_RADIOTAP_XCHANNEL);
+		off = radiotap_offset(th, n_tx_v, IEEE80211_RADIOTAP_XCHANNEL);
 	if (off == -1) {
 		if_printf(ic->ic_ifp, "%s: no tx channel, radiotap 0x%x",
 		    __func__, tx_radiotap);
@@ -79,9 +90,9 @@
 	/* calculate offset to channel data */
 	off = -1;
 	if (rx_radiotap & B(IEEE80211_RADIOTAP_CHANNEL))
-		off = radiotap_offset(rh, IEEE80211_RADIOTAP_CHANNEL);
+		off = radiotap_offset(rh, n_rx_v, IEEE80211_RADIOTAP_CHANNEL);
 	else if (rx_radiotap & B(IEEE80211_RADIOTAP_XCHANNEL))
-		off = radiotap_offset(rh, IEEE80211_RADIOTAP_XCHANNEL);
+		off = radiotap_offset(rh, n_rx_v, IEEE80211_RADIOTAP_XCHANNEL);
 	if (off == -1) {
 		if_printf(ic->ic_ifp, "%s: no rx channel, radiotap 0x%x",
 		    __func__, rx_radiotap);
@@ -260,7 +271,8 @@
  * known -1 is returned.
  */
 static int
-radiotap_offset(struct ieee80211_radiotap_header *rh, int item)
+radiotap_offset(struct ieee80211_radiotap_header *rh,
+    int n_vendor_attributes, int item)
 {
 	static const struct {
 		size_t	align, width;
@@ -334,6 +346,8 @@
 	int off, i;
 
 	off = sizeof(struct ieee80211_radiotap_header);
+	off += n_vendor_attributes * (sizeof(uint32_t));
+
 	for (i = 0; i < IEEE80211_RADIOTAP_EXT; i++) {
 		if ((present & (1<<i)) == 0)
 			continue;
Index: dev/ath/ath_hal/ar5416/ar5416_misc.c
===================================================================
--- dev/ath/ath_hal/ar5416/ar5416_misc.c	(revision 230855)
+++ dev/ath/ath_hal/ar5416/ar5416_misc.c	(working copy)
@@ -770,6 +770,7 @@
 
 	OS_REG_WRITE(ah, AR_PHY_RADAR_0, val | AR_PHY_RADAR_0_ENA);
 
+	/* XXX is this around the correct way?! */
 	if (pe->pe_usefir128 == 1)
 		OS_REG_CLR_BIT(ah, AR_PHY_RADAR_1, AR_PHY_RADAR_1_USE_FIR128);
 	else if (pe->pe_usefir128 == 0)
Index: dev/ath/if_athioctl.h
===================================================================
--- dev/ath/if_athioctl.h	(revision 230855)
+++ dev/ath/if_athioctl.h	(working copy)
@@ -188,10 +188,54 @@
 	(1 << IEEE80211_RADIOTAP_DBM_ANTSIGNAL)	| \
 	(1 << IEEE80211_RADIOTAP_DBM_ANTNOISE)	| \
 	(1 << IEEE80211_RADIOTAP_XCHANNEL)	| \
+	(1 << IEEE80211_RADIOTAP_VENDOREXT)	| \
+	(1 << IEEE80211_RADIOTAP_EXT)		| \
 	0)
 
+/*
+ * This is required to be aligned to two bytes.
+ */
+struct vendor_space {
+	u_int8_t	oui[3];
+	u_int8_t	sub_namespace;
+	u_int16_t	skip_length;
+} __packed;
+
+/*
+ * This is higher than the vendor bitmap used inside
+ * the Atheros reference codebase.
+ */
+
+/* Bit 8 */
+#define	ATH_RADIOTAP_VENDOR_HEADER	8
+
+/*
+ * ATH_RADIOTAP_VENDOR_HEADER
+ *
+ * This is required to be aligned to four bytes.
+ */
+struct ath_radiotap_vendor_hdr {
+	u_int8_t	vh_version;	/* 1 */
+	u_int8_t	vh_phyerr_code;
+	u_int8_t	vh_rstatus;
+	u_int8_t	vh_pad1[1];
+} __packed;
+
 struct ath_rx_radiotap_header {
-	struct ieee80211_radiotap_header wr_ihdr;
+	struct ieee80211_radiotap_header wr_ihdr;	/* 8 */
+	/* Vendor extension header bitmap */
+	u_int32_t	wr_ext_bitmap;		/* 4 */
+
+	/*
+	 * This padding is needed because:
+	 * + the radiotap header is 8 bytes;
+	 * + the extension bitmap is 4 bytes;
+	 * + the tsf is 8 bytes, so it must start on an 8 byte
+	 *   boundary.
+	 */
+	u_int32_t	wr_pad1;
+
+	/* Normal radiotap fields */
 	u_int64_t	wr_tsf;
 	u_int8_t	wr_flags;
 	u_int8_t	wr_rate;
@@ -203,6 +247,17 @@
 	u_int16_t	wr_chan_freq;
 	u_int8_t	wr_chan_ieee;
 	int8_t		wr_chan_maxpow;
+
+	/*
+	 * Vendor header section, as required by the
+	 * presence of the vendor extension bit and bitmap
+	 * entry.
+	 */
+	struct vendor_space		wr_vh;	/* 6 bytes */
+
+	struct ath_radiotap_vendor_hdr	wr_v;	/* 4 bytes */
+
+	/* XXX padding needed? */
 } __packed;
 
 #define ATH_TX_RADIOTAP_PRESENT (		\
Index: dev/ath/if_ath.c
===================================================================
--- dev/ath/if_ath.c	(revision 230855)
+++ dev/ath/if_ath.c	(working copy)
@@ -766,10 +766,14 @@
 	ic->ic_addba_stop = ath_addba_stop;
 	ic->ic_bar_response = ath_bar_response;
 
-	ieee80211_radiotap_attach(ic,
-	    &sc->sc_tx_th.wt_ihdr, sizeof(sc->sc_tx_th),
+	/*
+	 * There's one vendor bitmap entry in the RX radiotap
+	 * header; make sure that's taken into account.
+	 */
+	ieee80211_radiotap_attachv(ic,
+	    &sc->sc_tx_th.wt_ihdr, sizeof(sc->sc_tx_th), 0,
 		ATH_TX_RADIOTAP_PRESENT,
-	    &sc->sc_rx_th.wr_ihdr, sizeof(sc->sc_rx_th),
+	    &sc->sc_rx_th.wr_ihdr, sizeof(sc->sc_rx_th), 1,
 		ATH_RX_RADIOTAP_PRESENT);
 
 	/*
@@ -3884,6 +3888,34 @@
 }
 
 static void
+ath_rx_tap_vendor(struct ifnet *ifp, struct mbuf *m,
+    const struct ath_rx_status *rs, u_int64_t tsf, int16_t nf)
+{
+	struct ath_softc *sc = ifp->if_softc;
+
+	/* Fill in the extension bitmap */
+	sc->sc_rx_th.wr_ext_bitmap = htole32(1 << ATH_RADIOTAP_VENDOR_HEADER);
+
+	/* Fill in the vendor header */
+	sc->sc_rx_th.wr_vh.oui[0] = 0x7f;
+	sc->sc_rx_th.wr_vh.oui[1] = 0x03;
+	sc->sc_rx_th.wr_vh.oui[2] = 0x00;
+
+	/* XXX what should this be? */
+	sc->sc_rx_th.wr_vh.sub_namespace = 0;
+	sc->sc_rx_th.wr_vh.skip_length =
+	    htole16(sizeof(struct ath_radiotap_vendor_hdr));
+
+	/* phyerr info */
+	sc->sc_rx_th.wr_v.vh_version = 1;
+	sc->sc_rx_th.wr_v.vh_rstatus = rs->rs_status;
+	if (rs->rs_status & HAL_RXERR_PHY)
+		sc->sc_rx_th.wr_v.vh_phyerr_code = rs->rs_phyerr;
+	else
+		sc->sc_rx_th.wr_v.vh_phyerr_code = 0;
+}
+
+static void
 ath_rx_tap(struct ifnet *ifp, struct mbuf *m,
 	const struct ath_rx_status *rs, u_int64_t tsf, int16_t nf)
 {
@@ -4156,6 +4188,7 @@
 				m->m_pkthdr.len = m->m_len = len;
 				bf->bf_m = NULL;
 				ath_rx_tap(ifp, m, rs, rstamp, nf);
+				ath_rx_tap_vendor(ifp, m, rs, rstamp, nf);
 				ieee80211_radiotap_rx_all(ic, m);
 				m_freem(m);
 			}

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-VmonSrcSQmZeENgH1fagnn6DOMhX6rC-n9=dOtypTabcSLg>