Skip to contentSkip to conversation

Try #3 Radiotap on Monitor Mode interfaces for rx and tx

PATCH In review [email protected] 3 participants 7 messages Mar 18, 2007 – Mar 21, 2007 mac80211

lore
Share & tools
lore
#0 lore raw

cover Try #3 Radiotap on Monitor Mode interfaces for rx and tx

Hi folks -

This pair of patches change the Monitor Mode wireless interfaces to use radiotap both for monitoring and for packet injection. The monitoring side is done by a patch from Michael Wu. Tcpdump knows how to handle the result.

For injecting packets, the you issue a packet using libpcap or a SOCK_PACKET socket down an interface to the wireless device that is in Monitor Mode. The packet has a normal radiotap header prepended to the IEEE80211 header. The radiotap header is variable length depending on what the user wants to specify, currently the transmit rate, power and antenna can be specified using normal radiotap semantics. Any other entries are skipped.

A usermode app packetspammer is available from here

http://penumbra.warmcat.com/_twk/tiki-index.php?page=packetspammer

which allows easy injection of these packets from the commandline. At the moment it loops issuing packets at a variety of rates which can be seen from another machine's monitor mode interface on the same channel. There are instructions for build and using it on the page above.

Currently it has been tested for both rx and tx using zd1211rw-mac80211 and works except for 1Mbps TX (54Mbps - 2Mbps works) and the rx radiotap rate is not shown in tcpdump.

The patches should be based against wireless-dev.

#1 lore raw

PATCH 1/2 mac80211: Add radiotap support

From: Michael Wu <[email protected]>

---

 include/net/mac80211.h         |    3 ++
 net/mac80211/ieee80211.c       |   69 +++++++++++++++++++++++++++++++++-------
 net/mac80211/ieee80211_iface.c |    2 +
 3 files changed, 61 insertions(+), 13 deletions(-)
include/net/mac80211.h +3 -0
index 916b21b..050f126 100644
@@ -529,6 +529,9 @@ struct ieee80211_hw {
 	 * per-packet RC4 key with each TX frame when doing hwcrypto */
 #define IEEE80211_HW_TKIP_REQ_PHASE2_KEY (1<<14)
 
+	/* Driver supports radiotap. Temporary until all drivers support it. */
+#define IEEE80211_HW_RADIOTAP_SUPPORTED (1<<20)
+
 	u32 flags;			/* hardware flags defined above */
 
 	/* Set to the size of a needed device specific skb headroom for TX skbs. */
net/mac80211/ieee80211.c +57 -12

120 lines not on this page · raw message

net/mac80211/ieee80211_iface.c +1 -1
index 3e0b4fa..51197b1 100644
@@ -199,7 +199,7 @@ void ieee80211_if_set_type(struct net_device *dev, int type)
 		break;
 	}
 	case IEEE80211_IF_TYPE_MNTR:
-		dev->type = ARPHRD_IEEE80211_PRISM;
+		dev->type = ARPHRD_IEEE80211_RADIOTAP;
 		break;
 	default:
 		printk(KERN_WARNING "%s: %s: Unknown interface type 0x%x",
#2 lore raw

PATCH 2/2 mac80211: Monitor mode radiotap-based packet injection

In reply to [email protected]

From: Andy Green <[email protected]>

Try #3
- moved to Michael Wu's method of tracking if we came in on a monitor interface by using ifindex
- removed older proposed monitor interface tracking method and flags
- style fixes
- removed duped #include that is present in Michael Wu's patch already

Try #2
- took Michael Wu's advice about better tools and basing on wireless-dev
- took Luis Rodriguez's advice about coding style makeover
- took Pavel Roskin's advice about little-endian radiotap

Index: linux-2.6.20.i386/include/net/mac80211.h
===================================================

net/mac80211/ieee80211.c +248 -10
index fb33b90..21e8990 100644
@@ -1054,7 +1054,163 @@ ieee80211_tx_h_ps_buf(struct ieee80211_txrx_data *tx)
 }
 
 
