mac80211: use skb list for fragments

We are currently linking the skbs by using skb->next directly. This works, but the preferred way is to use a struct sk_buff_head instead. That also prepares for passing that to drivers directly. While at it I noticed we calculate the duration for fragments twice -- remove one of them. Signed-off-by: Johannes Berg <johannes.berg@intel.com> Signed-off-by: John W. Linville <linville@tuxdriver.com>
git-mirror · Nov 21, 2011 · 252b86c · 252b86c
1 parent 83c7657
commit 252b86c
Show file tree

Hide file tree

Showing 5 changed files with 82 additions and 71 deletions.
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
@@ -142,6 +142,7 @@ typedef unsigned __bitwise__ ieee80211_tx_result;
 
 struct ieee80211_tx_data {
 	struct sk_buff *skb;
+	struct sk_buff_head skbs;
 	struct ieee80211_local *local;
 	struct ieee80211_sub_if_data *sdata;
 	struct sta_info *sta;

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
@@ -35,15 +35,16 @@
 
 /* misc utils */
 
-static __le16 ieee80211_duration(struct ieee80211_tx_data *tx, int group_addr,
+static __le16 ieee80211_duration(struct ieee80211_tx_data *tx,
+				 struct sk_buff *skb, int group_addr,
 				 int next_frag_len)
 {
 	int rate, mrate, erp, dur, i;
 	struct ieee80211_rate *txrate;
 	struct ieee80211_local *local = tx->local;
 	struct ieee80211_supported_band *sband;
 	struct ieee80211_hdr *hdr;
-	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx->skb);
+	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 
 	/* assume HW handles this */
 	if (info->control.rates[0].flags & IEEE80211_TX_RC_MCS)
@@ -75,7 +76,7 @@ static __le16 ieee80211_duration(struct ieee80211_tx_data *tx, int group_addr,
 	 *   at the highest possible rate belonging to the PHY rates in the
 	 *   BSSBasicRateSet
 	 */
-	hdr = (struct ieee80211_hdr *)tx->skb->data;
+	hdr = (struct ieee80211_hdr *)skb->data;
 	if (ieee80211_is_ctl(hdr->frame_control)) {
 		/* TODO: These control frames are not currently sent by
 		 * mac80211, but should they be implemented, this function
@@ -841,18 +842,21 @@ ieee80211_tx_h_sequence(struct ieee80211_tx_data *tx)
 	return TX_CONTINUE;
 }
 
-static int ieee80211_fragment(struct ieee80211_local *local,
+static int ieee80211_fragment(struct ieee80211_tx_data *tx,
 			      struct sk_buff *skb, int hdrlen,
 			      int frag_threshold)
 {
-	struct sk_buff *tail = skb, *tmp;
+	struct ieee80211_local *local = tx->local;
+	struct sk_buff *tmp;
 	int per_fragm = frag_threshold - hdrlen - FCS_LEN;
 	int pos = hdrlen + per_fragm;
 	int rem = skb->len - hdrlen - per_fragm;
 
 	if (WARN_ON(rem < 0))
 		return -EINVAL;
 
+	/* first fragment was already added to queue by caller */
+
 	while (rem) {
 		int fraglen = per_fragm;
 
@@ -865,8 +869,9 @@ static int ieee80211_fragment(struct ieee80211_local *local,
 				    IEEE80211_ENCRYPT_TAILROOM);
 		if (!tmp)
 			return -ENOMEM;
-		tail->next = tmp;
-		tail = tmp;
+
+		__skb_queue_tail(&tx->skbs, tmp);
+
 		skb_reserve(tmp, local->tx_headroom +
 				 IEEE80211_ENCRYPT_HEADROOM);
 		/* copy control information */
@@ -882,6 +887,7 @@ static int ieee80211_fragment(struct ieee80211_local *local,
 		pos += fraglen;
 	}
 
+	/* adjust first fragment's length */
 	skb->len = hdrlen + per_fragm;
 	return 0;
 }
@@ -896,6 +902,10 @@ ieee80211_tx_h_fragment(struct ieee80211_tx_data *tx)
 	int hdrlen;
 	int fragnum;
 
+	/* no matter what happens, tx->skb moves to tx->skbs */
+	__skb_queue_tail(&tx->skbs, skb);
+	tx->skb = NULL;
+
 	if (info->flags & IEEE80211_TX_CTL_DONTFRAG)
 		return TX_CONTINUE;
 
@@ -924,21 +934,21 @@ ieee80211_tx_h_fragment(struct ieee80211_tx_data *tx)
 	 * of the fragments then we will simply pretend to accept the skb
 	 * but store it away as pending.
 	 */
-	if (ieee80211_fragment(tx->local, skb, hdrlen, frag_threshold))
+	if (ieee80211_fragment(tx, skb, hdrlen, frag_threshold))
 		return TX_DROP;
 
 	/* update duration/seq/flags of fragments */
 	fragnum = 0;
-	do {
+
+	skb_queue_walk(&tx->skbs, skb) {
 		int next_len;
 		const __le16 morefrags = cpu_to_le16(IEEE80211_FCTL_MOREFRAGS);
 
 		hdr = (void *)skb->data;
 		info = IEEE80211_SKB_CB(skb);
 
-		if (skb->next) {
+		if (!skb_queue_is_last(&tx->skbs, skb)) {
 			hdr->frame_control |= morefrags;
-			next_len = skb->next->len;
 			/*
 			 * No multi-rate retries for fragmented frames, that
 			 * would completely throw off the NAV at other STAs.
@@ -953,27 +963,26 @@ ieee80211_tx_h_fragment(struct ieee80211_tx_data *tx)
 			hdr->frame_control &= ~morefrags;
 			next_len = 0;
 		}
-		hdr->duration_id = ieee80211_duration(tx, 0, next_len);
 		hdr->seq_ctrl |= cpu_to_le16(fragnum & IEEE80211_SCTL_FRAG);
 		fragnum++;
-	} while ((skb = skb->next));
+	}
 
 	return TX_CONTINUE;
 }
 
 static ieee80211_tx_result debug_noinline
 ieee80211_tx_h_stats(struct ieee80211_tx_data *tx)
 {
-	struct sk_buff *skb = tx->skb;
+	struct sk_buff *skb;
 
 	if (!tx->sta)
 		return TX_CONTINUE;
 
 	tx->sta->tx_packets++;
-	do {
+	skb_queue_walk(&tx->skbs, skb) {
 		tx->sta->tx_fragments++;
 		tx->sta->tx_bytes += skb->len;
-	} while ((skb = skb->next));
+	}
 
 	return TX_CONTINUE;
 }
@@ -1012,21 +1021,25 @@ ieee80211_tx_h_encrypt(struct ieee80211_tx_data *tx)
 static ieee80211_tx_result debug_noinline
 ieee80211_tx_h_calculate_duration(struct ieee80211_tx_data *tx)
 {
-	struct sk_buff *skb = tx->skb;
+	struct sk_buff *skb;
 	struct ieee80211_hdr *hdr;
 	int next_len;
 	bool group_addr;
 
-	do {
+	skb_queue_walk(&tx->skbs, skb) {
 		hdr = (void *) skb->data;
 		if (unlikely(ieee80211_is_pspoll(hdr->frame_control)))
 			break; /* must not overwrite AID */
-		next_len = skb->next ? skb->next->len : 0;
+		if (!skb_queue_is_last(&tx->skbs, skb)) {
+			struct sk_buff *next = skb_queue_next(&tx->skbs, skb);
+			next_len = next->len;
+		} else
+			next_len = 0;
 		group_addr = is_multicast_ether_addr(hdr->addr1);
 
 		hdr->duration_id =
-			ieee80211_duration(tx, group_addr, next_len);
-	} while ((skb = skb->next));
+			ieee80211_duration(tx, skb, group_addr, next_len);
+	}
 
 	return TX_CONTINUE;
 }
@@ -1105,6 +1118,7 @@ ieee80211_tx_prepare(struct ieee80211_sub_if_data *sdata,
 	tx->local = local;
 	tx->sdata = sdata;
 	tx->channel = local->hw.conf.channel;
+	__skb_queue_head_init(&tx->skbs);
 
 	/*
 	 * If this flag is set to true anywhere, and we get here,
@@ -1180,17 +1194,18 @@ ieee80211_tx_prepare(struct ieee80211_sub_if_data *sdata,
 /*
  * Returns false if the frame couldn't be transmitted but was queued instead.
  */
-static bool __ieee80211_tx(struct ieee80211_local *local, struct sk_buff **skbp,
+static bool __ieee80211_tx(struct ieee80211_local *local,
+			   struct sk_buff_head *skbs,
 			   struct sta_info *sta, bool txpending)
 {
-	struct sk_buff *skb = *skbp, *next;
+	struct sk_buff *skb, *tmp;
 	struct ieee80211_tx_info *info;
 	struct ieee80211_sub_if_data *sdata;
 	unsigned long flags;
 	int len;
 	bool fragm = false;
 
-	while (skb) {
+	skb_queue_walk_safe(skbs, skb, tmp) {
 		int q = skb_get_queue_mapping(skb);
 		__le16 fc;
 
@@ -1202,24 +1217,10 @@ static bool __ieee80211_tx(struct ieee80211_local *local, struct sk_buff **skbp,
 			 * transmission from the tx-pending tasklet when the
 			 * queue is woken again.
 			 */
-
-			do {
-				next = skb->next;
-				skb->next = NULL;
-				/*
-				 * NB: If txpending is true, next must already
-				 * be NULL since we must've gone through this
-				 * loop before already; therefore we can just
-				 * queue the frame to the head without worrying
-				 * about reordering of fragments.
-				 */
-				if (unlikely(txpending))
-					__skb_queue_head(&local->pending[q],
-							 skb);
-				else
-					__skb_queue_tail(&local->pending[q],
-							 skb);
-			} while ((skb = next));
+			if (txpending)
+				skb_queue_splice(skbs, &local->pending[q]);
+			else
+				skb_queue_splice_tail(skbs, &local->pending[q]);
 
 			spin_unlock_irqrestore(&local->queue_stop_reason_lock,
 					       flags);
@@ -1233,10 +1234,9 @@ static bool __ieee80211_tx(struct ieee80211_local *local, struct sk_buff **skbp,
 			info->flags &= ~(IEEE80211_TX_CTL_CLEAR_PS_FILT |
 					 IEEE80211_TX_CTL_FIRST_FRAGMENT);
 
-		next = skb->next;
 		len = skb->len;
 
-		if (next)
+		if (!skb_queue_is_last(skbs, skb))
 			info->flags |= IEEE80211_TX_CTL_MORE_FRAMES;
 
 		sdata = vif_to_sdata(info->control.vif);
@@ -1260,14 +1260,17 @@ static bool __ieee80211_tx(struct ieee80211_local *local, struct sk_buff **skbp,
 			info->control.sta = NULL;
 
 		fc = ((struct ieee80211_hdr *)skb->data)->frame_control;
+
+		__skb_unlink(skb, skbs);
 		drv_tx(local, skb);
 
 		ieee80211_tpt_led_trig_tx(local, fc, len);
-		*skbp = skb = next;
 		ieee80211_led_tx(local, 1);
 		fragm = true;
 	}
 
+	WARN_ON(!skb_queue_empty(skbs));
+
 	return true;
 }
 
@@ -1277,8 +1280,7 @@ static bool __ieee80211_tx(struct ieee80211_local *local, struct sk_buff **skbp,
  */
 static int invoke_tx_handlers(struct ieee80211_tx_data *tx)
 {
-	struct sk_buff *skb = tx->skb;
-	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx->skb);
 	ieee80211_tx_result res = TX_DROP;
 
 #define CALL_TXH(txh) \
@@ -1312,13 +1314,10 @@ static int invoke_tx_handlers(struct ieee80211_tx_data *tx)
  txh_done:
 	if (unlikely(res == TX_DROP)) {
 		I802_DEBUG_INC(tx->local->tx_handlers_drop);
-		while (skb) {
-			struct sk_buff *next;
-
-			next = skb->next;
-			dev_kfree_skb(skb);
-			skb = next;
-		}
+		if (tx->skb)
+			dev_kfree_skb(tx->skb);
+		else
+			__skb_queue_purge(&tx->skbs);
 		return -1;
 	} else if (unlikely(res == TX_QUEUED)) {
 		I802_DEBUG_INC(tx->local->tx_handlers_queued);
@@ -1361,7 +1360,7 @@ static bool ieee80211_tx(struct ieee80211_sub_if_data *sdata,
 	info->band = tx.channel->band;
 
 	if (!invoke_tx_handlers(&tx))
-		result = __ieee80211_tx(local, &tx.skb, tx.sta, txpending);
+		result = __ieee80211_tx(local, &tx.skbs, tx.sta, txpending);
  out:
 	rcu_read_unlock();
 	return result;
@@ -2109,10 +2108,15 @@ static bool ieee80211_tx_pending_skb(struct ieee80211_local *local,
 	if (info->flags & IEEE80211_TX_INTFL_NEED_TXPROCESSING) {
 		result = ieee80211_tx(sdata, skb, true);
 	} else {
+		struct sk_buff_head skbs;
+
+		__skb_queue_head_init(&skbs);
+		__skb_queue_tail(&skbs, skb);
+
 		hdr = (struct ieee80211_hdr *)skb->data;
 		sta = sta_info_get(sdata, hdr->addr1);
 
-		result = __ieee80211_tx(local, &skb, sta, true);
+		result = __ieee80211_tx(local, &skbs, sta, true);
 	}
 
 	return result;

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
@@ -95,13 +95,13 @@ u8 *ieee80211_get_bssid(struct ieee80211_hdr *hdr, size_t len,
 
 void ieee80211_tx_set_protected(struct ieee80211_tx_data *tx)
 {
-	struct sk_buff *skb = tx->skb;
+	struct sk_buff *skb;
 	struct ieee80211_hdr *hdr;
 
-	do {
+	skb_queue_walk(&tx->skbs, skb) {
 		hdr = (struct ieee80211_hdr *) skb->data;
 		hdr->frame_control |= cpu_to_le16(IEEE80211_FCTL_PROTECTED);
-	} while ((skb = skb->next));
+	}
 }
 
 int ieee80211_frame_duration(struct ieee80211_local *local, size_t len,

diff --git a/net/mac80211/wep.c b/net/mac80211/wep.c
@@ -330,13 +330,12 @@ ieee80211_crypto_wep_encrypt(struct ieee80211_tx_data *tx)
 
 	ieee80211_tx_set_protected(tx);
 
-	skb = tx->skb;
-	do {
+	skb_queue_walk(&tx->skbs, skb) {
 		if (wep_encrypt_skb(tx, skb) < 0) {
 			I802_DEBUG_INC(tx->local->tx_handlers_drop_wep);
 			return TX_DROP;
 		}
-	} while ((skb = skb->next));
+	}
 
 	return TX_CONTINUE;
 }