[PATCH 1/6] mac80211: minstrel_ht: add a small optimization to minstrel_aggr_check

[PATCH 1/6] mac80211: minstrel_ht: add a small optimization to minstrel_aggr_check

[Felix Fietkau]

Check the queue mapping earlier, skb->queue_mapping is more likely than
skb->data to still be in d-cache.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 net/mac80211/rc80211_minstrel_ht.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index c50fd94..62ff7cf 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -690,6 +690,9 @@ minstrel_aggr_check(struct ieee80211_sta *pubsta, struct sk_buff *skb)
 	struct sta_info *sta = container_of(pubsta, struct sta_info, sta);
 	u16 tid;
 
+	if (skb_get_queue_mapping(skb) == IEEE80211_AC_VO)
+		return;
+
 	if (unlikely(!ieee80211_is_data_qos(hdr->frame_control)))
 		return;
 
@@ -700,9 +703,6 @@ minstrel_aggr_check(struct ieee80211_sta *pubsta, struct sk_buff *skb)
 	if (likely(sta->ampdu_mlme.tid_tx[tid]))
 		return;
 
-	if (skb_get_queue_mapping(skb) == IEEE80211_AC_VO)
-		return;
-
 	ieee80211_start_tx_ba_session(pubsta, tid, 5000);
 }
 
-- 
2.1.2

[PATCH 2/6] mac80211: minstrel_ht: move aggregation check to .get_rate()

[Felix Fietkau]

Preparation for adding a no-skb tx status path

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 net/mac80211/rc80211_minstrel_ht.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index 62ff7cf..32b969f 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -782,9 +782,6 @@ minstrel_ht_tx_status(void *priv, struct ieee80211_supported_band *sband,
 	if (time_after(jiffies, mi->stats_update + (mp->update_interval / 2 * HZ) / 1000)) {
 		update = true;
 		minstrel_ht_update_stats(mp, mi);
-		if (!(info->flags & IEEE80211_TX_CTL_AMPDU) &&
-		    mi->max_prob_rate / MCS_GROUP_RATES != MINSTREL_CCK_GROUP)
-			minstrel_aggr_check(sta, skb);
 	}
 
 	if (update)
@@ -1026,6 +1023,10 @@ minstrel_ht_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
 	if (!msp->is_ht)
 		return mac80211_minstrel.get_rate(priv, sta, &msp->legacy, txrc);
 
+	if (!(info->flags & IEEE80211_TX_CTL_AMPDU) &&
+		mi->max_prob_rate / MCS_GROUP_RATES != MINSTREL_CCK_GROUP)
+		minstrel_aggr_check(sta, txrc->skb);
+
 	info->flags |= mi->tx_flags;
 	minstrel_ht_check_cck_shortpreamble(mp, mi, txrc->short_preamble);
 
-- 
2.1.2

[PATCH 3/6] mac80211: add tx_status_noskb to rate_control_ops

[Felix Fietkau]

This op works like .tx_status, except it does not need access to the
skb. This will be used by drivers that cannot match tx status
information to specific packets.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 include/net/mac80211.h | 4 ++++
 net/mac80211/rate.h    | 6 +++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 5f203a6..32a779c 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -4727,6 +4727,10 @@ struct rate_control_ops {
 	void (*free_sta)(void *priv, struct ieee80211_sta *sta,
 			 void *priv_sta);
 
+	void (*tx_status_noskb)(void *priv,
+				struct ieee80211_supported_band *sband,
+				struct ieee80211_sta *sta, void *priv_sta,
+				struct ieee80211_tx_info *info);
 	void (*tx_status)(void *priv, struct ieee80211_supported_band *sband,
 			  struct ieee80211_sta *sta, void *priv_sta,
 			  struct sk_buff *skb);
diff --git a/net/mac80211/rate.h b/net/mac80211/rate.h
index 18babe3..dd25964 100644
--- a/net/mac80211/rate.h
+++ b/net/mac80211/rate.h
@@ -37,11 +37,15 @@ static inline void rate_control_tx_status(struct ieee80211_local *local,
 	struct rate_control_ref *ref = local->rate_ctrl;
 	struct ieee80211_sta *ista = &sta->sta;
 	void *priv_sta = sta->rate_ctrl_priv;
+	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 
 	if (!ref || !test_sta_flag(sta, WLAN_STA_RATE_CONTROL))
 		return;
 
-	ref->ops->tx_status(ref->priv, sband, ista, priv_sta, skb);
+	if (ref->ops->tx_status)
+		ref->ops->tx_status(ref->priv, sband, ista, priv_sta, skb);
+	else
+		ref->ops->tx_status_noskb(ref->priv, sband, ista, priv_sta, info);
 }
 
 
-- 
2.1.2

[PATCH 4/6] mac80211: minstrel: switch to .tx_status_noskb

[Felix Fietkau]

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 net/mac80211/rc80211_minstrel.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
index c2b91bf..d51f6b1 100644
--- a/net/mac80211/rc80211_minstrel.c
+++ b/net/mac80211/rc80211_minstrel.c
@@ -223,11 +223,10 @@ minstrel_update_stats(struct minstrel_priv *mp, struct minstrel_sta_info *mi)
 static void
 minstrel_tx_status(void *priv, struct ieee80211_supported_band *sband,
 		   struct ieee80211_sta *sta, void *priv_sta,
-		   struct sk_buff *skb)
+		   struct ieee80211_tx_info *info)
 {
 	struct minstrel_priv *mp = priv;
 	struct minstrel_sta_info *mi = priv_sta;
-	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	struct ieee80211_tx_rate *ar = info->status.rates;
 	int i, ndx;
 	int success;
@@ -674,7 +673,7 @@ static u32 minstrel_get_expected_throughput(void *priv_sta)
 
 const struct rate_control_ops mac80211_minstrel = {
 	.name = "minstrel",
-	.tx_status = minstrel_tx_status,
+	.tx_status_noskb = minstrel_tx_status,
 	.get_rate = minstrel_get_rate,
 	.rate_init = minstrel_rate_init,
 	.alloc = minstrel_alloc,
-- 
2.1.2

[PATCH 5/6] mac80211: minstrel_ht: switch to .tx_status_noskb

[Felix Fietkau]

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 net/mac80211/rc80211_minstrel_ht.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index 32b969f..3775b06 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -709,11 +709,10 @@ minstrel_aggr_check(struct ieee80211_sta *pubsta, struct sk_buff *skb)
 static void
 minstrel_ht_tx_status(void *priv, struct ieee80211_supported_band *sband,
                       struct ieee80211_sta *sta, void *priv_sta,
-                      struct sk_buff *skb)
+                      struct ieee80211_tx_info *info)
 {
 	struct minstrel_ht_sta_priv *msp = priv_sta;
 	struct minstrel_ht_sta *mi = &msp->ht;
-	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	struct ieee80211_tx_rate *ar = info->status.rates;
 	struct minstrel_rate_stats *rate, *rate2;
 	struct minstrel_priv *mp = priv;
@@ -721,7 +720,8 @@ minstrel_ht_tx_status(void *priv, struct ieee80211_supported_band *sband,
 	int i;
 
 	if (!msp->is_ht)
-		return mac80211_minstrel.tx_status(priv, sband, sta, &msp->legacy, skb);
+		return mac80211_minstrel.tx_status_noskb(priv, sband, sta,
+							 &msp->legacy, info);
 
 	/* This packet was aggregated but doesn't carry status info */
 	if ((info->flags & IEEE80211_TX_CTL_AMPDU) &&
@@ -1343,7 +1343,7 @@ static u32 minstrel_ht_get_expected_throughput(void *priv_sta)
 
 static const struct rate_control_ops mac80211_minstrel_ht = {
 	.name = "minstrel_ht",
-	.tx_status = minstrel_ht_tx_status,
+	.tx_status_noskb = minstrel_ht_tx_status,
 	.get_rate = minstrel_ht_get_rate,
 	.rate_init = minstrel_ht_rate_init,
 	.rate_update = minstrel_ht_rate_update,
-- 
2.1.2

[PATCH 6/6] mac80211: add ieee80211_tx_status_noskb

[Felix Fietkau]

This can be used by drivers that cannot reliably map tx status
information onto specific skbs.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 include/net/mac80211.h |  22 ++++++++++
 net/mac80211/rate.h    |  17 ++++++++
 net/mac80211/status.c  | 116 +++++++++++++++++++++++++++++++++++++++----------
 3 files changed, 132 insertions(+), 23 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 32a779c..28ea999 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -3517,6 +3517,28 @@ void ieee80211_tx_status(struct ieee80211_hw *hw,
 			 struct sk_buff *skb);
 
 /**
+ * ieee80211_tx_status_noskb - transmit status callback without skb
+ *
+ * This function can be used as a replacement for ieee80211_tx_status
+ * in drivers that cannot reliably map tx status information back to
+ * specific skbs.
+ *
+ * This function may not be called in IRQ context. Calls to this function
+ * for a single hardware must be synchronized against each other. Calls
+ * to this function, ieee80211_tx_status_ni() and ieee80211_tx_status_irqsafe()
+ * may not be mixed for a single hardware. Must not run concurrently with
+ * ieee80211_rx() or ieee80211_rx_ni().
+ *
+ * @hw: the hardware the frame was transmitted by
+ * @sta: the receiver station to which this packet is sent
+ *	(NULL for multicast packets)
+ * @info: tx status information
+ */
+void ieee80211_tx_status_noskb(struct ieee80211_hw *hw,
+			       struct ieee80211_sta *sta,
+			       struct ieee80211_tx_info *info);
+
+/**
  * ieee80211_tx_status_ni - transmit status callback (in process context)
  *
  * Like ieee80211_tx_status() but can be called in process context.
diff --git a/net/mac80211/rate.h b/net/mac80211/rate.h
index dd25964..a0b18a8 100644
--- a/net/mac80211/rate.h
+++ b/net/mac80211/rate.h
@@ -49,6 +49,23 @@ static inline void rate_control_tx_status(struct ieee80211_local *local,
 }
 
 
+static inline void
+rate_control_tx_status_noskb(struct ieee80211_local *local,
+			     struct ieee80211_supported_band *sband,
+			     struct sta_info *sta,
+			     struct ieee80211_tx_info *info)
+{
+	struct rate_control_ref *ref = local->rate_ctrl;
+	struct ieee80211_sta *ista = &sta->sta;
+	void *priv_sta = sta->rate_ctrl_priv;
+
+	if (!ref || !test_sta_flag(sta, WLAN_STA_RATE_CONTROL))
+		return;
+
+	ref->ops->tx_status_noskb(ref->priv, sband, ista, priv_sta, info);
+}
+
+
 static inline void rate_control_rate_init(struct sta_info *sta)
 {
 	struct ieee80211_local *local = sta->sdata->local;
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 9612d89..8a6d3ad 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -541,10 +541,9 @@ static void ieee80211_tx_latency_end_msrmnt(struct ieee80211_local *local,
 #define STA_LOST_TDLS_PKT_THRESHOLD	10
 #define STA_LOST_TDLS_PKT_TIME		(10*HZ) /* 10secs since last ACK */
 
-static void ieee80211_lost_packet(struct sta_info *sta, struct sk_buff *skb)
+static void ieee80211_lost_packet(struct sta_info *sta,
+				  struct ieee80211_tx_info *info)
 {
-	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
-
 	/* This packet was aggregated but doesn't carry status info */
 	if ((info->flags & IEEE80211_TX_CTL_AMPDU) &&
 	    !(info->flags & IEEE80211_TX_STAT_AMPDU))
@@ -571,24 +570,13 @@ static void ieee80211_lost_packet(struct sta_info *sta, struct sk_buff *skb)
 	sta->lost_packets = 0;
 }
 
-void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
+static int ieee80211_tx_get_rates(struct ieee80211_hw *hw,
+				  struct ieee80211_tx_info *info,
+				  int *retry_count)
 {
-	struct sk_buff *skb2;
-	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
-	struct ieee80211_local *local = hw_to_local(hw);
-	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
-	__le16 fc;
-	struct ieee80211_supported_band *sband;
-	struct ieee80211_sub_if_data *sdata;
-	struct net_device *prev_dev = NULL;
-	struct sta_info *sta, *tmp;
-	int retry_count = -1, i;
 	int rates_idx = -1;
-	bool send_to_cooked;
-	bool acked;
-	struct ieee80211_bar *bar;
-	int rtap_len;
-	int shift = 0;
+	int count = -1;
+	int i;
 
 	for (i = 0; i < IEEE80211_TX_MAX_RATES; i++) {
 		if ((info->flags & IEEE80211_TX_CTL_AMPDU) &&
@@ -606,12 +594,94 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
 			break;
 		}
 
-		retry_count += info->status.rates[i].count;
+		count += info->status.rates[i].count;
 	}
 	rates_idx = i - 1;
 
-	if (retry_count < 0)
-		retry_count = 0;
+	if (count < 0)
+		count = 0;
+
+	*retry_count = count;
+	return rates_idx;
+}
+
+void ieee80211_tx_status_noskb(struct ieee80211_hw *hw,
+			       struct ieee80211_sta *pubsta,
+			       struct ieee80211_tx_info *info)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+	struct ieee80211_supported_band *sband;
+	int retry_count;
+	int rates_idx;
+	bool acked;
+
+	rates_idx = ieee80211_tx_get_rates(hw, info, &retry_count);
+
+	sband = hw->wiphy->bands[info->band];
+
+	acked = !!(info->flags & IEEE80211_TX_STAT_ACK);
+	if (pubsta) {
+		struct sta_info *sta;
+
+		sta = container_of(pubsta, struct sta_info, sta);
+
+		if (info->flags & IEEE80211_TX_STATUS_EOSP)
+			clear_sta_flag(sta, WLAN_STA_SP);
+
+		if (!acked)
+			sta->tx_retry_failed++;
+		sta->tx_retry_count += retry_count;
+
+		if (acked) {
+			sta->last_rx = jiffies;
+
+			if (sta->lost_packets)
+				sta->lost_packets = 0;
+
+			/* Track when last TDLS packet was ACKed */
+			if (test_sta_flag(sta, WLAN_STA_TDLS_PEER_AUTH))
+				sta->last_tdls_pkt_time = jiffies;
+		} else {
+			ieee80211_lost_packet(sta, info);
+		}
+
+		rate_control_tx_status_noskb(local, sband, sta, info);
+	}
+
+	if (acked) {
+		    local->dot11TransmittedFrameCount++;
+		    if (!pubsta)
+			    local->dot11MulticastTransmittedFrameCount++;
+		    if (retry_count > 0)
+			    local->dot11RetryCount++;
+		    if (retry_count > 1)
+			    local->dot11MultipleRetryCount++;
+	} else {
+		local->dot11FailedCount++;
+	}
+}
+EXPORT_SYMBOL(ieee80211_tx_status_noskb);
+
+void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
+{
+	struct sk_buff *skb2;
+	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+	struct ieee80211_local *local = hw_to_local(hw);
+	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+	__le16 fc;
+	struct ieee80211_supported_band *sband;
+	struct ieee80211_sub_if_data *sdata;
+	struct net_device *prev_dev = NULL;
+	struct sta_info *sta, *tmp;
+	int retry_count;
+	int rates_idx;
+	bool send_to_cooked;
+	bool acked;
+	struct ieee80211_bar *bar;
+	int rtap_len;
+	int shift = 0;
+
+	rates_idx = ieee80211_tx_get_rates(hw, info, &retry_count);
 
 	rcu_read_lock();
 
@@ -716,7 +786,7 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
 				if (test_sta_flag(sta, WLAN_STA_TDLS_PEER_AUTH))
 					sta->last_tdls_pkt_time = jiffies;
 			} else {
-				ieee80211_lost_packet(sta, skb);
+				ieee80211_lost_packet(sta, info);
 			}
 		}
 
-- 
2.1.2

Re: [PATCH 1/6] mac80211: minstrel_ht: add a small optimization to minstrel_aggr_check

[Johannes Berg]

On Sun, 2014-11-16 at 00:27 +0100, Felix Fietkau wrote:
> Check the queue mapping earlier, skb->queue_mapping is more likely than
> skb->data to still be in d-cache.

Applied.

johannes

Re: [PATCH 2/6] mac80211: minstrel_ht: move aggregation check to .get_rate()

[Johannes Berg]

On Sun, 2014-11-16 at 00:27 +0100, Felix Fietkau wrote:

> @@ -1026,6 +1023,10 @@ minstrel_ht_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
>  	if (!msp->is_ht)
>  		return mac80211_minstrel.get_rate(priv, sta, &msp->legacy, txrc);
>  
> +	if (!(info->flags & IEEE80211_TX_CTL_AMPDU) &&
> +		mi->max_prob_rate / MCS_GROUP_RATES != MINSTREL_CCK_GROUP)
> +		minstrel_aggr_check(sta, txrc->skb);

Indentation is now broken.

johannes

Re: [PATCH 3/6] mac80211: add tx_status_noskb to rate_control_ops

[Johannes Berg]

> +++ b/net/mac80211/rate.h
> @@ -37,11 +37,15 @@ static inline void rate_control_tx_status(struct ieee80211_local *local,
>  	struct rate_control_ref *ref = local->rate_ctrl;
>  	struct ieee80211_sta *ista = &sta->sta;
>  	void *priv_sta = sta->rate_ctrl_priv;
> +	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
>  
>  	if (!ref || !test_sta_flag(sta, WLAN_STA_RATE_CONTROL))
>  		return;
>  
> -	ref->ops->tx_status(ref->priv, sband, ista, priv_sta, skb);
> +	if (ref->ops->tx_status)
> +		ref->ops->tx_status(ref->priv, sband, ista, priv_sta, skb);
> +	else
> +		ref->ops->tx_status_noskb(ref->priv, sband, ista, priv_sta, info);

I can't say I like this - you're going to have to pass NULL or something
as the SKB pointer, and then rely on having a rate control algorithm
that deals with it, etc....

johannes

Re: [PATCH 6/6] mac80211: add ieee80211_tx_status_noskb

[Johannes Berg]

>  /**
> + * ieee80211_tx_status_noskb - transmit status callback without skb
> + *
> + * This function can be used as a replacement for ieee80211_tx_status
> + * in drivers that cannot reliably map tx status information back to
> + * specific skbs.
> + *
> + * This function may not be called in IRQ context. Calls to this function
> + * for a single hardware must be synchronized against each other. Calls
> + * to this function, ieee80211_tx_status_ni() and ieee80211_tx_status_irqsafe()
> + * may not be mixed for a single hardware. Must not run concurrently with
> + * ieee80211_rx() or ieee80211_rx_ni().

None of that seems very likely. Did you just copy/paste it? :)


> +static inline void
> +rate_control_tx_status_noskb(struct ieee80211_local *local,
> +			     struct ieee80211_supported_band *sband,
> +			     struct sta_info *sta,
> +			     struct ieee80211_tx_info *info)
> +{
> +	struct rate_control_ref *ref = local->rate_ctrl;
> +	struct ieee80211_sta *ista = &sta->sta;
> +	void *priv_sta = sta->rate_ctrl_priv;
> +
> +	if (!ref || !test_sta_flag(sta, WLAN_STA_RATE_CONTROL))
> +		return;
> +
> +	ref->ops->tx_status_noskb(ref->priv, sband, ista, priv_sta, info);
> +}

Oh, so you're adding another one ... I guess I understand better now.

> +
> +

two blank lines?

> -static void ieee80211_lost_packet(struct sta_info *sta, struct sk_buff *skb)
> +static void ieee80211_lost_packet(struct sta_info *sta,
> +				  struct ieee80211_tx_info *info)
>  {
> -	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> -

some of this refactoring might better be in a separate patch.

>  	/* This packet was aggregated but doesn't carry status info */
>  	if ((info->flags & IEEE80211_TX_CTL_AMPDU) &&
>  	    !(info->flags & IEEE80211_TX_STAT_AMPDU))
> @@ -571,24 +570,13 @@ static void ieee80211_lost_packet(struct sta_info *sta, struct sk_buff *skb)
>  	sta->lost_packets = 0;
>  }
>  
> -void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
> +static int ieee80211_tx_get_rates(struct ieee80211_hw *hw,
> +				  struct ieee80211_tx_info *info,
> +				  int *retry_count)
>  {
> -	struct sk_buff *skb2;
> -	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
> -	struct ieee80211_local *local = hw_to_local(hw);
> -	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> -	__le16 fc;
> -	struct ieee80211_supported_band *sband;
> -	struct ieee80211_sub_if_data *sdata;
> -	struct net_device *prev_dev = NULL;
> -	struct sta_info *sta, *tmp;
> -	int retry_count = -1, i;
>  	int rates_idx = -1;
> -	bool send_to_cooked;
> -	bool acked;
> -	struct ieee80211_bar *bar;
> -	int rtap_len;
> -	int shift = 0;
> +	int count = -1;
> +	int i;

ditto - too big for here.

> +	acked = !!(info->flags & IEEE80211_TX_STAT_ACK);
> +	if (pubsta) {
> +		struct sta_info *sta;
> +
> +		sta = container_of(pubsta, struct sta_info, sta);
> +
> +		if (info->flags & IEEE80211_TX_STATUS_EOSP)
> +			clear_sta_flag(sta, WLAN_STA_SP);

That doesn't seem reasonable really - if you're reporting out of band
then don't report it as TX status but rather with the eosp() call.

johannes

Re: [PATCH 6/6] mac80211: add ieee80211_tx_status_noskb

[Felix Fietkau]

On 2014-11-19 19:38, Johannes Berg wrote:
> 
>>  /**
>> + * ieee80211_tx_status_noskb - transmit status callback without skb
>> + *
>> + * This function can be used as a replacement for ieee80211_tx_status
>> + * in drivers that cannot reliably map tx status information back to
>> + * specific skbs.
>> + *
>> + * This function may not be called in IRQ context. Calls to this function
>> + * for a single hardware must be synchronized against each other. Calls
>> + * to this function, ieee80211_tx_status_ni() and ieee80211_tx_status_irqsafe()
>> + * may not be mixed for a single hardware. Must not run concurrently with
>> + * ieee80211_rx() or ieee80211_rx_ni().
> 
> None of that seems very likely. Did you just copy/paste it? :)
Yes, I copy/pasted it. I wasn't sure if these requirements would be
necessary for the no-skb status as well, just figured it'd be safe to
leave them in.

>> +static inline void
>> +rate_control_tx_status_noskb(struct ieee80211_local *local,
>> +			     struct ieee80211_supported_band *sband,
>> +			     struct sta_info *sta,
>> +			     struct ieee80211_tx_info *info)
>> +{
>> +	struct rate_control_ref *ref = local->rate_ctrl;
>> +	struct ieee80211_sta *ista = &sta->sta;
>> +	void *priv_sta = sta->rate_ctrl_priv;
>> +
>> +	if (!ref || !test_sta_flag(sta, WLAN_STA_RATE_CONTROL))
>> +		return;
>> +
>> +	ref->ops->tx_status_noskb(ref->priv, sband, ista, priv_sta, info);
>> +}
> 
> Oh, so you're adding another one ... I guess I understand better now.
> 
>> +
>> +
> 
> two blank lines?
Will fix that.

>> -static void ieee80211_lost_packet(struct sta_info *sta, struct sk_buff *skb)
>> +static void ieee80211_lost_packet(struct sta_info *sta,
>> +				  struct ieee80211_tx_info *info)
>>  {
>> -	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
>> -
> 
> some of this refactoring might better be in a separate patch.
> 
>>  	/* This packet was aggregated but doesn't carry status info */
>>  	if ((info->flags & IEEE80211_TX_CTL_AMPDU) &&
>>  	    !(info->flags & IEEE80211_TX_STAT_AMPDU))
>> @@ -571,24 +570,13 @@ static void ieee80211_lost_packet(struct sta_info *sta, struct sk_buff *skb)
>>  	sta->lost_packets = 0;
>>  }
>>  
>> -void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
>> +static int ieee80211_tx_get_rates(struct ieee80211_hw *hw,
>> +				  struct ieee80211_tx_info *info,
>> +				  int *retry_count)
>>  {
>> -	struct sk_buff *skb2;
>> -	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
>> -	struct ieee80211_local *local = hw_to_local(hw);
>> -	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
>> -	__le16 fc;
>> -	struct ieee80211_supported_band *sband;
>> -	struct ieee80211_sub_if_data *sdata;
>> -	struct net_device *prev_dev = NULL;
>> -	struct sta_info *sta, *tmp;
>> -	int retry_count = -1, i;
>>  	int rates_idx = -1;
>> -	bool send_to_cooked;
>> -	bool acked;
>> -	struct ieee80211_bar *bar;
>> -	int rtap_len;
>> -	int shift = 0;
>> +	int count = -1;
>> +	int i;
> 
> ditto - too big for here.
OK.

>> +	acked = !!(info->flags & IEEE80211_TX_STAT_ACK);
>> +	if (pubsta) {
>> +		struct sta_info *sta;
>> +
>> +		sta = container_of(pubsta, struct sta_info, sta);
>> +
>> +		if (info->flags & IEEE80211_TX_STATUS_EOSP)
>> +			clear_sta_flag(sta, WLAN_STA_SP);
> 
> That doesn't seem reasonable really - if you're reporting out of band
> then don't report it as TX status but rather with the eosp() call.
OK.

- Felix

Re: [PATCH 6/6] mac80211: add ieee80211_tx_status_noskb

[Johannes Berg]

On Wed, 2014-11-19 at 19:47 +0100, Felix Fietkau wrote:

> >> + * This function may not be called in IRQ context. Calls to this function
> >> + * for a single hardware must be synchronized against each other. Calls
> >> + * to this function, ieee80211_tx_status_ni() and ieee80211_tx_status_irqsafe()
> >> + * may not be mixed for a single hardware. Must not run concurrently with
> >> + * ieee80211_rx() or ieee80211_rx_ni().
> > 
> > None of that seems very likely. Did you just copy/paste it? :)
> Yes, I copy/pasted it. I wasn't sure if these requirements would be
> necessary for the no-skb status as well, just figured it'd be safe to
> leave them in.

I think if you move the eosp() that leaves you pretty much with only
things that are safe? 

johannes