-static void inline
+/* deal with packet injection down monitor interface
+ * with Radiotap Header -- only called for monitor mode interface
+ */
+
+static ieee80211_txrx_result
+__ieee80211_convert_radiotap_to_control_and_remove(
+	struct ieee80211_txrx_data *tx,
+	struct sk_buff *skb, struct ieee80211_tx_control *control)
+{
+	/* this is the moment to interpret the radiotap header that
+	 * must be at the start of the packet injected in Monitor
+	 * mode into control and then discard the radiotap header
+	 *
+	 * Need to take some care with endian-ness since radiotap
+	 * is apparently a little-endian struct, including the args
+	*/
+
+	struct ieee80211_radiotap_header *rthdr =
+		(struct ieee80211_radiotap_header *) skb->data;
+
+	/* small length lookup table for all radiotap types we heard of
+	 * starting from b0 in the bitmap, so we can walk the payload
+	 * area of the radiotap header
+	 */
+
+	static const u8 radiotap_entry_sizes[] = {
+		8, /* IEEE80211_RADIOTAP_TSFT */
+		1, /* IEEE80211_RADIOTAP_FLAGS */
+		1, /* IEEE80211_RADIOTAP_RATE */
+		4, /* IEEE80211_RADIOTAP_CHANNEL */
+		2, /* IEEE80211_RADIOTAP_FHSS */
+		1, /* IEEE80211_RADIOTAP_DBM_ANTSIGNAL */
+		1, /* IEEE80211_RADIOTAP_DBM_ANTNOISE */
+		2, /* IEEE80211_RADIOTAP_LOCK_QUALITY */
+		2, /* IEEE80211_RADIOTAP_TX_ATTENUATION */
+		2, /* IEEE80211_RADIOTAP_DB_TX_ATTENUATION */
+		1, /* IEEE80211_RADIOTAP_DBM_TX_POWER */
+		1, /* IEEE80211_RADIOTAP_ANTENNA */
+		1, /* IEEE80211_RADIOTAP_DB_ANTSIGNAL */
+		1  /* IEEE80211_RADIOTAP_DB_ANTNOISE */
+		/* add more here as they are defined */
+	};
+	int tap_index = 0;
+	u8 *tap_arg = skb->data + sizeof(struct ieee80211_radiotap_header);
+	u32 *curr_arg_bitmap = &rthdr->it_present;
+	u32 arg_bitmap = le32_to_cpu(*curr_arg_bitmap);
+
+	if (rthdr->it_version) return TXRX_DROP; /* version byte as magic */
+
+	/* sanity check for skb length and radiotap length field */
+	if (skb->len < (le16_to_cpu(rthdr->it_len) +
+	    sizeof(struct ieee80211_hdr)))
+		return TXRX_DROP;
+
+	/* find payload start allowing for extended bitmap(s) */
+
+	if (le32_to_cpu(rthdr->it_present) & 0x80000000) {
+		while( le32_to_cpu(*((u32 *)tap_arg)) & 0x80000000)
+			tap_arg += sizeof(u32);
+		tap_arg += sizeof(u32);
+	}
+
+	/* default control situation for all injected packets */
+
+	control->retry_limit = 1; /* no retry */
+	control->key_idx = -1; /* no encryption key */
+	control->flags &= ~(IEEE80211_TXCTL_USE_RTS_CTS |
+		IEEE80211_TXCTL_USE_CTS_PROTECT);
+	control->flags |= (IEEE80211_TXCTL_DO_NOT_ENCRYPT |
+		IEEE80211_TXCTL_NO_ACK);
+	control->antenna_sel_tx = 0; /* default to default/diversity */
+
+	/* for every radiotap entry we can at
+	 * least skip (by knowing the length)...
+	 */
+
+	while (tap_index < sizeof(radiotap_entry_sizes)) {
+		int i, target_rate;
+
+		if (!(arg_bitmap & 1)) goto next_entry;
+
+		/* arg is present */
+
+		switch(tap_index) { /* deal with if interested */
+
+		case IEEE80211_RADIOTAP_RATE:
+			/* radiotap "rate" u8 is in
+			 * 500kbps units, eg, 0x02=1Mbps
+			 * ieee80211 "rate" int is
+			 * in 100kbps units, eg, 0x0a=1Mbps
+			 */
+			target_rate = (*tap_arg) * 5;
+			for (i = 0; i < tx->local->num_curr_rates; i++) {
+				struct ieee80211_rate *r =
+					&tx->local->curr_rates[i];
+
+				if (r->rate <= target_rate) {
+					if (r->flags &
+					   IEEE80211_RATE_PREAMBLE2) {
+						control->tx_rate = r->val2;
+					} else {
+						control->tx_rate = r->val;
+					}
+				
+
+						/* end on exact match */
+					if (r->rate == target_rate)
+						i = tx->local->num_curr_rates;
+				}
+			}
+			break;
+
+		case IEEE80211_RADIOTAP_ANTENNA:
+			/* radiotap uses 0 for 1st ant,
+			 * mac80211 is 1 for 1st ant
+			 * absence of IEEE80211_RADIOTAP_ANTENNA
+			 * gives default/diversity
+			*/
+			control->antenna_sel_tx = (*tap_arg) + 1;
+			break;
+
+		case IEEE80211_RADIOTAP_DBM_TX_POWER:
+			control->power_level = *tap_arg;
+			break;
+
+		default:
+			break;
+		}
+
+		tap_arg += radiotap_entry_sizes[tap_index];
+
+		next_entry:
+
+		tap_index++;
+		if (unlikely((tap_index & 31) == 0)) {
+			/* completed current u32 bitmap */
+			if (arg_bitmap & 1) { /* b31 was set, there is more */
+				/* move to next u32 bitmap */
+				curr_arg_bitmap++;
+				arg_bitmap = le32_to_cpu(*curr_arg_bitmap);
+			} else {
+				/* he didn't give us any more bitmaps: end */
+				tap_index = sizeof(radiotap_entry_sizes);
+			}
+		} else { /* just try the next bit */
+			arg_bitmap >>= 1;
+		}
+	}
+
+		/* remove the radiotap header */
+	skb_pull(skb, le16_to_cpu(rthdr->it_len));
+
+	return TXRX_CONTINUE;
+}
+
+
+static ieee80211_txrx_result inline
 __ieee80211_tx_prepare(struct ieee80211_txrx_data *tx,
 		       struct sk_buff *skb,
 		       struct net_device *dev,
@@ -1062,6 +1218,9 @@ __ieee80211_tx_prepare(struct ieee80211_txrx_data *tx,
 {
 	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+	struct ieee80211_sub_if_data *sdata;
+	ieee80211_txrx_result res=TXRX_CONTINUE;
+
 	int hdrlen;
 
 	memset(tx, 0, sizeof(*tx));
@@ -1071,7 +1230,32 @@ __ieee80211_tx_prepare(struct ieee80211_txrx_data *tx,
 	tx->sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	tx->sta = sta_info_get(local, hdr->addr1);
 	tx->fc = le16_to_cpu(hdr->frame_control);
+
+	/* set defaults for things that can be set by
+	 * injected radiotap headers
+	 */
+
 	control->power_level = local->hw.conf.power_level;
+	control->antenna_sel_tx = local->hw.conf.antenna_sel_tx;
+	if (local->sta_antenna_sel != STA_ANTENNA_SEL_AUTO && tx->sta )
+		control->antenna_sel_tx = tx->sta->antenna_sel_tx;
+
+	/* process and remove the injection radiotap header */
+
+	sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	if (unlikely(sdata->type==IEEE80211_IF_TYPE_MNTR)) {
+		if (__ieee80211_convert_radiotap_to_control_and_remove(
+		    tx, skb, control) == TXRX_DROP) {
+			return TXRX_DROP;
+		}
+		/* we removed the radiotap header after this point,
+		 * we filled control with what we could use
+		 * set to the actual ieee header now
+		 */
+		hdr = (struct ieee80211_hdr *) skb->data;
+		res=TXRX_QUEUED; /* indication it was monitor packet */
+	}
+
 	tx->u.tx.control = control;
 	tx->u.tx.unicast = !is_multicast_ether_addr(hdr->addr1);
 	if (is_multicast_ether_addr(hdr->addr1))
@@ -1088,9 +1272,6 @@ __ieee80211_tx_prepare(struct ieee80211_txrx_data *tx,
 		control->flags |= IEEE80211_TXCTL_CLEAR_DST_MASK;
 		tx->sta->clear_dst_mask = 0;
 	}
-	control->antenna_sel_tx = local->hw.conf.antenna_sel_tx;
-	if (local->sta_antenna_sel != STA_ANTENNA_SEL_AUTO && tx->sta)
-		control->antenna_sel_tx = tx->sta->antenna_sel_tx;
 	hdrlen = ieee80211_get_hdrlen(tx->fc);
 	if (skb->len > hdrlen + sizeof(rfc1042_header) + 2) {
 		u8 *pos = &skb->data[hdrlen + sizeof(rfc1042_header)];
@@ -1098,6 +1279,7 @@ __ieee80211_tx_prepare(struct ieee80211_txrx_data *tx,
 	}
 	control->flags |= IEEE80211_TXCTL_FIRST_FRAGMENT;
 
+	return res;
 }
 
 static int inline is_ieee80211_device(struct net_device *dev,
@@ -1205,7 +1387,7 @@ static int ieee80211_tx(struct net_device *dev, struct sk_buff *skb,
 	struct sta_info *sta;
 	ieee80211_tx_handler *handler;
 	struct ieee80211_txrx_data tx;
-	ieee80211_txrx_result res = TXRX_DROP;
+	ieee80211_txrx_result res = TXRX_DROP, res_prepare;
 	int ret, i;
 
 	WARN_ON(__ieee80211_queue_pending(local, control->queue));
@@ -1215,14 +1397,24 @@ static int ieee80211_tx(struct net_device *dev, struct sk_buff *skb,
 		return 0;
 	}
 
-	__ieee80211_tx_prepare(&tx, skb, dev, control);
+	res_prepare=__ieee80211_tx_prepare(&tx, skb, dev, control);
+
+	if (res_prepare==TXRX_DROP) {
+		dev_kfree_skb(skb);
+		return 0;
+	}
+
 	sta = tx.sta;
 	tx.u.tx.mgmt_interface = mgmt;
 
-	for (handler = local->tx_handlers; *handler != NULL; handler++) {
-		res = (*handler)(&tx);
-		if (res != TXRX_CONTINUE)
-			break;
+	if (res_prepare==TXRX_QUEUED) { /* if it was an injected packet */
+		res=TXRX_CONTINUE;
+	} else {
+		for (handler = local->tx_handlers; *handler != NULL; handler++) {
+			res = (*handler)(&tx);
+			if (res != TXRX_CONTINUE)
+				break;
+		}
 	}
 
 	skb = tx.skb; /* handlers are allowed to change skb */
@@ -1456,6 +1648,52 @@ static int ieee80211_subif_start_xmit(struct sk_buff *skb,
 		goto fail;
 	}
 
+	if (unlikely(sdata->type==IEEE80211_IF_TYPE_MNTR)) {
+		struct ieee80211_radiotap_header * prthdr=
+			(struct ieee80211_radiotap_header *)skb->data;
+
+			/* there must be a radiotap header at the
+			 * start in this case
+			 */
+
+		if (unlikely(prthdr->it_version)) {
+			/* radiotap version used as magic */
+			ret=0;
+			goto fail;
+		}
+
+		skb->dev = local->mdev;
+
+		pkt_data = (struct ieee80211_tx_packet_data *)skb->cb;
+		memset(pkt_data, 0, sizeof(struct ieee80211_tx_packet_data));
+		pkt_data->ifindex = sdata->dev->ifindex;
+			/*local->mdev->ifindex*/
+		pkt_data->mgmt_iface = 0;
+		pkt_data->do_not_encrypt = 1;
+
+			/* above needed because we set skb device to master */
+
+			/* fix up the pointers accounting for the radiotap
+			 * header still being in there.  We are being given
+			 * a precooked IEEE80211 header so no need for
+			 * normal processing
+			 */
+
+		skb->mac.raw = skb->data+prthdr->it_len;
+		skb->nh.raw = skb->data+prthdr->it_len+
+			sizeof(struct ieee80211_hdr);
+		skb->h.raw = skb->data+prthdr->it_len+
+			sizeof(struct ieee80211_hdr);
+
+			/* pass the radiotap header up to
+			 * the next stage intact
+			 */
+
+		dev_queue_xmit(skb);
+
+		return 0;
+	}
+
 	nh_pos = skb->nh.raw - skb->data;
 	h_pos = skb->h.raw - skb->data;
 
#3 lore raw

Re: [PATCH 2/2] mac80211: Monitor mode radiotap-based packet injection

Quoted 9 lines · On Sunday 18 March 2007 06:15, [email protected] wrote:

From: Andy Green <[email protected]>

Try #3
- moved to Michael Wu's method of tracking if we came in on a monitor interface by using ifindex
- removed older proposed monitor interface tracking method and flags
- style fixes
- removed duped #include that is present in Michael Wu's patch already

Quoted 5 lines

Try #2
- took Michael Wu's advice about better tools and basing on wireless-dev
- took Luis Rodriguez's advice about coding style makeover
- took Pavel Roskin's advice about little-endian radiotap

I've mostly made comments about style issues. There are only comments on the first instance of any style problem so please check the rest of the code for the same problems.

Quoted 18 lines

Index: linux-2.6.20.i386/include/net/mac80211.h
===================================================

net/mac80211/ieee80211.c +6 -1
index fb33b90..21e8990 100644
@@ -1054,7 +1054,163 @@ ieee80211_tx_h_ps_buf(struct ieee80211_txrx_data
 *tx) }
 
 
-static void inline
+/* deal with packet injection down monitor interface
+ * with Radiotap Header -- only called for monitor mode interface
+ */
+
+static ieee80211_txrx_result
+__ieee80211_convert_radiotap_to_control_and_remove(

net/mac80211/ieee80211.c ieee80211_tx_h_ps_buf(struct ieee80211_txrx_data · line 1072

Quoted 10 lines
net/mac80211/ieee80211.c +10 -0
+	struct ieee80211_txrx_data *tx,
+	struct sk_buff *skb, struct ieee80211_tx_control *control)
+{
+	/* this is the moment to interpret the radiotap header that
+	 * must be at the start of the packet injected in Monitor
+	 * mode into control and then discard the radiotap header
+	 *
+	 * Need to take some care with endian-ness since radiotap
+	 * is apparently a little-endian struct, including the args
+	*/

Line this up. :)

Quoted 16 lines
net/mac80211/ieee80211.c +16 -0
+
+	struct ieee80211_radiotap_header *rthdr =
+		(struct ieee80211_radiotap_header *) skb->data;
+
+	/* small length lookup table for all radiotap types we heard of
+	 * starting from b0 in the bitmap, so we can walk the payload
+	 * area of the radiotap header
+	 */
+
+	static const u8 radiotap_entry_sizes[] = {
+		8, /* IEEE80211_RADIOTAP_TSFT */
+		1, /* IEEE80211_RADIOTAP_FLAGS */
+		1, /* IEEE80211_RADIOTAP_RATE */
+		4, /* IEEE80211_RADIOTAP_CHANNEL */
+		2, /* IEEE80211_RADIOTAP_FHSS */
+		1, /* IEEE80211_RADIOTAP_DBM_ANTSIGNAL */

net/mac80211/ieee80211.c ieee80211_tx_h_ps_buf(struct ieee80211_txrx_data *tx) · line 1098

Quoted 10 lines
net/mac80211/ieee80211.c +10 -0
+		1, /* IEEE80211_RADIOTAP_DBM_ANTNOISE */
+		2, /* IEEE80211_RADIOTAP_LOCK_QUALITY */
+		2, /* IEEE80211_RADIOTAP_TX_ATTENUATION */
+		2, /* IEEE80211_RADIOTAP_DB_TX_ATTENUATION */
+		1, /* IEEE80211_RADIOTAP_DBM_TX_POWER */
+		1, /* IEEE80211_RADIOTAP_ANTENNA */
+		1, /* IEEE80211_RADIOTAP_DB_ANTSIGNAL */
+		1  /* IEEE80211_RADIOTAP_DB_ANTNOISE */
+		/* add more here as they are defined */
+	};

Hm, this could be integrated into the switch statement below, but it doesn't really matter much.

Have you looked into padding issues with radiotap headers? For example, if there is a 1 byte field which is then followed by a 4 byte field, there needs to be 3 bytes of padding after the first field, but if the field after were 2 bytes long, the padding would only be 1 byte (according to my understanding of the radiotap specs).

net/mac80211/ieee80211.c ieee80211_tx_h_ps_buf(struct ieee80211_txrx_data *tx) · line 1104

Quoted 6 lines
net/mac80211/ieee80211.c +6 -0
+	int tap_index = 0;
+	u8 *tap_arg = skb->data + sizeof(struct ieee80211_radiotap_header);
+	u32 *curr_arg_bitmap = &rthdr->it_present;
+	u32 arg_bitmap = le32_to_cpu(*curr_arg_bitmap);
+
+	if (rthdr->it_version) return TXRX_DROP; /* version byte as magic */

Put the return on the next line.

net/mac80211/ieee80211.c ieee80211_tx_h_ps_buf(struct ieee80211_txrx_data *tx) · line 1114

Quoted 10 lines
net/mac80211/ieee80211.c +10 -0
+
+	/* sanity check for skb length and radiotap length field */
+	if (skb->len < (le16_to_cpu(rthdr->it_len) +
+	    sizeof(struct ieee80211_hdr)))
+		return TXRX_DROP;
+
+	/* find payload start allowing for extended bitmap(s) */
+
+	if (le32_to_cpu(rthdr->it_present) & 0x80000000) {
+		while( le32_to_cpu(*((u32 *)tap_arg)) & 0x80000000)

Space after while, remove the space before le32_to_cpu.

Quoted 16 lines
net/mac80211/ieee80211.c +16 -0
+			tap_arg += sizeof(u32);
+		tap_arg += sizeof(u32);
+	}
+
+	/* default control situation for all injected packets */
+
+	control->retry_limit = 1; /* no retry */
+	control->key_idx = -1; /* no encryption key */
+	control->flags &= ~(IEEE80211_TXCTL_USE_RTS_CTS |
+		IEEE80211_TXCTL_USE_CTS_PROTECT);
+	control->flags |= (IEEE80211_TXCTL_DO_NOT_ENCRYPT |
+		IEEE80211_TXCTL_NO_ACK);
+	control->antenna_sel_tx = 0; /* default to default/diversity */
+
+	/* for every radiotap entry we can at
+	 * least skip (by knowing the length)...

net/mac80211/ieee80211.c ieee80211_tx_h_ps_buf(struct ieee80211_txrx_data *tx) · line 1140

Quoted 10 lines
net/mac80211/ieee80211.c +10 -0
+	 */
+
+	while (tap_index < sizeof(radiotap_entry_sizes)) {
+		int i, target_rate;
+
+		if (!(arg_bitmap & 1)) goto next_entry;
+
+		/* arg is present */
+
+		switch(tap_index) { /* deal with if interested */

Space after switch.

Quoted 12 lines
net/mac80211/ieee80211.c +12 -0
+
+		case IEEE80211_RADIOTAP_RATE:
+			/* radiotap "rate" u8 is in
+			 * 500kbps units, eg, 0x02=1Mbps
+			 * ieee80211 "rate" int is
+			 * in 100kbps units, eg, 0x0a=1Mbps
+			 */
+			target_rate = (*tap_arg) * 5;
+			for (i = 0; i < tx->local->num_curr_rates; i++) {
+				struct ieee80211_rate *r =
+					&tx->local->curr_rates[i];
+

net/mac80211/ieee80211.c ieee80211_tx_h_ps_buf(struct ieee80211_txrx_data *tx) · line 1162

Quoted 10 lines
net/mac80211/ieee80211.c +10 -0
+				if (r->rate <= target_rate) {
+					if (r->flags &
+					   IEEE80211_RATE_PREAMBLE2) {
+						control->tx_rate = r->val2;
+					} else {
+						control->tx_rate = r->val;
+					}
+
+
+						/* end on exact match */

Too much indenting on comments again.

Quoted 16 lines
net/mac80211/ieee80211.c +16 -0
+					if (r->rate == target_rate)
+						i = tx->local->num_curr_rates;
+				}
+			}
+			break;
+
+		case IEEE80211_RADIOTAP_ANTENNA:
+			/* radiotap uses 0 for 1st ant,
+			 * mac80211 is 1 for 1st ant
+			 * absence of IEEE80211_RADIOTAP_ANTENNA
+			 * gives default/diversity
+			*/
+			control->antenna_sel_tx = (*tap_arg) + 1;
+			break;
+
+		case IEEE80211_RADIOTAP_DBM_TX_POWER:

net/mac80211/ieee80211.c ieee80211_tx_h_ps_buf(struct ieee80211_txrx_data *tx) · line 1188

Quoted 10 lines
net/mac80211/ieee80211.c +10 -0
+			control->power_level = *tap_arg;
+			break;
+
+		default:
+			break;
+		}
+
+		tap_arg += radiotap_entry_sizes[tap_index];
+
+		next_entry:

This label is indented too much.

net/mac80211/ieee80211.c ieee80211_tx_h_ps_buf(struct ieee80211_txrx_data *tx) · line 1198

Quoted 10 lines
net/mac80211/ieee80211.c +10 -0
+
+		tap_index++;
+		if (unlikely((tap_index & 31) == 0)) {
+			/* completed current u32 bitmap */
+			if (arg_bitmap & 1) { /* b31 was set, there is more */
+				/* move to next u32 bitmap */
+				curr_arg_bitmap++;
+				arg_bitmap = le32_to_cpu(*curr_arg_bitmap);
+			} else {
+				/* he didn't give us any more bitmaps: end */

I suppose this is an accurate assumption 99% of the time, but I think being gender neutral sounds better.

net/mac80211/ieee80211.c __ieee80211_tx_prepare(struct ieee80211_txrx_data · line 1222

Quoted 6 lines
net/mac80211/ieee80211.c +2 -0
@@ -1062,6 +1218,9 @@ __ieee80211_tx_prepare(struct ieee80211_txrx_data
 *tx, {
 	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+	struct ieee80211_sub_if_data *sdata;
+	ieee80211_txrx_result res=TXRX_CONTINUE;

Space before and after the =.

net/mac80211/ieee80211.c __ieee80211_tx_prepare(struct ieee80211_txrx_data · line 1240

Quoted 12 lines
net/mac80211/ieee80211.c +7 -0
@@ -1071,7 +1230,32 @@ __ieee80211_tx_prepare(struct ieee80211_txrx_data
 *tx, tx->sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	tx->sta = sta_info_get(local, hdr->addr1);
 	tx->fc = le16_to_cpu(hdr->frame_control);
+
+	/* set defaults for things that can be set by
+	 * injected radiotap headers
+	 */
+
 	control->power_level = local->hw.conf.power_level;
+	control->antenna_sel_tx = local->hw.conf.antenna_sel_tx;
+	if (local->sta_antenna_sel != STA_ANTENNA_SEL_AUTO && tx->sta )

Kill the space before the parenthesis.

net/mac80211/ieee80211.c static int ieee80211_tx(struct net_device *dev, · line 1402

Quoted 8 lines
net/mac80211/ieee80211.c +3 -1
@@ -1215,14 +1397,24 @@ static int ieee80211_tx(struct net_device *dev,
 struct sk_buff *skb, return 0;
 	}
 
-	__ieee80211_tx_prepare(&tx, skb, dev, control);
+	res_prepare=__ieee80211_tx_prepare(&tx, skb, dev, control);
+
+	if (res_prepare==TXRX_DROP) {

Space before and after the ==.

Quoted 23 lines
net/mac80211/ieee80211.c +12 -4
+		dev_kfree_skb(skb);
+		return 0;
+	}
+
 	sta = tx.sta;
 	tx.u.tx.mgmt_interface = mgmt;
 
-	for (handler = local->tx_handlers; *handler != NULL; handler++) {
-		res = (*handler)(&tx);
-		if (res != TXRX_CONTINUE)
-			break;
+	if (res_prepare==TXRX_QUEUED) { /* if it was an injected packet */
+		res=TXRX_CONTINUE;
+	} else {
+		for (handler = local->tx_handlers; *handler != NULL; handler++) {
+			res = (*handler)(&tx);
+			if (res != TXRX_CONTINUE)
+				break;
+		}
 	}
 
 	skb = tx.skb; /* handlers are allowed to change skb */

net/mac80211/ieee80211.c static int ieee80211_subif_start_xmit(struct sk_buff · line 1652

Quoted 5 lines
net/mac80211/ieee80211.c +2 -0
@@ -1456,6 +1648,52 @@ static int ieee80211_subif_start_xmit(struct sk_buff
 *skb, goto fail;
 	}
 
+	if (unlikely(sdata->type==IEEE80211_IF_TYPE_MNTR)) {
+		struct ieee80211_radiotap_header * prthdr=

Space after prthdr.

We could avoid this check altogether by spinning this code into a different function and setting the xmit handler appropriately depending on if we're initializing/switching to a monitor interface or not. Not entirely sure if it's worth it, but I thought I'd mention it.

Thanks,
-Michael Wu

author
#4 lore raw

Re: [PATCH 2/2] mac80211: Monitor mode radiotap-based packet injection

Quoted 1 line · Michael Wu

I've mostly made comments about style issues. There are only comments on the first instance of any style problem so please check the rest of the code for the same problems.

Thanks for this feedback Michael. I have changed all the style problems my eyes could see, assisted by visiting every = in the patch.

Quoted 1 line

Hm, this could be integrated into the switch statement below, but it doesn't really matter much.

It's also very handy for...

Quoted 1 line

Have you looked into padding issues with radiotap headers? For example, if there is a 1 byte field which is then followed by a 4 byte field, there needs to be 3 bytes of padding after the first field, but if the field after were 2 bytes long, the padding would only be 1 byte (according to my understanding of the radiotap specs).

I googled for radiotap specs but I didn't find anything useful. I added some small code to enforce the alignment rules you mention above.

I found there was no docs in ./Documentation about 80211, I added a small explanation and examples about injection including this alignment Gotcha so the knowledge isn't lost.

Quoted 1 line

We could avoid this check altogether by spinning this code into a different function and setting the xmit handler appropriately depending on if we're initializing/switching to a monitor interface or not. Not entirely sure if it's worth it, but I thought I'd mention it.

Yes, that method would be marginally better, but there is only a single int getting tested in the main path and even that is marked up as unlikely(). So I leave it as it is for now.

-Andy

#5 lore raw

Re: [PATCH 2/2] mac80211: Monitor mode radiotap-based packet injection

Quoted 8 lines · Andy Green
Quoted 3 lines · Michael Wu

I've mostly made comments about style issues. There are only comments on the first instance of any style problem so please check the rest of the code for the same problems.

Thanks for this feedback Michael. I have changed all the style problems my eyes could see, assisted by visiting every = in the patch.

Just one last thing.. a bunch of the comments in the last chunk of the patch are still indented too much. Otherwise, the 4th version of the patch looks good as far as style goes.

Quoted 9 lines
Quoted 5 lines

Have you looked into padding issues with radiotap headers? For example, if there is a 1 byte field which is then followed by a 4 byte field, there needs to be 3 bytes of padding after the first field, but if the field after were 2 bytes long, the padding would only be 1 byte (according to my understanding of the radiotap specs).

I googled for radiotap specs but I didn't find anything useful. I added some small code to enforce the alignment rules you mention above.

I got the information on padding from:
http://madwifi.org/wiki/DevDocs/RadiotapHeader

I don't remember how I managed to find that, but there it is.

Thanks,
-Michael Wu

#6 lore raw

Re: [PATCH 2/2] mac80211: Monitor mode radiotap-based packet injection

Quoted 7 lines · Michael Wu
Quoted 3 lines

I googled for radiotap specs but I didn't find anything useful. I added some small code to enforce the alignment rules you mention above.

I got the information on padding from:
http://madwifi.org/wiki/DevDocs/RadiotapHeader

I don't remember how I managed to find that, but there it is.

The packet-radiotap.c in Wireshark has the following comments:

/* Written with info from:
*
* http://madwifi.org/wiki/DevDocs/RadiotapHeader
* NetBSD's ieee80211_radiotap.h file
*/

/*
* The NetBSD ieee80211_radiotap man page
* (http://netbsd.gw.com/cgi-bin/man-cgi?ieee80211_radiotap+9+NetBSD-current)
* says:
*

 *    Radiotap capture fields must be naturally aligned.  That is, 16-, 32-,
 *    and 64-bit fields must begin on 16-, 32-, and 64-bit boundaries, respec-
 *    tively.  In this way, drivers can avoid unaligned accesses to radiotap
 *    capture fields.  radiotap-compliant drivers must insert padding before a
 *    capture field to ensure its natural alignment.  radiotap-compliant packet
 *    dissectors, such as tcpdump(8), expect the padding.

*/

Ciao
Joerg
--
Joerg Mayer <[email protected]>
We are stuck with technology when what we really want is just stuff that works. Some say that should read Microsoft instead of technology.