linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] submit fragmented packets to drivers at once
@ 2011-11-16 14:28 Johannes Berg
  2011-11-16 14:28 ` [PATCH 1/4] mac80211: use skb list for fragments Johannes Berg
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Johannes Berg @ 2011-11-16 14:28 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

This is something I have wanted to do for a while -- many
drivers can handle multiple fragments at once much easier
than getting every single one at a time.

I've long thought about how to do this best and arrived at
the conclusion now that simply adding a compat handler for
tx_frags (which mac80211 uses) is best.

Tested on iwlagn with the compat handler, will use the new
tx_frags soon I hope.

johannes


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] mac80211: use skb list for fragments
  2011-11-16 14:28 [PATCH 0/4] submit fragmented packets to drivers at once Johannes Berg
@ 2011-11-16 14:28 ` Johannes Berg
  2011-11-16 14:28 ` [PATCH 2/4] mac80211: move fragment flag adjustment Johannes Berg
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Johannes Berg @ 2011-11-16 14:28 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

From: Johannes Berg <johannes.berg@intel.com>

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>
---
 net/mac80211/ieee80211_i.h |    1 
 net/mac80211/tx.c          |  116 +++++++++++++++++++++++----------------------
 net/mac80211/util.c        |    6 +-
 net/mac80211/wep.c         |    5 -
 net/mac80211/wpa.c         |   25 ++++++---
 5 files changed, 82 insertions(+), 71 deletions(-)

--- a/net/mac80211/ieee80211_i.h	2011-11-16 15:19:36.000000000 +0100
+++ b/net/mac80211/ieee80211_i.h	2011-11-16 15:19:38.000000000 +0100
@@ -142,6 +142,7 @@ typedef unsigned __bitwise__ ieee80211_t
 
 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;
--- a/net/mac80211/tx.c	2011-11-16 15:19:36.000000000 +0100
+++ b/net/mac80211/tx.c	2011-11-16 15:19:43.000000000 +0100
@@ -36,7 +36,8 @@
 
 /* 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;
@@ -44,7 +45,7 @@ static __le16 ieee80211_duration(struct
 	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)
@@ -76,7 +77,7 @@ static __le16 ieee80211_duration(struct
 	 *   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
@@ -844,11 +845,12 @@ ieee80211_tx_h_sequence(struct ieee80211
 	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;
@@ -856,6 +858,8 @@ static int ieee80211_fragment(struct iee
 	if (WARN_ON(rem < 0))
 		return -EINVAL;
 
+	/* first fragment was already added to queue by caller */
+
 	while (rem) {
 		int fraglen = per_fragm;
 
@@ -868,8 +872,9 @@ static int ieee80211_fragment(struct iee
 				    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 */
@@ -885,6 +890,7 @@ static int ieee80211_fragment(struct iee
 		pos += fraglen;
 	}
 
+	/* adjust first fragment's length */
 	skb->len = hdrlen + per_fragm;
 	return 0;
 }
@@ -899,6 +905,10 @@ ieee80211_tx_h_fragment(struct ieee80211
 	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;
 
@@ -927,21 +937,21 @@ ieee80211_tx_h_fragment(struct ieee80211
 	 * 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.
@@ -956,10 +966,9 @@ ieee80211_tx_h_fragment(struct ieee80211
 			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;
 }
@@ -967,16 +976,16 @@ ieee80211_tx_h_fragment(struct ieee80211
 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;
 }
@@ -1015,21 +1024,25 @@ ieee80211_tx_h_encrypt(struct ieee80211_
 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;
 }
@@ -1108,6 +1121,7 @@ ieee80211_tx_prepare(struct ieee80211_su
 	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,
@@ -1183,17 +1197,18 @@ ieee80211_tx_prepare(struct ieee80211_su
 /*
  * 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;
 
@@ -1205,24 +1220,10 @@ static bool __ieee80211_tx(struct ieee80
 			 * 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);
@@ -1236,10 +1237,9 @@ static bool __ieee80211_tx(struct ieee80
 			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);
@@ -1263,14 +1263,17 @@ static bool __ieee80211_tx(struct ieee80
 			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;
 }
 
@@ -1280,8 +1283,7 @@ static bool __ieee80211_tx(struct ieee80
  */
 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) \
@@ -1315,13 +1317,10 @@ static int invoke_tx_handlers(struct iee
  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);
@@ -1364,7 +1363,7 @@ static bool ieee80211_tx(struct ieee8021
 	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;
@@ -2112,10 +2111,15 @@ static bool ieee80211_tx_pending_skb(str
 	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;
--- a/net/mac80211/util.c	2011-11-16 15:19:36.000000000 +0100
+++ b/net/mac80211/util.c	2011-11-16 15:19:38.000000000 +0100
@@ -96,13 +96,13 @@ u8 *ieee80211_get_bssid(struct ieee80211
 
 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,
--- a/net/mac80211/wep.c	2011-11-16 15:19:36.000000000 +0100
+++ b/net/mac80211/wep.c	2011-11-16 15:19:38.000000000 +0100
@@ -330,13 +330,12 @@ ieee80211_crypto_wep_encrypt(struct ieee
 
 	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;
 }
--- a/net/mac80211/wpa.c	2011-11-16 15:19:36.000000000 +0100
+++ b/net/mac80211/wpa.c	2011-11-16 15:19:38.000000000 +0100
@@ -223,14 +223,14 @@ static int tkip_encrypt_skb(struct ieee8
 ieee80211_tx_result
 ieee80211_crypto_tkip_encrypt(struct ieee80211_tx_data *tx)
 {
-	struct sk_buff *skb = tx->skb;
+	struct sk_buff *skb;
 
 	ieee80211_tx_set_protected(tx);
 
-	do {
+	skb_queue_walk(&tx->skbs, skb) {
 		if (tkip_encrypt_skb(tx, skb) < 0)
 			return TX_DROP;
-	} while ((skb = skb->next));
+	}
 
 	return TX_CONTINUE;
 }
@@ -449,14 +449,14 @@ static int ccmp_encrypt_skb(struct ieee8
 ieee80211_tx_result
 ieee80211_crypto_ccmp_encrypt(struct ieee80211_tx_data *tx)
 {
-	struct sk_buff *skb = tx->skb;
+	struct sk_buff *skb;
 
 	ieee80211_tx_set_protected(tx);
 
-	do {
+	skb_queue_walk(&tx->skbs, skb) {
 		if (ccmp_encrypt_skb(tx, skb) < 0)
 			return TX_DROP;
-	} while ((skb = skb->next));
+	}
 
 	return TX_CONTINUE;
 }
@@ -554,15 +554,22 @@ static inline void bip_ipn_swap(u8 *d, c
 ieee80211_tx_result
 ieee80211_crypto_aes_cmac_encrypt(struct ieee80211_tx_data *tx)
 {
-	struct sk_buff *skb = tx->skb;
-	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+	struct sk_buff *skb;
+	struct ieee80211_tx_info *info;
 	struct ieee80211_key *key = tx->key;
 	struct ieee80211_mmie *mmie;
 	u8 aad[20];
 	u64 pn64;
 
+	if (WARN_ON(skb_queue_len(&tx->skbs) != 1))
+		return TX_DROP;
+
+	skb = skb_peek(&tx->skbs);
+
+	info = IEEE80211_SKB_CB(skb);
+
 	if (info->control.hw_key)
-		return 0;
+		return TX_CONTINUE;
 
 	if (WARN_ON(skb_tailroom(skb) < sizeof(*mmie)))
 		return TX_DROP;



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/4] mac80211: move fragment flag adjustment
  2011-11-16 14:28 [PATCH 0/4] submit fragmented packets to drivers at once Johannes Berg
  2011-11-16 14:28 ` [PATCH 1/4] mac80211: use skb list for fragments Johannes Berg
