Search Linux Wireless

Re: [patch 3/5] A-MSDU Rx aggregation support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



modified patch at the end
Johannes Berg wrote:
On Mon, 2007-03-26 at 04:40 -0700, mohamed wrote:

Scratch my previous comments. Sorry about that, I should've looked
better.

+inline static int is_agg_frame(struct sk_buff *skb, u16 fc)
+{
+       u8 *p = skb->data + ieee80211_get_hdrlen(fc) - 2;

See below, but isn't this broken if somebody includes an HT field?

+       if (!(fc & IEEE80211_STYPE_QOS_DATA));
+               return 0;
+
+       if (*p & QOS_CONTROL_A_MSDU_PRESENT)
+               return 1;
+       else
+               return 0;
+}

+ payload = skb->data + hdrlen;
Ok so payload now points to the subframes.

+	if (unlikely(skb->len - hdrlen < 8)) {
+		if (net_ratelimit()) {
+			printk(KERN_DEBUG "%s: RX too short data frame "
+				"payload\n", dev->name);

Nit: this should increase the counter we have for too short frames
somewhere.

+		}
+		return TXRX_DROP;
+	}
+
+	ethertype = (payload[6] << 8) | payload[7];

Is that really correct?

+	if (likely((memcmp(payload, rfc1042_header, 6) == 0 &&
+		ethertype != ETH_P_AARP && ethertype != ETH_P_IPX) ||
+		memcmp(payload, bridge_tunnel_header, 6) == 0)) {
+		/* remove RFC1042 or Bridge-Tunnel encapsulation and
+		* replace EtherType */	
+		skb_pull(skb, hdrlen + 6);
+	} else {
+		skb_pull(skb, hdrlen);
+	}
+
+	eth = (struct ethhdr*)skb->data;

So that now points to the first actual subframe ethhdr. I misread that
in my first comment. Why do you actually skb_pull on this skb? It would
probably be more efficient to just assign "eth" in the different cases
since that needs no memory write (eth will probably be kept in a
register). And you throw it away anyway...

