Date: Thu, 9 Feb 2012 23:18:13 -0800 From: Adrian Chadd <adrian@freebsd.org> To: Bernhard Schmidt <bschmidt@freebsd.org> Cc: freebsd-wireless@freebsd.org Subject: Re: [net80211] support vendor bitmap entries; teach if_ath to export PHY error code in error frames Message-ID: <CAJ-Vmonmk=0maQcJd8iZscZvRxi%2BZcZiwWQyZ1r1Ju4QyhgKXw@mail.gmail.com> In-Reply-To: <CAJ-Vmo=6xF8pWceBQVb4x3PYbLoGiiiAh9u2ZA8A=bAQdrOFdg@mail.gmail.com> References: <CAJ-VmonSrcSQmZeENgH1fagnn6DOMhX6rC-n9=dOtypTabcSLg@mail.gmail.com> <CAAgh0_aAijDoguQCJO8jR=cZ5LrZGLfxpGruZWGsjGf987-zOw@mail.gmail.com> <CAJ-Vmon%2BpQ9U7-YEQ1pYVbHM=5D=cp-z6eoPGJhV_GU3r7hy8A@mail.gmail.com> <201202062059.16816.bschmidt@freebsd.org> <CAJ-Vmo=6xF8pWceBQVb4x3PYbLoGiiiAh9u2ZA8A=bAQdrOFdg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
Hi,
Please find the second pass of this attached.
Fixes:
* I was missing a call to ath_rx_tap_vendor() when processing normal frames
* I've extended the data stored in the radiotap vendor header to
include the EVM and per-chain RSSI.
* I'm also storing the _raw_ combined RSSI in the radiotap header.
* The rs_status and rs_phyerr_code fields are also stored.
The EVM isn't strictly needed for radar work and the only RSSI
required for radar is rssi_ctl[0] and rssi_ext[0] (with the combined
RSSI used in some instances.)
What's left:
* I have a need for some continuous feedback about how busy the
primary/extension channels are. I may add some method of encoding this
in the radiotap header as a vendor extension; I'm not sure yet.
I don't like the idea of doing this via radiotap but I need a way to
encode this live as part of the stream so it can be played back with
the rest of the radar error frames.
I'm not sure if I really _want_ this to be part of the default FreeBSD
build but honestly, having these as vendor extensions will make
debugging a few things a lot easier moving forward. For example, users
could begin writing live traffic sniffing applications that track
per-RX-rate EVM in userland and provide graphical feedback.
Bernhard: what do you think?
Adrian
[-- Attachment #2 --]
Index: sys/net80211/ieee80211_node.c
===================================================================
--- sys/net80211/ieee80211_node.c (revision 230855)
+++ sys/net80211/ieee80211_node.c (working copy)
@@ -34,6 +34,8 @@
#include <sys/mbuf.h>
#include <sys/malloc.h>
#include <sys/kernel.h>
+#include <sys/types.h>
+#include <sys/proc.h>
#include <sys/socket.h>
@@ -431,6 +433,9 @@
ni = ieee80211_alloc_node(&ic->ic_sta, vap, vap->iv_myaddr);
KASSERT(ni != NULL, ("unable to setup initial BSS node"));
obss = vap->iv_bss;
+ printf("[%lld]: %s: iv_bss=%p, new iv_bss=%p\n",
+ (long long int) curthread->td_tid,
+ __func__,vap->iv_bss, ni);
vap->iv_bss = ieee80211_ref_node(ni);
if (obss != NULL) {
copy_bss(ni, obss);
@@ -700,6 +705,9 @@
/*
* Committed to selbs, setup state.
*/
+ printf("[%lld]: %s: selbs=%p, obss=%p\n",
+ (long long int) curthread->td_tid,
+ __func__, selbs, vap->iv_bss);
obss = vap->iv_bss;
/*
* Check if old+new node have the same address in which
Index: sys/net80211/ieee80211_wds.c
===================================================================
--- sys/net80211/ieee80211_wds.c (revision 230855)
+++ sys/net80211/ieee80211_wds.c (working copy)
@@ -46,6 +46,8 @@
#include <sys/errno.h>
#include <sys/proc.h>
#include <sys/sysctl.h>
+#include <sys/types.h>
+#include <sys/proc.h>
#include <net/if.h>
#include <net/if_media.h>
@@ -175,6 +177,9 @@
/*
* Committed to new node, setup state.
*/
+ printf("[%lld]: %s: iv_bss=%p, new iv_bss=%p\n",
+ (long long int) (curthread->td_tid),
+ __func__, vap->iv_bss, ni);
obss = vap->iv_bss;
vap->iv_bss = ni;
ni->ni_wdsvap = vap;
@@ -197,6 +202,9 @@
*/
ni = ieee80211_node_create_wds(vap, vap->iv_des_bssid, chan);
if (ni != NULL) {
+ printf("[%lld]: %s: iv_bss=%p, new iv_bss=%p\n",
+ (long long int) (curthread->td_tid),
+ __func__, vap->iv_bss, ni);
obss = vap->iv_bss;
vap->iv_bss = ieee80211_ref_node(ni);
ni->ni_flags |= IEEE80211_NODE_AREF;
Index: sys/net80211/ieee80211_var.h
===================================================================
--- sys/net80211/ieee80211_var.h (revision 230855)
+++ sys/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: sys/net80211/ieee80211_radiotap.c
===================================================================
--- sys/net80211/ieee80211_radiotap.c (revision 230855)
+++ sys/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: sys/net80211/ieee80211_radiotap.h
===================================================================
--- sys/net80211/ieee80211_radiotap.h (revision 230855)
+++ sys/net80211/ieee80211_radiotap.h (working copy)
@@ -54,6 +54,12 @@
#define IEEE80211_RADIOTAP_HDRLEN 64 /* XXX deprecated */
+struct ieee80211_radiotap_vendor_header {
+ uint8_t vh_oui[3]; /* 3 byte vendor OUI */
+ uint8_t vh_sub_ns; /* Sub namespace of this section */
+ uint16_t vh_skip_len; /* Length of this vendor section */
+} __packed;
+
/*
* The radio capture header precedes the 802.11 header.
*
Index: sys/dev/ath/ath_hal/ar5416/ar5416_misc.c
===================================================================
--- sys/dev/ath/ath_hal/ar5416/ar5416_misc.c (revision 230855)
+++ sys/dev/ath/ath_hal/ar5416/ar5416_misc.c (working copy)
@@ -714,13 +714,11 @@
pe->pe_relpwr = MS(val, AR_PHY_RADAR_1_RELPWR_THRESH);
if (temp)
pe->pe_relpwr |= HAL_PHYERR_PARAM_ENABLE;
- temp = val & AR_PHY_RADAR_1_RELSTEP_CHECK;
pe->pe_relstep = MS(val, AR_PHY_RADAR_1_RELSTEP_THRESH);
- if (temp)
- pe->pe_enabled = 1;
- else
- pe->pe_enabled = 0;
+ pe->pe_enabled = !!
+ (OS_REG_READ(ah, AR_PHY_RADAR_0) & AR_PHY_RADAR_0_ENA);
+
pe->pe_maxlen = MS(val, AR_PHY_RADAR_1_MAXLEN);
pe->pe_extchannel = !! (OS_REG_READ(ah, AR_PHY_RADAR_EXT) &
AR_PHY_RADAR_EXT_ENA);
@@ -767,9 +765,15 @@
/*Enable FFT data*/
val |= AR_PHY_RADAR_0_FFT_ENA;
+ OS_REG_WRITE(ah, AR_PHY_RADAR_0, val);
- OS_REG_WRITE(ah, AR_PHY_RADAR_0, val | AR_PHY_RADAR_0_ENA);
+ /* Implicitly enable */
+ if (pe->pe_enabled == 1)
+ OS_REG_SET_BIT(ah, AR_PHY_RADAR_0, AR_PHY_RADAR_0_ENA);
+ else if (pe->pe_enabled == 0)
+ OS_REG_CLR_BIT(ah, AR_PHY_RADAR_0, 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: sys/dev/ath/if_athioctl.h
===================================================================
--- sys/dev/ath/if_athioctl.h (revision 230855)
+++ sys/dev/ath/if_athioctl.h (working copy)
@@ -188,10 +188,70 @@
(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 higher than the vendor bitmap used inside
+ * the Atheros reference codebase.
+ */
+
+/* Bit 8 */
+#define ATH_RADIOTAP_VENDOR_HEADER 8
+
+
+/*
+ * Using four chains makes all the fields in the
+ * per-chain info header be 4-byte aligned.
+ */
+#define ATH_RADIOTAP_MAX_CHAINS 4
+
+/*
+ * The vendor radiotap header data needs to be:
+ *
+ * + Aligned to a 4 byte address
+ * + .. so all internal fields are 4 bytes aligned;
+ * + .. and no 64 bit fields are allowed.
+ *
+ * So padding is required to ensure this is the case.
+ *
+ * Note that because of the lack of alignment with the
+ * vendor header (6 bytes), the first field must be
+ * two bytes so it can be accessed by alignment-strict
+ * platform (eg MIPS.)
+ */
+struct ath_radiotap_vendor_hdr { /* 30 bytes */
+ u_int8_t vh_version; /* 1 */
+ u_int8_t vh_rx_chainmask; /* 1 */
+
+ /* At this point it should be 4 byte aligned */
+ u_int32_t evm[ATH_RADIOTAP_MAX_CHAINS]; /* 4 * 4 = 16 */
+
+ u_int8_t rssi_ctl[ATH_RADIOTAP_MAX_CHAINS]; /* 4 */
+ u_int8_t rssi_ext[ATH_RADIOTAP_MAX_CHAINS]; /* 4 */
+
+ u_int8_t vh_phyerr_code; /* Phy error code, or 0xff */
+ u_int8_t vh_rs_status; /* RX status */
+ u_int8_t vh_rssi; /* Raw RSSI */
+ u_int8_t vh_pad1[1]; /* Pad to 4 byte boundary */
+} __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 +263,24 @@
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.
+ *
+ * XXX This must be aligned to a 4 byte address?
+ * XXX or 8 byte address?
+ */
+ struct ieee80211_radiotap_vendor_header wr_vh; /* 6 bytes */
+
+ /*
+ * Because of the lack of alignment enforced by the above
+ * header, this vendor section won't be aligned in any
+ * useful way. So, this will include a two-byte version
+ * value which will force the structure to be 4-byte aligned.
+ */
+ struct ath_radiotap_vendor_hdr wr_v;
} __packed;
#define ATH_TX_RADIOTAP_PRESENT ( \
Index: sys/dev/ath/ath_dfs/null/dfs_null.c
===================================================================
--- sys/dev/ath/ath_dfs/null/dfs_null.c (revision 231099)
+++ sys/dev/ath/ath_dfs/null/dfs_null.c (working copy)
@@ -120,7 +120,7 @@
int
ath_dfs_radar_enable(struct ath_softc *sc, struct ieee80211_channel *chan)
{
-#if 0
+#if 1
HAL_PHYERR_PARAM pe;
/* Check if the current channel is radar-enabled */
Index: sys/dev/ath/if_ath.c
===================================================================
--- sys/dev/ath/if_ath.c (revision 230855)
+++ sys/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,53 @@
}
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.vh_oui[0] = 0x7f;
+ sc->sc_rx_th.wr_vh.vh_oui[1] = 0x03;
+ sc->sc_rx_th.wr_vh.vh_oui[2] = 0x00;
+
+ /* XXX what should this be? */
+ sc->sc_rx_th.wr_vh.vh_sub_ns = 0;
+ sc->sc_rx_th.wr_vh.vh_skip_len =
+ htole16(sizeof(struct ath_radiotap_vendor_hdr));
+
+ /* General version info */
+ sc->sc_rx_th.wr_v.vh_version = 1;
+
+ sc->sc_rx_th.wr_v.vh_rx_chainmask = sc->sc_rxchainmask;
+
+ /* rssi */
+ sc->sc_rx_th.wr_v.rssi_ctl[0] = rs->rs_rssi_ctl[0];
+ sc->sc_rx_th.wr_v.rssi_ctl[1] = rs->rs_rssi_ctl[1];
+ sc->sc_rx_th.wr_v.rssi_ctl[2] = rs->rs_rssi_ctl[2];
+ sc->sc_rx_th.wr_v.rssi_ext[0] = rs->rs_rssi_ext[0];
+ sc->sc_rx_th.wr_v.rssi_ext[1] = rs->rs_rssi_ext[1];
+ sc->sc_rx_th.wr_v.rssi_ext[2] = rs->rs_rssi_ext[2];
+
+ /* evm */
+ sc->sc_rx_th.wr_v.evm[0] = rs->rs_evm0;
+ sc->sc_rx_th.wr_v.evm[1] = rs->rs_evm1;
+ sc->sc_rx_th.wr_v.evm[2] = rs->rs_evm2;
+ /* XXX TODO: extend this to include 3-stream EVM */
+
+ /* phyerr info */
+ 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 = 0xff;
+ sc->sc_rx_th.wr_v.vh_rs_status = rs->rs_status;
+ sc->sc_rx_th.wr_v.vh_rssi = rs->rs_rssi;
+}
+
+static void
ath_rx_tap(struct ifnet *ifp, struct mbuf *m,
const struct ath_rx_status *rs, u_int64_t tsf, int16_t nf)
{
@@ -3915,7 +3966,7 @@
sc->sc_rx_th.wr_flags |= IEEE80211_RADIOTAP_F_SHORTGI;
}
#endif
- sc->sc_rx_th.wr_tsf = htole64(ath_extend_tsf(sc, rs->rs_tstamp, tsf));
+ sc->sc_rx_th.wr_tsf = tsf;
if (rs->rs_status & HAL_RXERR_CRC)
sc->sc_rx_th.wr_flags |= IEEE80211_RADIOTAP_F_BADFCS;
/* XXX propagate other error flags from descriptor */
@@ -4156,6 +4207,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);
}
@@ -4251,8 +4303,10 @@
* material required by ieee80211_input. Note that
* noise setting is filled in above.
*/
- if (ieee80211_radiotap_active(ic))
+ if (ieee80211_radiotap_active(ic)) {
ath_rx_tap(ifp, m, rs, rstamp, nf);
+ ath_rx_tap_vendor(ifp, m, rs, rstamp, nf);
+ }
/*
* From this point on we assume the frame is at least
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-Vmonmk=0maQcJd8iZscZvRxi%2BZcZiwWQyZ1r1Ju4QyhgKXw>