@ 2011-11-16 14:28 ` Johannes Berg
  2011-11-16 14:28 ` [PATCH 3/4] mac80211: make TX LED handling independent of fragmentation Johannes Berg
  2011-11-16 14:28 ` [PATCH 4/4] mac80211: transmit fragment list to drivers Johannes Berg
  3 siblings, 0 replies; 9+ messages in thread
From: Johannes Berg @ 2011-11-16 14:28 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

From: Johannes Berg <johannes.berg@intel.com>

Instead of adjusting the fragment flags at
TX time, adjust them at fragmentation time.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/tx.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

--- a/net/mac80211/tx.c	2011-11-16 15:15:57.000000000 +0100
+++ b/net/mac80211/tx.c	2011-11-16 15:16:45.000000000 +0100
@@ -850,6 +850,7 @@ static int ieee80211_fragment(struct iee
 			      int frag_threshold)
 {
 	struct ieee80211_local *local = tx->local;
+	struct ieee80211_tx_info *info;
 	struct sk_buff *tmp;
 	int per_fragm = frag_threshold - hdrlen - FCS_LEN;
 	int pos = hdrlen + per_fragm;
@@ -879,6 +880,14 @@ static int ieee80211_fragment(struct iee
 				 IEEE80211_ENCRYPT_HEADROOM);
 		/* copy control information */
 		memcpy(tmp->cb, skb->cb, sizeof(tmp->cb));
+
+		info = IEEE80211_SKB_CB(tmp);
+		info->flags &= ~(IEEE80211_TX_CTL_CLEAR_PS_FILT |
+				 IEEE80211_TX_CTL_FIRST_FRAGMENT);
+
+		if (rem)
+			info->flags |= IEEE80211_TX_CTL_MORE_FRAMES;
+
 		skb_copy_queue_mapping(tmp, skb);
 		tmp->priority = skb->priority;
 		tmp->dev = skb->dev;
@@ -1206,7 +1215,6 @@ static bool __ieee80211_tx(struct ieee80
 	struct ieee80211_sub_if_data *sdata;
 	unsigned long flags;
 	int len;
-	bool fragm = false;
 
 	skb_queue_walk_safe(skbs, skb, tmp) {
 		int q = skb_get_queue_mapping(skb);
@@ -1233,15 +1241,8 @@ static bool __ieee80211_tx(struct ieee80
 
 		info = IEEE80211_SKB_CB(skb);
 
-		if (fragm)
-			info->flags &= ~(IEEE80211_TX_CTL_CLEAR_PS_FILT |
-					 IEEE80211_TX_CTL_FIRST_FRAGMENT);
-
 		len = skb->len;
 
-		if (!skb_queue_is_last(skbs, skb))
-			info->flags |= IEEE80211_TX_CTL_MORE_FRAMES;
-
 		sdata = vif_to_sdata(info->control.vif);
 
 		switch (sdata->vif.type) {
@@ -1269,7 +1270,6 @@ static bool __ieee80211_tx(struct ieee80
 
 		ieee80211_tpt_led_trig_tx(local, fc, len);
 		ieee80211_led_tx(local, 1);
-		fragm = true;
 	}
 
 	WARN_ON(!skb_queue_empty(skbs));



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/4] mac80211: make TX LED handling independent of fragmentation
  2011-11-16 14:28 [PATCH 0/4] submit fragmented packets to drivers at once Johannes Berg
  2011-11-16 14:28 ` [PATCH 1/4] mac80211: use skb list for fragments Johannes Berg
  2011-11-16 14:28 ` [PATCH 2/4] mac80211: move fragment flag adjustment Johannes Berg
@ 2011-11-16 14:28 ` Johannes Berg
  2011-11-16 14:28 ` [PATCH 4/4] mac80211: transmit fragment list to drivers Johannes Berg
  3 siblings, 0 replies; 9+ messages in thread
From: Johannes Berg @ 2011-11-16 14:28 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

From: Johannes Berg <johannes.berg@intel.com>

This just prepares for passing the entire fragment
list to the driver. No significant changes, but the
TX throughput is calculated slightly differently
now and we blink only once for each MSDU.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/tx.c |   28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

--- a/net/mac80211/tx.c	2011-11-16 15:16:45.000000000 +0100
+++ b/net/mac80211/tx.c	2011-11-16 15:17:19.000000000 +0100
@@ -1207,18 +1207,23 @@ ieee80211_tx_prepare(struct ieee80211_su
  * Returns false if the frame couldn't be transmitted but was queued instead.
  */
 static bool __ieee80211_tx(struct ieee80211_local *local,
-			   struct sk_buff_head *skbs,
+			   struct sk_buff_head *skbs, int led_len,
 			   struct sta_info *sta, bool txpending)
 {
 	struct sk_buff *skb, *tmp;
 	struct ieee80211_tx_info *info;
 	struct ieee80211_sub_if_data *sdata;
 	unsigned long flags;
-	int len;
+	__le16 fc;
+
+	if (WARN_ON(skb_queue_empty(skbs)))
+		return true;
+
+	skb = skb_peek(skbs);
+	fc = ((struct ieee80211_hdr *)skb->data)->frame_control;
 
 	skb_queue_walk_safe(skbs, skb, tmp) {
 		int q = skb_get_queue_mapping(skb);
-		__le16 fc;
 
 		spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
 		if (local->queue_stop_reasons[q] ||
@@ -1241,8 +1246,6 @@ static bool __ieee80211_tx(struct ieee80
 
 		info = IEEE80211_SKB_CB(skb);
 
-		len = skb->len;
-
 		sdata = vif_to_sdata(info->control.vif);
 
 		switch (sdata->vif.type) {
@@ -1263,15 +1266,13 @@ static bool __ieee80211_tx(struct ieee80
 		else
 			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);
-		ieee80211_led_tx(local, 1);
 	}
 
+	ieee80211_tpt_led_trig_tx(local, fc, led_len);
+	ieee80211_led_tx(local, 1);
+
 	WARN_ON(!skb_queue_empty(skbs));
 
 	return true;
@@ -1341,6 +1342,7 @@ static bool ieee80211_tx(struct ieee8021
 	ieee80211_tx_result res_prepare;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	bool result = true;
+	int led_len;
 
 	if (unlikely(skb->len < 10)) {
 		dev_kfree_skb(skb);
@@ -1350,6 +1352,7 @@ static bool ieee80211_tx(struct ieee8021
 	rcu_read_lock();
 
 	/* initialises tx */
+	led_len = skb->len;
 	res_prepare = ieee80211_tx_prepare(sdata, &tx, skb);
 
 	if (unlikely(res_prepare == TX_DROP)) {
@@ -1363,7 +1366,8 @@ static bool ieee80211_tx(struct ieee8021
 	info->band = tx.channel->band;
 
 	if (!invoke_tx_handlers(&tx))
-		result = __ieee80211_tx(local, &tx.skbs, tx.sta, txpending);
+		result = __ieee80211_tx(local, &tx.skbs, led_len,
+					tx.sta, txpending);
  out:
 	rcu_read_unlock();
 	return result;
@@ -2119,7 +2123,7 @@ static bool ieee80211_tx_pending_skb(str
 		hdr = (struct ieee80211_hdr *)skb->data;
 		sta = sta_info_get(sdata, hdr->addr1);
 
-		result = __ieee80211_tx(local, &skbs, sta, true);
+		result = __ieee80211_tx(local, &skbs, skb->len, sta, true);
 	}
 
 	return result;



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/4] mac80211: transmit fragment list to drivers
  2011-11-16 14:28 [PATCH 0/4] submit fragmented packets to drivers at once Johannes Berg
                   ` (2 preceding siblings ...)
  2011-11-16 14:28 ` [PATCH 3/4] mac80211: make TX LED handling independent of fragmentation Johannes Berg
@ 2011-11-16 14:28 ` Johannes Berg
  2011-11-16 14:40   ` Ivo Van Doorn
  2011-11-16 15:02   ` [PATCH v2 " Johannes Berg
  3 siblings, 2 replies; 9+ messages in thread
From: Johannes Berg @ 2011-11-16 14:28 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

From: Johannes Berg <johannes.berg@intel.com>

Drivers can usually handle fragmented packets
much easier when they get the entire list of
fragments at once. The only thing they need to
do is keep enough space on the queues for up
to ten fragments of a single MSDU.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 drivers/net/wireless/adm8211.c                        |    1 
 drivers/net/wireless/at76c50x-usb.c                   |    1 
 drivers/net/wireless/ath/ath5k/mac80211-ops.c         |    1 
 drivers/net/wireless/ath/ath9k/htc_drv_main.c         |    1 
 drivers/net/wireless/ath/ath9k/main.c                 |    1 
 drivers/net/wireless/ath/carl9170/main.c              |    1 
 drivers/net/wireless/b43/main.c                       |    1 
 drivers/net/wireless/b43legacy/main.c                 |    1 
 drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c |    1 
 drivers/net/wireless/iwlegacy/iwl-4965.c              |    1 
 drivers/net/wireless/iwlegacy/iwl3945-base.c          |    1 
 drivers/net/wireless/iwlwifi/iwl-mac80211.c           |    1 
 drivers/net/wireless/libertas_tf/main.c               |    1 
 drivers/net/wireless/mac80211_hwsim.c                 |    1 
 drivers/net/wireless/mwl8k.c                          |    1 
 drivers/net/wireless/p54/main.c                       |    1 
 drivers/net/wireless/rt2x00/rt2400pci.c               |    1 
 drivers/net/wireless/rt2x00/rt2500pci.c               |    1 
 drivers/net/wireless/rt2x00/rt2500usb.c               |    1 
 drivers/net/wireless/rt2x00/rt2800pci.c               |    1 
 drivers/net/wireless/rt2x00/rt2800usb.c               |    1 
 drivers/net/wireless/rt2x00/rt61pci.c                 |    1 
 drivers/net/wireless/rt2x00/rt73usb.c                 |    1 
 drivers/net/wireless/rtl818x/rtl8180/dev.c            |    1 
 drivers/net/wireless/rtl818x/rtl8187/dev.c            |    1 
 drivers/net/wireless/rtlwifi/core.c                   |    1 
 drivers/net/wireless/wl1251/main.c                    |    1 
 drivers/net/wireless/wl12xx/main.c                    |    1 
 drivers/net/wireless/zd1211rw/zd_mac.c                |    1 
 drivers/staging/winbond/wbusb.c                       |    1 
 include/net/mac80211.h                                |   40 ++++++-
 net/mac80211/driver-ops.h                             |   12 ++
 net/mac80211/tx.c                                     |   98 +++++++++++-------
 33 files changed, 138 insertions(+), 42 deletions(-)

--- a/include/net/mac80211.h	2011-11-16 15:19:31.000000000 +0100
+++ b/include/net/mac80211.h	2011-11-16 15:20:03.000000000 +0100
@@ -1760,11 +1760,20 @@ enum ieee80211_frame_release_type {
  *	skb contains the buffer starting from the IEEE 802.11 header.
  *	The low-level driver should send the frame out based on
  *	configuration in the TX control data. This handler should,
- *	preferably, never fail and stop queues appropriately, more
- *	importantly, however, it must never fail for A-MPDU-queues.
- *	This function should return NETDEV_TX_OK except in very
- *	limited cases.
- *	Must be implemented and atomic.
+ *	preferably, never fail and stop queues appropriately.
+ *	This needs only be implemented if @tx_frags is set to the
+ *	ieee80211_generic_tx_frags handler.
+ *	Must be atomic.
+ *
+ * @tx_frags: Called to transmit multiple fragments of a single MSDU.
+ *	This handler must consume all fragments, sending out some of
+ *	them only is useless and it can't ask for some of them to be
+ *	queued again. If the frame is not fragmented the queue has a
+ *	single SKB only.
+ *	If this is called, the tx_info @vif and @sta pointer will be
+ *	invalid -- you must not use them in that case.
+ *	Must be implemented and atomic. If the driver implements @tx
+ *	only then this must be set to ieee80211_generic_tx_frags().
  *
  * @start: Called before the first netdevice attached to the hardware
  *	is enabled. This should turn on the hardware and must turn on
@@ -2101,6 +2110,9 @@ enum ieee80211_frame_release_type {
  */
 struct ieee80211_ops {
 	void (*tx)(struct ieee80211_hw *hw, struct sk_buff *skb);
+	void (*tx_frags)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
+			 struct ieee80211_sta *sta, struct sk_buff_head *skbs,
+			 void *internal);
 	int (*start)(struct ieee80211_hw *hw);
 	void (*stop)(struct ieee80211_hw *hw);
 #ifdef CONFIG_PM
@@ -2572,6 +2584,24 @@ void ieee80211_sta_set_buffered(struct i
 				u8 tid, bool buffered);
 
 /**
+ * ieee80211_generic_tx_frags - generic tx_frags handler
+ * @hw: the hardware
+ * @vif: virtual interface, or %NULL
+ * @sta: station, or %NULL
+ * @skbs: the skbs to transmit
+ * @internal: internal data
+ *
+ * This is the generic handler for the @tx_frags operation when
+ * the driver prefers @tx over @tx_frags for whatever reason. It
+ * must not be called in any other context.
+ */
+void ieee80211_generic_tx_frags(struct ieee80211_hw *hw,
+				struct ieee80211_vif *vif,
+				struct ieee80211_sta *sta,
+				struct sk_buff_head *skbs,
+				void *internal);
+
+/**
  * ieee80211_tx_status - transmit status callback
  *
  * Call this function for all transmitted frames after they have been
--- a/net/mac80211/driver-ops.h	2011-11-16 15:19:31.000000000 +0100
+++ b/net/mac80211/driver-ops.h	2011-11-16 15:20:03.000000000 +0100
@@ -15,6 +15,18 @@ static inline void drv_tx(struct ieee802
 	local->ops->tx(&local->hw, skb);
 }
 
+static inline void drv_tx_frags(struct ieee80211_local *local,
+				struct ieee80211_sub_if_data *sdata,
+				struct sta_info *sta,
+				struct sk_buff_head *skbs,
+				void *internal)
+{
+	local->ops->tx_frags(&local->hw,
+			     sdata ? &sdata->vif : NULL,
+			     sta ? &sta->sta : NULL,
+			     skbs, internal);
+}
+
 static inline int drv_start(struct ieee80211_local *local)
 {
 	int ret;
--- a/net/mac80211/tx.c	2011-11-16 15:20:02.000000000 +0100
+++ b/net/mac80211/tx.c	2011-11-16 15:20:03.000000000 +0100
@@ -1203,79 +1203,103 @@ ieee80211_tx_prepare(struct ieee80211_su
 	return TX_CONTINUE;
 }
 
-/*
- * Returns false if the frame couldn't be transmitted but was queued instead.
- */
-static bool __ieee80211_tx(struct ieee80211_local *local,
-			   struct sk_buff_head *skbs, int led_len,
-			   struct sta_info *sta, bool txpending)
+struct internal_tx_data {
+	bool txpending, result;
+};
+
+void ieee80211_generic_tx_frags(struct ieee80211_hw *hw,
+				struct ieee80211_vif *vif,
+				struct ieee80211_sta *sta,
+				struct sk_buff_head *skbs,
+				void *internal)
 {
+	struct internal_tx_data *itd = internal;
+	struct ieee80211_local *local = hw_to_local(hw);
 	struct sk_buff *skb, *tmp;
 	struct ieee80211_tx_info *info;
-	struct ieee80211_sub_if_data *sdata;
 	unsigned long flags;
-	__le16 fc;
-
-	if (WARN_ON(skb_queue_empty(skbs)))
-		return true;
-
-	skb = skb_peek(skbs);
-	fc = ((struct ieee80211_hdr *)skb->data)->frame_control;
 
 	skb_queue_walk_safe(skbs, skb, tmp) {
 		int q = skb_get_queue_mapping(skb);
 
 		spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
 		if (local->queue_stop_reasons[q] ||
-		    (!txpending && !skb_queue_empty(&local->pending[q]))) {
+		    (!itd->txpending && !skb_queue_empty(&local->pending[q]))) {
 			/*
 			 * Since queue is stopped, queue up frames for later
 			 * transmission from the tx-pending tasklet when the
 			 * queue is woken again.
 			 */
-			if (txpending)
+			if (itd->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);
-			return false;
+			itd->result = false;
+			return;
 		}
 		spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
 
 		info = IEEE80211_SKB_CB(skb);
+		info->control.vif = vif;
+		info->control.sta = sta;
 
-		sdata = vif_to_sdata(info->control.vif);
+		__skb_unlink(skb, skbs);
+		drv_tx(local, skb);
+	}
 
-		switch (sdata->vif.type) {
-		case NL80211_IFTYPE_MONITOR:
-			info->control.vif = NULL;
-			break;
-		case NL80211_IFTYPE_AP_VLAN:
-			info->control.vif = &container_of(sdata->bss,
-				struct ieee80211_sub_if_data, u.ap)->vif;
-			break;
-		default:
-			/* keep */
-			break;
-		}
+	itd->result = true;
+}
+EXPORT_SYMBOL(ieee80211_generic_tx_frags);
 
-		if (sta && sta->uploaded)
-			info->control.sta = &sta->sta;
-		else
-			info->control.sta = NULL;
+/*
+ * Returns false if the frame couldn't be transmitted but was queued instead.
+ */
+static bool __ieee80211_tx(struct ieee80211_local *local,
+			   struct sk_buff_head *skbs, int led_len,
+			   struct sta_info *sta, bool txpending)
+{
+	struct internal_tx_data itd = {
+		.txpending = txpending,
+		.result = true,
+	};
+	struct ieee80211_tx_info *info;
+	struct ieee80211_sub_if_data *sdata;
+	struct sk_buff *skb;
+	__le16 fc;
 
-		__skb_unlink(skb, skbs);
-		drv_tx(local, skb);
+	if (WARN_ON(skb_queue_empty(skbs)))
+		return true;
+
+	skb = skb_peek(skbs);
+	fc = ((struct ieee80211_hdr *)skb->data)->frame_control;
+	info = IEEE80211_SKB_CB(skb);
+	sdata = vif_to_sdata(info->control.vif);
+	if (sta && !sta->uploaded)
+		sta = NULL;
+
+	switch (sdata->vif.type) {
+	case NL80211_IFTYPE_MONITOR:
+		sdata = NULL;
+		break;
+	case NL80211_IFTYPE_AP_VLAN:
+		sdata = container_of(sdata->bss,
+				     struct ieee80211_sub_if_data, u.ap);
+	default:
+		/* keep */
+		break;
 	}
 
+	drv_tx_frags(local, sdata, sta, skbs, &itd);
+
 	ieee80211_tpt_led_trig_tx(local, fc, led_len);
 	ieee80211_led_tx(local, 1);
 
 	WARN_ON(!skb_queue_empty(skbs));
 
-	return true;
+	return itd.result;
 }
 
 /*
--- a/drivers/net/wireless/adm8211.c	2011-11-16 15:19:32.000000000 +0100
+++ b/drivers/net/wireless/adm8211.c	2011-11-16 15:20:03.000000000 +0100
@@ -1747,6 +1747,7 @@ static int adm8211_alloc_rings(struct ie
 }
 
 static const struct ieee80211_ops adm8211_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= adm8211_tx,
 	.start			= adm8211_start,
 	.stop			= adm8211_stop,
--- a/drivers/net/wireless/at76c50x-usb.c	2011-11-16 15:19:33.000000000 +0100
+++ b/drivers/net/wireless/at76c50x-usb.c	2011-11-16 15:20:03.000000000 +0100
@@ -2100,6 +2100,7 @@ static int at76_set_key(struct ieee80211
 }
 
 static const struct ieee80211_ops at76_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx = at76_mac80211_tx,
 	.add_interface = at76_add_interface,
 	.remove_interface = at76_remove_interface,
--- a/drivers/net/wireless/ath/ath5k/mac80211-ops.c	2011-11-16 15:19:32.000000000 +0100
+++ b/drivers/net/wireless/ath/ath5k/mac80211-ops.c	2011-11-16 15:20:03.000000000 +0100
@@ -769,6 +769,7 @@ static int ath5k_set_ringparam(struct ie
 
 
 const struct ieee80211_ops ath5k_hw_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= ath5k_tx,
 	.start			= ath5k_start,
 	.stop			= ath5k_stop,
--- a/drivers/net/wireless/ath/ath9k/htc_drv_main.c	2011-11-16 15:19:32.000000000 +0100
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_main.c	2011-11-16 15:20:03.000000000 +0100
@@ -1757,6 +1757,7 @@ static int ath9k_htc_get_stats(struct ie
 }
 
 struct ieee80211_ops ath9k_htc_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx                 = ath9k_htc_tx,
 	.start              = ath9k_htc_start,
 	.stop               = ath9k_htc_stop,
--- a/drivers/net/wireless/ath/ath9k/main.c	2011-11-16 15:19:32.000000000 +0100
+++ b/drivers/net/wireless/ath/ath9k/main.c	2011-11-16 15:20:03.000000000 +0100
@@ -2485,6 +2485,7 @@ static int ath9k_get_antenna(struct ieee
 }
 
 struct ieee80211_ops ath9k_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx 		    = ath9k_tx,
 	.start 		    = ath9k_start,
 	.stop 		    = ath9k_stop,
--- a/drivers/net/wireless/ath/carl9170/main.c	2011-11-16 15:19:32.000000000 +0100
+++ b/drivers/net/wireless/ath/carl9170/main.c	2011-11-16 15:20:03.000000000 +0100
@@ -1680,6 +1680,7 @@ static bool carl9170_tx_frames_pending(s
 }
 
 static const struct ieee80211_ops carl9170_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.start			= carl9170_op_start,
 	.stop			= carl9170_op_stop,
 	.tx			= carl9170_op_tx,
--- a/drivers/net/wireless/b43/main.c	2011-11-16 15:19:31.000000000 +0100
+++ b/drivers/net/wireless/b43/main.c	2011-11-16 15:20:03.000000000 +0100
@@ -4915,6 +4915,7 @@ static int b43_op_get_survey(struct ieee
 }
 
 static const struct ieee80211_ops b43_hw_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= b43_op_tx,
 	.conf_tx		= b43_op_conf_tx,
 	.add_interface		= b43_op_add_interface,
--- a/drivers/net/wireless/b43legacy/main.c	2011-11-16 15:19:33.000000000 +0100
+++ b/drivers/net/wireless/b43legacy/main.c	2011-11-16 15:20:03.000000000 +0100
@@ -3490,6 +3490,7 @@ static int b43legacy_op_get_survey(struc
 }
 
 static const struct ieee80211_ops b43legacy_hw_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= b43legacy_op_tx,
 	.conf_tx		= b43legacy_op_conf_tx,
 	.add_interface		= b43legacy_op_add_interface,
--- a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c	2011-11-16 15:19:32.000000000 +0100
+++ b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c	2011-11-16 15:20:03.000000000 +0100
@@ -710,6 +710,7 @@ static void brcms_ops_flush(struct ieee8
 }
 
 static const struct ieee80211_ops brcms_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx = brcms_ops_tx,
 	.start = brcms_ops_start,
 	.stop = brcms_ops_stop,
--- a/drivers/net/wireless/iwlegacy/iwl-4965.c	2011-11-16 15:19:32.000000000 +0100
+++ b/drivers/net/wireless/iwlegacy/iwl-4965.c	2011-11-16 15:20:03.000000000 +0100
@@ -2112,6 +2112,7 @@ static const struct iwl_legacy_ops iwl49
 };
 
 struct ieee80211_ops iwl4965_hw_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx = iwl4965_mac_tx,
 	.start = iwl4965_mac_start,
 	.stop = iwl4965_mac_stop,
--- a/drivers/net/wireless/iwlegacy/iwl3945-base.c	2011-11-16 15:19:32.000000000 +0100
+++ b/drivers/net/wireless/iwlegacy/iwl3945-base.c	2011-11-16 15:20:03.000000000 +0100
@@ -3510,6 +3510,7 @@ static struct attribute_group iwl3945_at
 };
 
 struct ieee80211_ops iwl3945_hw_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx = iwl3945_mac_tx,
 	.start = iwl3945_mac_start,
 	.stop = iwl3945_mac_stop,
--- a/drivers/net/wireless/iwlwifi/iwl-mac80211.c	2011-11-16 15:19:31.000000000 +0100
+++ b/drivers/net/wireless/iwlwifi/iwl-mac80211.c	2011-11-16 15:20:03.000000000 +0100
@@ -1577,6 +1577,7 @@ static void iwlagn_mac_sta_notify(struct
 }
 
 struct ieee80211_ops iwlagn_hw_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx = iwlagn_mac_tx,
 	.start = iwlagn_mac_start,
 	.stop = iwlagn_mac_stop,
--- a/drivers/net/wireless/libertas_tf/main.c	2011-11-16 15:19:33.000000000 +0100
+++ b/drivers/net/wireless/libertas_tf/main.c	2011-11-16 15:20:03.000000000 +0100
@@ -543,6 +543,7 @@ static int lbtf_op_get_survey(struct iee
 }
 
 static const struct ieee80211_ops lbtf_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= lbtf_op_tx,
 	.start			= lbtf_op_start,
 	.stop			= lbtf_op_stop,
--- a/drivers/net/wireless/mac80211_hwsim.c	2011-11-16 15:19:31.000000000 +0100
+++ b/drivers/net/wireless/mac80211_hwsim.c	2011-11-16 15:20:03.000000000 +0100
@@ -1178,6 +1178,7 @@ static void mac80211_hwsim_sw_scan_compl
 
 static struct ieee80211_ops mac80211_hwsim_ops =
 {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx = mac80211_hwsim_tx,
 	.start = mac80211_hwsim_start,
 	.stop = mac80211_hwsim_stop,
--- a/drivers/net/wireless/mwl8k.c	2011-11-16 15:19:32.000000000 +0100
+++ b/drivers/net/wireless/mwl8k.c	2011-11-16 15:20:03.000000000 +0100
@@ -5082,6 +5082,7 @@ mwl8k_ampdu_action(struct ieee80211_hw *
 }
 
 static const struct ieee80211_ops mwl8k_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= mwl8k_tx,
 	.start			= mwl8k_start,
 	.stop			= mwl8k_stop,
--- a/drivers/net/wireless/p54/main.c	2011-11-16 15:19:33.000000000 +0100
+++ b/drivers/net/wireless/p54/main.c	2011-11-16 15:20:03.000000000 +0100
@@ -693,6 +693,7 @@ static void p54_set_coverage_class(struc
 }
 
 static const struct ieee80211_ops p54_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= p54_tx_80211,
 	.start			= p54_start,
 	.stop			= p54_stop,
--- a/drivers/net/wireless/rt2x00/rt2400pci.c	2011-11-16 15:19:32.000000000 +0100
+++ b/drivers/net/wireless/rt2x00/rt2400pci.c	2011-11-16 15:20:03.000000000 +0100
@@ -1699,6 +1699,7 @@ static int rt2400pci_tx_last_beacon(stru
 }
 
 static const struct ieee80211_ops rt2400pci_mac80211_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= rt2x00mac_tx,
 	.start			= rt2x00mac_start,
 	.stop			= rt2x00mac_stop,
--- a/drivers/net/wireless/rt2x00/rt2500pci.c	2011-11-16 15:19:31.000000000 +0100
+++ b/drivers/net/wireless/rt2x00/rt2500pci.c	2011-11-16 15:20:03.000000000 +0100
@@ -1991,6 +1991,7 @@ static int rt2500pci_tx_last_beacon(stru
 }
 
 static const struct ieee80211_ops rt2500pci_mac80211_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= rt2x00mac_tx,
 	.start			= rt2x00mac_start,
 	.stop			= rt2x00mac_stop,
--- a/drivers/net/wireless/rt2x00/rt2500usb.c	2011-11-16 15:19:32.000000000 +0100
+++ b/drivers/net/wireless/rt2x00/rt2500usb.c	2011-11-16 15:20:03.000000000 +0100
@@ -1808,6 +1808,7 @@ static int rt2500usb_probe_hw(struct rt2
 }
 
 static const struct ieee80211_ops rt2500usb_mac80211_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= rt2x00mac_tx,
 	.start			= rt2x00mac_start,
 	.stop			= rt2x00mac_stop,
--- a/drivers/net/wireless/rt2x00/rt2800pci.c	2011-11-16 15:19:31.000000000 +0100
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c	2011-11-16 15:20:03.000000000 +0100
@@ -1002,6 +1002,7 @@ static int rt2800pci_probe_hw(struct rt2
 }
 
 static const struct ieee80211_ops rt2800pci_mac80211_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= rt2x00mac_tx,
 	.start			= rt2x00mac_start,
 	.stop			= rt2x00mac_stop,
--- a/drivers/net/wireless/rt2x00/rt2800usb.c	2011-11-16 15:19:31.000000000 +0100
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c	2011-11-16 15:20:03.000000000 +0100
@@ -746,6 +746,7 @@ static int rt2800usb_probe_hw(struct rt2
 }
 
 static const struct ieee80211_ops rt2800usb_mac80211_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= rt2x00mac_tx,
 	.start			= rt2x00mac_start,
 	.stop			= rt2x00mac_stop,
--- a/drivers/net/wireless/rt2x00/rt61pci.c	2011-11-16 15:19:32.000000000 +0100
+++ b/drivers/net/wireless/rt2x00/rt61pci.c	2011-11-16 15:20:03.000000000 +0100
@@ -2956,6 +2956,7 @@ static u64 rt61pci_get_tsf(struct ieee80
 }
 
 static const struct ieee80211_ops rt61pci_mac80211_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= rt2x00mac_tx,
 	.start			= rt2x00mac_start,
 	.stop			= rt2x00mac_stop,
--- a/drivers/net/wireless/rt2x00/rt73usb.c	2011-11-16 15:19:32.000000000 +0100
+++ b/drivers/net/wireless/rt2x00/rt73usb.c	2011-11-16 15:20:03.000000000 +0100
@@ -2295,6 +2295,7 @@ static u64 rt73usb_get_tsf(struct ieee80
 }
 
 static const struct ieee80211_ops rt73usb_mac80211_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= rt2x00mac_tx,
 	.start			= rt2x00mac_start,
 	.stop			= rt2x00mac_stop,
--- a/drivers/net/wireless/rtl818x/rtl8180/dev.c	2011-11-16 15:19:33.000000000 +0100
+++ b/drivers/net/wireless/rtl818x/rtl8180/dev.c	2011-11-16 15:20:03.000000000 +0100
@@ -853,6 +853,7 @@ static void rtl8180_configure_filter(str
 }
 
 static const struct ieee80211_ops rtl8180_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= rtl8180_tx,
 	.start			= rtl8180_start,
 	.stop			= rtl8180_stop,
--- a/drivers/net/wireless/rtl818x/rtl8187/dev.c	2011-11-16 15:19:33.000000000 +0100
+++ b/drivers/net/wireless/rtl818x/rtl8187/dev.c	2011-11-16 15:20:03.000000000 +0100
@@ -1288,6 +1288,7 @@ static u64 rtl8187_get_tsf(struct ieee80
 }
 
 static const struct ieee80211_ops rtl8187_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= rtl8187_tx,
 	.start			= rtl8187_start,
 	.stop			= rtl8187_stop,
--- a/drivers/net/wireless/rtlwifi/core.c	2011-11-16 15:19:32.000000000 +0100
+++ b/drivers/net/wireless/rtlwifi/core.c	2011-11-16 15:20:03.000000000 +0100
@@ -1130,6 +1130,7 @@ static void rtl_op_flush(struct ieee8021
 }
 
 const struct ieee80211_ops rtl_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.start = rtl_op_start,
 	.stop = rtl_op_stop,
 	.tx = rtl_op_tx,
--- a/drivers/net/wireless/wl1251/main.c	2011-11-16 15:19:33.000000000 +0100
+++ b/drivers/net/wireless/wl1251/main.c	2011-11-16 15:20:03.000000000 +0100
@@ -1227,6 +1227,7 @@ static struct ieee80211_supported_band w
 };
 
 static const struct ieee80211_ops wl1251_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.start = wl1251_op_start,
 	.stop = wl1251_op_stop,
 	.add_interface = wl1251_op_add_interface,
--- a/drivers/net/wireless/wl12xx/main.c	2011-11-16 15:19:32.000000000 +0100
+++ b/drivers/net/wireless/wl12xx/main.c	2011-11-16 15:20:03.000000000 +0100
@@ -4406,6 +4406,7 @@ static const u8 *wl1271_band_rate_to_idx
 };
 
 static const struct ieee80211_ops wl1271_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.start = wl1271_op_start,
 	.stop = wl1271_op_stop,
 	.add_interface = wl1271_op_add_interface,
--- a/drivers/net/wireless/zd1211rw/zd_mac.c	2011-11-16 15:19:31.000000000 +0100
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c	2011-11-16 15:20:03.000000000 +0100
@@ -1339,6 +1339,7 @@ static u64 zd_op_get_tsf(struct ieee8021
 }
 
 static const struct ieee80211_ops zd_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= zd_op_tx,
 	.start			= zd_op_start,
 	.stop			= zd_op_stop,
--- a/drivers/staging/winbond/wbusb.c	2011-11-16 15:19:31.000000000 +0100
+++ b/drivers/staging/winbond/wbusb.c	2011-11-16 15:20:03.000000000 +0100
@@ -285,6 +285,7 @@ static u64 wbsoft_get_tsf(struct ieee802
 }
 
 static const struct ieee80211_ops wbsoft_ops = {
+	.tx_frags = ieee80211_generic_tx_frags,
 	.tx			= wbsoft_tx,
 	.start			= wbsoft_start,
 	.stop			= wbsoft_stop,



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] mac80211: transmit fragment list to drivers
  2011-11-16 14:28 ` [PATCH 4/4] mac80211: transmit fragment list to drivers Johannes Berg
@ 2011-11-16 14:40   ` Ivo Van Doorn
  2011-11-16 14:42     ` Johannes Berg
  2011-11-16 15:02   ` [PATCH v2 " Johannes Berg
  1 sibling, 1 reply; 9+ messages in thread
From: Ivo Van Doorn @ 2011-11-16 14:40 UTC (permalink / raw)
  To: Johannes Berg; +Cc: John Linville, linux-wireless

Hi,

> --- a/include/net/mac80211.h    2011-11-16 15:19:31.000000000 +0100
> +++ b/include/net/mac80211.h    2011-11-16 15:20:03.000000000 +0100
> @@ -1760,11 +1760,20 @@ enum ieee80211_frame_release_type {
>  *     skb contains the buffer starting from the IEEE 802.11 header.
>  *     The low-level driver should send the frame out based on
>  *     configuration in the TX control data. This handler should,
> - *     preferably, never fail and stop queues appropriately, more
> - *     importantly, however, it must never fail for A-MPDU-queues.
> - *     This function should return NETDEV_TX_OK except in very
> - *     limited cases.
> - *     Must be implemented and atomic.
> + *     preferably, never fail and stop queues appropriately.
> + *     This needs only be implemented if @tx_frags is set to the
> + *     ieee80211_generic_tx_frags handler.
> + *     Must be atomic.
> + *
> + * @tx_frags: Called to transmit multiple fragments of a single MSDU.
> + *     This handler must consume all fragments, sending out some of
> + *     them only is useless and it can't ask for some of them to be
> + *     queued again. If the frame is not fragmented the queue has a
> + *     single SKB only.
> + *     If this is called, the tx_info @vif and @sta pointer will be
> + *     invalid -- you must not use them in that case.
> + *     Must be implemented and atomic. If the driver implements @tx
> + *     only then this must be set to ieee80211_generic_tx_frags().

<..snip..>

> --- a/net/mac80211/driver-ops.h 2011-11-16 15:19:31.000000000 +0100
> +++ b/net/mac80211/driver-ops.h 2011-11-16 15:20:03.000000000 +0100
> @@ -15,6 +15,18 @@ static inline void drv_tx(struct ieee802
>        local->ops->tx(&local->hw, skb);
>  }
>
> +static inline void drv_tx_frags(struct ieee80211_local *local,
> +                               struct ieee80211_sub_if_data *sdata,
> +                               struct sta_info *sta,
> +                               struct sk_buff_head *skbs,
> +                               void *internal)
> +{
> +       local->ops->tx_frags(&local->hw,
> +                            sdata ? &sdata->vif : NULL,
> +                            sta ? &sta->sta : NULL,
> +                            skbs, internal);
> +}

Instead of changing every driver and making .tx_frags callback mandatory,
isn't it easier to have a if-else here?
Then it is much easier for the drivers as you can document that either
.tx or .tx_frags is mandatory but never both.

Ivo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] mac80211: transmit fragment list to drivers
  2011-11-16 14:40   ` Ivo Van Doorn
@ 2011-11-16 14:42     ` Johannes Berg
  2011-11-16 14:46       ` Ivo Van Doorn
  0 siblings, 1 reply; 9+ messages in thread
From: Johannes Berg @ 2011-11-16 14:42 UTC (permalink / raw)
  To: Ivo Van Doorn; +Cc: John Linville, linux-wireless

On Wed, 2011-11-16 at 15:40 +0100, Ivo Van Doorn wrote:

> > +static inline void drv_tx_frags(struct ieee80211_local *local,
> > +                               struct ieee80211_sub_if_data *sdata,
> > +                               struct sta_info *sta,
> > +                               struct sk_buff_head *skbs,
> > +                               void *internal)
> > +{
> > +       local->ops->tx_frags(&local->hw,
> > +                            sdata ? &sdata->vif : NULL,
> > +                            sta ? &sta->sta : NULL,
> > +                            skbs, internal);
> > +}
> 
> Instead of changing every driver and making .tx_frags callback mandatory,
> isn't it easier to have a if-else here?

We certainly can't have the if-else here, if there should be one it
should be in __ieee80211_tx().

> Then it is much easier for the drivers as you can document that either
> .tx or .tx_frags is mandatory but never both.

Maybe that's the better option. Somehow I thought it would be more
efficient this way, but I guess it doesn't actually make much of a
difference and I can get rid of the void *internal parameter.

I'll change.

johannes


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] mac80211: transmit fragment list to drivers
  2011-11-16 14:42     ` Johannes Berg
@ 2011-11-16 14:46       ` Ivo Van Doorn
  0 siblings, 0 replies; 9+ messages in thread
From: Ivo Van Doorn @ 2011-11-16 14:46 UTC (permalink / raw)
  To: Johannes Berg; +Cc: John Linville, linux-wireless

On Wed, Nov 16, 2011 at 3:42 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Wed, 2011-11-16 at 15:40 +0100, Ivo Van Doorn wrote:
>
>> > +static inline void drv_tx_frags(struct ieee80211_local *local,
>> > +                               struct ieee80211_sub_if_data *sdata,
>> > +                               struct sta_info *sta,
>> > +                               struct sk_buff_head *skbs,
>> > +                               void *internal)
>> > +{
>> > +       local->ops->tx_frags(&local->hw,
>> > +                            sdata ? &sdata->vif : NULL,
>> > +                            sta ? &sta->sta : NULL,
>> > +                            skbs, internal);
>> > +}
>>
>> Instead of changing every driver and making .tx_frags callback mandatory,
>> isn't it easier to have a if-else here?
>
> We certainly can't have the if-else here, if there should be one it
> should be in __ieee80211_tx().

Yeah, I was too lazy to search for the call to drv_tx_frags() itself. :)

>> Then it is much easier for the drivers as you can document that either
>> .tx or .tx_frags is mandatory but never both.
>
> Maybe that's the better option. Somehow I thought it would be more
> efficient this way, but I guess it doesn't actually make much of a
> difference and I can get rid of the void *internal parameter.
>
> I'll change.

Thanks,

Ivo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 4/4] mac80211: transmit fragment list to drivers
  2011-11-16 14:28 ` [PATCH 4/4] mac80211: transmit fragment list to drivers Johannes Berg
  2011-11-16 14:40   ` Ivo Van Doorn
@ 2011-11-16 15:02   ` Johannes Berg
  1 sibling, 0 replies; 9+ messages in thread
From: Johannes Berg @ 2011-11-16 15:02 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless, Ivo Van Doorn

From: Johannes Berg <johannes.berg@intel.com>

Drivers can usually handle fragmented packets
much easier when they get the entire list of
fragments at once. The only thing they need to
do is keep enough space on the queues for up
to ten fragments of a single MSDU.

This allows them to implement this with a new
operation tx_frags.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
v2: don't modify all drivers, thanks Ivo.

 include/net/mac80211.h    |   22 ++++++++--
 net/mac80211/driver-ops.h |    8 +++
 net/mac80211/main.c       |    2 
 net/mac80211/tx.c         |   94 +++++++++++++++++++++++++++++-----------------
 4 files changed, 86 insertions(+), 40 deletions(-)

--- a/include/net/mac80211.h	2011-11-16 15:43:47.000000000 +0100
+++ b/include/net/mac80211.h	2011-11-16 15:54:26.000000000 +0100
@@ -1760,11 +1760,21 @@ enum ieee80211_frame_release_type {
  *	skb contains the buffer starting from the IEEE 802.11 header.
  *	The low-level driver should send the frame out based on
  *	configuration in the TX control data. This handler should,
- *	preferably, never fail and stop queues appropriately, more
- *	importantly, however, it must never fail for A-MPDU-queues.
- *	This function should return NETDEV_TX_OK except in very
- *	limited cases.
- *	Must be implemented and atomic.
+ *	preferably, never fail and stop queues appropriately.
+ *	This must be implemented if @tx_frags is not.
+ *	Must be atomic.
+ *
+ * @tx_frags: Called to transmit multiple fragments of a single MSDU.
+ *	This handler must consume all fragments, sending out some of
+ *	them only is useless and it can't ask for some of them to be
+ *	queued again. If the frame is not fragmented the queue has a
+ *	single SKB only. To avoid issues with the networking stack
+ *	when TX status is reported the frames should be removed from
+ *	the skb queue.
+ *	If this is used, the tx_info @vif and @sta pointers will be
+ *	invalid -- you must not use them in that case.
+ *	This must be implemented if @tx isn't.
+ *	Must be atomic.
  *
  * @start: Called before the first netdevice attached to the hardware
  *	is enabled. This should turn on the hardware and must turn on
@@ -2101,6 +2111,8 @@ enum ieee80211_frame_release_type {
  */
 struct ieee80211_ops {
 	void (*tx)(struct ieee80211_hw *hw, struct sk_buff *skb);
+	void (*tx_frags)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
+			 struct ieee80211_sta *sta, struct sk_buff_head *skbs);
 	int (*start)(struct ieee80211_hw *hw);
 	void (*stop)(struct ieee80211_hw *hw);
 #ifdef CONFIG_PM
--- a/net/mac80211/driver-ops.h	2011-11-16 15:43:47.000000000 +0100
+++ b/net/mac80211/driver-ops.h	2011-11-16 15:49:51.000000000 +0100
@@ -15,6 +15,14 @@ static inline void drv_tx(struct ieee802
 	local->ops->tx(&local->hw, skb);
 }
 
+static inline void drv_tx_frags(struct ieee80211_local *local,
+				struct ieee80211_vif *vif,
+				struct ieee80211_sta *sta,
+				struct sk_buff_head *skbs)
+{
+	local->ops->tx_frags(&local->hw, vif, sta, skbs);
+}
+
 static inline int drv_start(struct ieee80211_local *local)
 {
 	int ret;
--- a/net/mac80211/tx.c	2011-11-16 15:43:47.000000000 +0100
+++ b/net/mac80211/tx.c	2011-11-16 15:50:35.000000000 +0100
@@ -1203,24 +1203,15 @@ ieee80211_tx_prepare(struct ieee80211_su
 	return TX_CONTINUE;
 }
 
-/*
- * Returns false if the frame couldn't be transmitted but was queued instead.
- */
-static bool __ieee80211_tx(struct ieee80211_local *local,
-			   struct sk_buff_head *skbs, int led_len,
-			   struct sta_info *sta, bool txpending)
+static bool ieee80211_tx_frags(struct ieee80211_local *local,
+			       struct ieee80211_vif *vif,
+			       struct ieee80211_sta *sta,
+			       struct sk_buff_head *skbs,
+			       bool txpending)
 {
 	struct sk_buff *skb, *tmp;
 	struct ieee80211_tx_info *info;
-	struct ieee80211_sub_if_data *sdata;
 	unsigned long flags;
-	__le16 fc;
-
-	if (WARN_ON(skb_queue_empty(skbs)))
-		return true;
-
-	skb = skb_peek(skbs);
-	fc = ((struct ieee80211_hdr *)skb->data)->frame_control;
 
 	skb_queue_walk_safe(skbs, skb, tmp) {
 		int q = skb_get_queue_mapping(skb);
@@ -1245,37 +1236,72 @@ static bool __ieee80211_tx(struct ieee80
 		spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
 
 		info = IEEE80211_SKB_CB(skb);
+		info->control.vif = vif;
+		info->control.sta = sta;
 
-		sdata = vif_to_sdata(info->control.vif);
+		__skb_unlink(skb, skbs);
+		drv_tx(local, skb);
+	}
 
-		switch (sdata->vif.type) {
-		case NL80211_IFTYPE_MONITOR:
-			info->control.vif = NULL;
-			break;
-		case NL80211_IFTYPE_AP_VLAN:
-			info->control.vif = &container_of(sdata->bss,
-				struct ieee80211_sub_if_data, u.ap)->vif;
-			break;
-		default:
-			/* keep */
-			break;
-		}
+	return true;
+}
 
-		if (sta && sta->uploaded)
-			info->control.sta = &sta->sta;
-		else
-			info->control.sta = NULL;
+/*
+ * Returns false if the frame couldn't be transmitted but was queued instead.
+ */
+static bool __ieee80211_tx(struct ieee80211_local *local,
+			   struct sk_buff_head *skbs, int led_len,
+			   struct sta_info *sta, bool txpending)
+{
+	struct ieee80211_tx_info *info;
+	struct ieee80211_sub_if_data *sdata;
+	struct ieee80211_vif *vif;
+	struct ieee80211_sta *pubsta;
+	struct sk_buff *skb;
+	bool result = true;
+	__le16 fc;
 
-		__skb_unlink(skb, skbs);
-		drv_tx(local, skb);
+	if (WARN_ON(skb_queue_empty(skbs)))
+		return true;
+
+	skb = skb_peek(skbs);
+	fc = ((struct ieee80211_hdr *)skb->data)->frame_control;
+	info = IEEE80211_SKB_CB(skb);
+	sdata = vif_to_sdata(info->control.vif);
+	if (sta && !sta->uploaded)
+		sta = NULL;
+
+	if (sta)
+		pubsta = &sta->sta;
+	else
+		pubsta = NULL;
+
+	switch (sdata->vif.type) {
+	case NL80211_IFTYPE_MONITOR:
+		sdata = NULL;
+		vif = NULL;
+		break;
+	case NL80211_IFTYPE_AP_VLAN:
+		sdata = container_of(sdata->bss,
+				     struct ieee80211_sub_if_data, u.ap);
+		/* fall through */
+	default:
+		vif = &sdata->vif;
+		break;
 	}
 
+	if (local->ops->tx_frags)
+		drv_tx_frags(local, vif, pubsta, skbs);
+	else
+		result = ieee80211_tx_frags(local, vif, pubsta, skbs,
+					    txpending);
+
 	ieee80211_tpt_led_trig_tx(local, fc, led_len);
 	ieee80211_led_tx(local, 1);
 
 	WARN_ON(!skb_queue_empty(skbs));
 
-	return true;
+	return result;
 }
 
 /*
--- a/net/mac80211/main.c	2011-11-15 14:02:07.000000000 +0100
+++ b/net/mac80211/main.c	2011-11-16 15:48:09.000000000 +0100
@@ -609,7 +609,7 @@ struct ieee80211_hw *ieee80211_alloc_hw(
 
 	local->hw.priv = (char *)local + ALIGN(sizeof(*local), NETDEV_ALIGN);
 
-	BUG_ON(!ops->tx);
+	BUG_ON(!ops->tx && !ops->tx_frags);
 	BUG_ON(!ops->start);
 	BUG_ON(!ops->stop);
 	BUG_ON(!ops->config);



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2011-11-16 15:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-16 14:28 [PATCH 0/4] submit fragmented packets to drivers at once Johannes Berg
2011-11-16 14:28 ` [PATCH 1/4] mac80211: use skb list for fragments Johannes Berg
2011-11-16 14:28 ` [PATCH 2/4] mac80211: move fragment flag adjustment Johannes Berg
2011-11-16 14:28 ` [PATCH 3/4] mac80211: make TX LED handling independent of fragmentation Johannes Berg
2011-11-16 14:28 ` [PATCH 4/4] mac80211: transmit fragment list to drivers Johannes Berg
2011-11-16 14:40   ` Ivo Van Doorn
2011-11-16 14:42     ` Johannes Berg
2011-11-16 14:46       ` Ivo Van Doorn
2011-11-16 15:02   ` [PATCH v2 " Johannes Berg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).