+	while ((u8*)eth < skb->data + skb->len) {

And that compares the pointer, not the value at it as I thought. Sorry.

+ unsigned int eth_len = sizeof(struct ethhdr) + + ntohs(eth->h_proto);

rename to subframe_len maybe?

+		frame = dev_alloc_skb(3000);
+
+ if (frame == NULL) + return TXRX_DROP;
+
+		frame->mac.raw = frame->data;
+		memcpy(skb_put(frame, eth_len), (u8*)eth, eth_len);

Nope, not nice to do this. You really need to check if eth_len doesn't
exceed the original skb's length or people can crash this by sending you
short frames with bogus huge eth len embedded in it. And if you move the
check up a bit then you can reuse "skb" for the last subframe in the
case where people don't send us garbage:

	int remaining = subframe_len - ((u8*)eth - skb->data);
	if (remaining < 0)
		/* idiots. should blacklist their mac address */
		return TXRX_DROP;
	if (remaining < 4 /* padding */) {
		frame = skb;
		skb = NULL;
		skb_pull(blabla)
	} else {
		frame = dev_alloc_skb(...)
		if (!frame)
			return TXRX_DROP;
	}

or something like that.

Don't you also need to set ((ethhdr*)frame->data)->h_proto to something
other than the length? Not exactly sure though, it seems eth_type_trans
can handle that and I don't have the 802.3 specs handy.

+		frame->dev = dev;

I think it'd be good to move that closer to the netif_rx.

+ * directly to it and do not pass + * the frame to local net stack.
+					*/
+					skb2 = skb;
+					skb = NULL;

I'm pretty sure you mean frame here instead of skb in both cases and
this is a copy/paste error from the other data rx handler.

+				}
+				if (dsta)
+					sta_info_put(dsta);
+			}
+		}
+		if (frame) {
+			/* deliver to local stack */
+			frame->protocol = eth_type_trans(frame, dev);
+			memset(frame->cb, 0, sizeof(frame->cb));

That memset is useless on a newly allocated frame.


johannes
Add A-MSDU Rx aggregation support.

To support IEEE 802.11n, we need to be able to process A-MSDU frames.
The present of the HT control field indicates it is A-MSDU frame.
This patch adds support to discover and process A-MSDU frames.

Signed-off-by: Mohamed Abbas <mabbas@xxxxxxxxxxxxxxx>

diff --git a/net/mac80211/ieee80211.c b/net/mac80211/ieee80211.c
index 3bf0be4..31cf5b8 100644
--- a/net/mac80211/ieee80211.c
+++ b/net/mac80211/ieee80211.c
@@ -2460,6 +2460,139 @@ static inline int ieee80211_bssid_match(
}


+inline static unsigned int calc_pad_len(unsigned int len)
+{
+	return ((4 - len) & 0x3);
+}
+
+static ieee80211_txrx_result
+ieee80211_rx_h_data_agg(struct ieee80211_txrx_data *rx)
+{
+	struct net_device *dev = rx->dev;
+	struct ieee80211_local *local = rx->local;
+	u16 fc, hdrlen, ethertype;
+	u8 *payload;
+	struct sk_buff *skb = rx->skb, *skb2, *frame;
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	const struct ethhdr* eth;
+	int remaining;
+
+	fc = rx->fc;
+	if (unlikely((fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA))
+		return TXRX_CONTINUE;
+
+	if (unlikely(!WLAN_FC_DATA_PRESENT(fc)))
+		return TXRX_DROP;
+
+	if (!rx->u.rx.is_agg_frame)
+		return TXRX_CONTINUE;
+
+	hdrlen = ieee80211_get_hdrlen(fc);
+
+	payload = skb->data + hdrlen;
+
+	if (unlikely((skb->len - hdrlen) < 8)) {
+		if (net_ratelimit())
+			printk(KERN_DEBUG "%s: RX too short data frame "
+			       "payload\n", dev->name);
+		return TXRX_DROP;
+	}
+
+	ethertype = (payload[6] << 8) | payload[7];
+
+	if (likely((compare_ether_addr(payload, rfc1042_header) == 0 &&
+		    ethertype != ETH_P_AARP && ethertype != ETH_P_IPX) ||
+	    	   compare_ether_addr(payload, bridge_tunnel_header) == 0)) {
+		/* remove RFC1042 or Bridge-Tunnel encapsulation and
+		* replace EtherType */	
+		eth = (struct ethhdr*) (skb->data + hdrlen + 6);
+		remaining = skb->len - (hdrlen + 6);
+	} else {
+		eth = (struct ethhdr*) (skb->data + hdrlen);
+		remaining = skb->len - hdrlen;
+	}
+
+	while ((u8*)eth < skb->data + skb->len) {
+		u8 padding;
+        	unsigned int subframe_len = sizeof(struct ethhdr) +
+						   ntohs(eth->h_proto);
+
+		padding = calc_pad_len(subframe_len);
+		/* the last MSDU has no padding */
+		if (subframe_len > remaining)
+			return TXRX_DROP;
+
+ frame = dev_alloc_skb(local->hw.extra_tx_headroom + + subframe_len);
+
+ if (frame == NULL) + return TXRX_DROP;
+
+		memcpy(skb_put(frame, subframe_len), (u8*)eth, subframe_len);
+		frame->mac.raw = frame->data;
+		skb2 = NULL;
+
+		sdata->stats.rx_packets++;
+		sdata->stats.rx_bytes += frame->len;
+
+ if (local->bridge_packets && + (sdata->type == IEEE80211_IF_TYPE_AP || + sdata->type == IEEE80211_IF_TYPE_VLAN) && + rx->u.rx.ra_match) {
+			if (is_multicast_ether_addr(frame->data)) {
+				/* send multicast frames both to higher layers
+ * in local net stack and back to the wireless + * media */
+				skb2 = skb_copy(frame, GFP_ATOMIC);
+				if (!skb2)
+					printk(KERN_DEBUG "%s: failed to clone"
+					       " multicast frame\n", dev->name);
+			} else {
+				struct sta_info *dsta;
+
+				dsta = sta_info_get(local, frame->data);
+ if (dsta && !dsta->dev) + printk(KERN_DEBUG "Station with null "
+					       "dev structure!\n");
+				else if (dsta && dsta->dev == dev) {
+ /* Destination station is associated + * to this AP, so send the frame + * directly to it and do not pass + * the frame to local net stack.
+					*/
+					skb2 = frame;
+					frame = NULL;
+				}
+				if (dsta)
+					sta_info_put(dsta);
+			}
+		}
+		if (frame) {
+			/* deliver to local stack */
+			frame->protocol = eth_type_trans(frame, dev);
+			frame->priority = skb->priority;
+			frame->dev = dev;
+			netif_rx(frame);
+		}
+
+		if (skb2) {
+			/* send to wireless media */
+			skb2->protocol = __constant_htons(ETH_P_802_3);
+			skb2->mac.raw = skb2->nh.raw = skb2->data;
+			skb2->priority = skb->priority;
+			skb2->dev = dev;
+			dev_queue_xmit(skb2);
+		}
+
+		eth = (struct ethhdr*)((u8*)eth + subframe_len + padding);
+
+		remaining -= (subframe_len + padding);
+	}
+
+	dev_kfree_skb(skb);
+	return TXRX_QUEUED;
+}
+
static ieee80211_txrx_result
ieee80211_rx_h_data(struct ieee80211_txrx_data *rx)
{
@@ -4389,6 +4522,7 @@ static ieee80211_rx_handler ieee80211_rx
	ieee80211_rx_h_remove_qos_control,
	ieee80211_rx_h_802_1x_pae,
	ieee80211_rx_h_drop_unencrypted,
+	ieee80211_rx_h_data_agg,
	ieee80211_rx_h_data,
	ieee80211_rx_h_mgmt,
	NULL
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index dd1d031..1cf8e82 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -142,10 +142,12 @@ struct ieee80211_txrx_data {
			int sent_ps_buffered;
			int queue;
			int load;
+			u16 qos_control;
			unsigned int in_scan:1;
			/* frame is destined to interface currently processed
			 * (including multicast frames) */
			unsigned int ra_match:1;
+			unsigned int is_agg_frame:1;
		} rx;
	} u;
#ifdef CONFIG_HOSTAPD_WPA_TESTING
diff --git a/net/mac80211/wme.c b/net/mac80211/wme.c
index d57be24..63b503c 100644
--- a/net/mac80211/wme.c
+++ b/net/mac80211/wme.c
@@ -31,12 +31,18 @@ ieee80211_rx_h_parse_qos(struct ieee8021
{
	u8 *data = rx->skb->data;
	int tid;
+	unsigned int is_agg_frame = 0;

	/* does the frame have a qos control field? */
	if (WLAN_FC_IS_QOS_DATA(rx->fc)) {
		u8 *qc = data + ieee80211_get_hdrlen(rx->fc) - QOS_CONTROL_LEN;
+		
		/* frame has qos control */
-		tid = qc[0] & QOS_CONTROL_TID_MASK;
+		rx->u.rx.qos_control = le16_to_cpu(*((__le16*)qc));
+		tid = rx->u.rx.qos_control & QOS_CONTROL_TID_MASK;
+ if (rx->u.rx.qos_control & + IEEE80211_QOS_CONTROL_A_MSDU_PRESENT)
+			is_agg_frame = 1;
	} else {
		if (unlikely((rx->fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_MGMT)) {
			/* Separate TID for management frames */
@@ -45,6 +51,7 @@ ieee80211_rx_h_parse_qos(struct ieee8021
			/* no qos control present */
			tid = 0; /* 802.1d - Best Effort */
		}
+		rx->u.rx.qos_control = 0;
	}
#ifdef CONFIG_MAC80211_DEBUG_COUNTERS
	I802_DEBUG_INC(rx->local->wme_rx_queue[tid]);
@@ -54,6 +61,7 @@ #ifdef CONFIG_MAC80211_DEBUG_COUNTERS
#endif /* CONFIG_MAC80211_DEBUG_COUNTERS */

	rx->u.rx.queue = tid;
+	rx->u.rx.is_agg_frame = is_agg_frame;
	/* Set skb->priority to 1d tag if highest order bit of TID is not set.
	 * For now, set skb->priority to 0 for other cases. */
	rx->skb->priority = (tid > 7) ? 0 : tid;
-
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux