[PATCH 1/4] mac80211: Move IEEE80211_TX_CTL_PS_RESPONSE

[PATCH 1/4] mac80211: Move IEEE80211_TX_CTL_PS_RESPONSE

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

Move IEEE80211_TX_CTL_PS_RESPONSE to info->control.flags since
this is used only in the TX path (by ath9k). This frees up
a bit which can be used for other purposes.

Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath9k/xmit.c | 6 ++++--
 include/net/mac80211.h                | 6 +++---
 net/mac80211/sta_info.c               | 7 ++++---
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index e9bd02c..4caee66 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -2259,7 +2259,7 @@ int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
 	struct ath_txq *txq = txctl->txq;
 	struct ath_atx_tid *tid = NULL;
 	struct ath_buf *bf;
-	bool queue, skip_uapsd = false;
+	bool queue, skip_uapsd = false, ps_resp;
 	int q, ret;
 
 	if (vif)
@@ -2268,6 +2268,8 @@ int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
 	if (info->flags & IEEE80211_TX_CTL_TX_OFFCHAN)
 		txctl->force_channel = true;
 
+	ps_resp = !!(info->control.flags & IEEE80211_TX_CTRL_PS_RESPONSE);
+
 	ret = ath_tx_prepare(hw, skb, txctl);
 	if (ret)
 	    return ret;
@@ -2310,7 +2312,7 @@ int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
 	if (txctl->an && queue)
 		tid = ath_get_skb_tid(sc, txctl->an, skb);
 
-	if (!skip_uapsd && (info->flags & IEEE80211_TX_CTL_PS_RESPONSE)) {
+	if (!skip_uapsd && ps_resp) {
 		ath_txq_unlock(sc, txq);
 		txq = sc->tx.uapsdq;
 		ath_txq_lock(sc, txq);
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 58d719d..b36e60d 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -505,8 +505,6 @@ struct ieee80211_bss_conf {
  * @IEEE80211_TX_CTL_DONTFRAG: Don't fragment this packet even if it
  *	would be fragmented by size (this is optional, only used for
  *	monitor injection).
- * @IEEE80211_TX_CTL_PS_RESPONSE: This frame is a response to a poll
- *	frame (PS-Poll or uAPSD).
  *
  * Note: If you have to add new flags to the enumeration, then don't
  *	 forget to update %IEEE80211_TX_TEMPORARY_FLAGS when necessary.
@@ -542,7 +540,6 @@ enum mac80211_tx_info_flags {
 	IEEE80211_TX_STATUS_EOSP		= BIT(28),
 	IEEE80211_TX_CTL_USE_MINRATE		= BIT(29),
 	IEEE80211_TX_CTL_DONTFRAG		= BIT(30),
-	IEEE80211_TX_CTL_PS_RESPONSE		= BIT(31),
 };
 
 #define IEEE80211_TX_CTL_STBC_SHIFT		23
@@ -552,11 +549,14 @@ enum mac80211_tx_info_flags {
  *
  * @IEEE80211_TX_CTRL_PORT_CTRL_PROTO: this frame is a port control
  *	protocol frame (e.g. EAP)
+ * @IEEE80211_TX_CTRL_PS_RESPONSE: This frame is a response to a poll
+ *	frame (PS-Poll or uAPSD).
  *
  * These flags are used in tx_info->control.flags.
  */
 enum mac80211_tx_control_flags {
 	IEEE80211_TX_CTRL_PORT_CTRL_PROTO	= BIT(0),
+	IEEE80211_TX_CTRL_PS_RESPONSE		= BIT(1),
 };
 
 /*
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index a42f5b2..db8b07a 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1243,10 +1243,11 @@ static void ieee80211_send_null_response(struct ieee80211_sub_if_data *sdata,
 	 * ends the poll/service period.
 	 */
 	info->flags |= IEEE80211_TX_CTL_NO_PS_BUFFER |
-		       IEEE80211_TX_CTL_PS_RESPONSE |
 		       IEEE80211_TX_STATUS_EOSP |
 		       IEEE80211_TX_CTL_REQ_TX_STATUS;
 
+	info->control.flags |= IEEE80211_TX_CTRL_PS_RESPONSE;
+
 	if (call_driver)
 		drv_allow_buffered_frames(local, sta, BIT(tid), 1,
 					  reason, false);
@@ -1395,8 +1396,8 @@ ieee80211_sta_ps_deliver_response(struct sta_info *sta,
 			 * STA may still remain is PS mode after this frame
 			 * exchange.
 			 */
-			info->flags |= IEEE80211_TX_CTL_NO_PS_BUFFER |
-				       IEEE80211_TX_CTL_PS_RESPONSE;
+			info->flags |= IEEE80211_TX_CTL_NO_PS_BUFFER;
+			info->control.flags |= IEEE80211_TX_CTRL_PS_RESPONSE;
 
 			/*
 			 * Use MoreData flag to indicate whether there are
-- 
2.1.3

[PATCH 2/4] mac80211: Fix accounting of multicast frames

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

Since multicast frames are marked as no-ack, using
IEEE80211_TX_STAT_ACK to check if they have been
successfully transmitted by the driver is incorrect
since a driver can choose to ignore transmission status
for no-ack frames. This results in incorrect accounting
for such frames.

To fix this issue, this patch introduces a new flag
that can be used by drivers to indicate error-free
transmission of no-ack frames.

Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
---
 include/net/mac80211.h | 4 ++++
 net/mac80211/status.c  | 9 ++++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index b36e60d..3c15440 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -505,6 +505,9 @@ struct ieee80211_bss_conf {
  * @IEEE80211_TX_CTL_DONTFRAG: Don't fragment this packet even if it
  *	would be fragmented by size (this is optional, only used for
  *	monitor injection).
+ * @IEEE80211_TX_STAT_NOACK_TRANSMITTED: A frame that was marked with
+ *	IEEE80211_TX_CTL_NO_ACK has been successfully transmitted without
+ *	any errors (like issues specific to the driver/HW).
  *
  * Note: If you have to add new flags to the enumeration, then don't
  *	 forget to update %IEEE80211_TX_TEMPORARY_FLAGS when necessary.
@@ -540,6 +543,7 @@ enum mac80211_tx_info_flags {
 	IEEE80211_TX_STATUS_EOSP		= BIT(28),
 	IEEE80211_TX_CTL_USE_MINRATE		= BIT(29),
 	IEEE80211_TX_CTL_DONTFRAG		= BIT(30),
+	IEEE80211_TX_STAT_NOACK_TRANSMITTED	= BIT(31),
 };
 
 #define IEEE80211_TX_CTL_STBC_SHIFT		23
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index bb146f3..d64037c 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -664,13 +664,15 @@ void ieee80211_tx_status_noskb(struct ieee80211_hw *hw,
 	struct ieee80211_supported_band *sband;
 	int retry_count;
 	int rates_idx;
-	bool acked;
+	bool acked, noack_success;
 
 	rates_idx = ieee80211_tx_get_rates(hw, info, &retry_count);
 
 	sband = hw->wiphy->bands[info->band];
 
 	acked = !!(info->flags & IEEE80211_TX_STAT_ACK);
+	noack_success = !!(info->flags & IEEE80211_TX_STAT_NOACK_TRANSMITTED);
+
 	if (pubsta) {
 		struct sta_info *sta;
 
@@ -696,7 +698,7 @@ void ieee80211_tx_status_noskb(struct ieee80211_hw *hw,
 		rate_control_tx_status_noskb(local, sband, sta, info);
 	}
 
-	if (acked) {
+	if (acked || noack_success) {
 		    local->dot11TransmittedFrameCount++;
 		    if (!pubsta)
 			    local->dot11MulticastTransmittedFrameCount++;
@@ -856,7 +858,8 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
 	 * Fragments are passed to low-level drivers as separate skbs, so these
 	 * are actually fragments, not frames. Update frame counters only for
 	 * the first fragment of the frame. */
-	if (info->flags & IEEE80211_TX_STAT_ACK) {
+	if ((info->flags & IEEE80211_TX_STAT_ACK) ||
+	    (info->flags & IEEE80211_TX_STAT_NOACK_TRANSMITTED)) {
 		if (ieee80211_is_first_frag(hdr->seq_ctrl)) {
 			local->dot11TransmittedFrameCount++;
 			if (is_multicast_ether_addr(hdr->addr1))
-- 
2.1.3

Re: [PATCH 2/4] mac80211: Fix accounting of multicast frames

[Johannes Berg]

On Wed, 2014-12-10 at 21:26 +0530, Sujith Manoharan wrote:
> From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> 
> Since multicast frames are marked as no-ack, using
> IEEE80211_TX_STAT_ACK to check if they have been
> successfully transmitted by the driver is incorrect
> since a driver can choose to ignore transmission status
> for no-ack frames. This results in incorrect accounting
> for such frames.
> 
> To fix this issue, this patch introduces a new flag
> that can be used by drivers to indicate error-free
> transmission of no-ack frames.

Seems fine, applied. I've worded the documentation a bit more strongly.

johannes

Re: [PATCH 2/4] mac80211: Fix accounting of multicast frames

[Sujith Manoharan]

Johannes Berg wrote:
> Seems fine, applied. I've worded the documentation a bit more strongly.

Thanks.

Sujith

[PATCH 3/4] ath10k: Fix no-ack frame status

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

Use the new IEEE80211_TX_STAT_NOACK_TRANSMITTED flag
to indicate successful transmission of no-ack frames.
This fixes multicast frame accounting.

Cc: ath10k@lists.infradead.org
Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/htt.h    | 1 +
 drivers/net/wireless/ath/ath10k/htt_rx.c | 2 ++
 drivers/net/wireless/ath/ath10k/txrx.c   | 9 +++++++--
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 1bd5545..cb74211 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1159,6 +1159,7 @@ struct htt_tx_done {
 	u32 msdu_id;
 	bool discard;
 	bool no_ack;
+	bool success;
 };
 
 struct htt_peer_map_event {
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 9c782a4..d4d082e 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1476,6 +1476,7 @@ static void ath10k_htt_rx_frm_tx_compl(struct ath10k *ar,
 		tx_done.no_ack = true;
 		break;
 	case HTT_DATA_TX_STATUS_OK:
+		tx_done.success = true;
 		break;
 	case HTT_DATA_TX_STATUS_DISCARD:
 	case HTT_DATA_TX_STATUS_POSTPONE:
@@ -1627,6 +1628,7 @@ void ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 
 		switch (status) {
 		case HTT_MGMT_TX_STATUS_OK:
+			tx_done.success = true;
 			break;
 		case HTT_MGMT_TX_STATUS_RETRY:
 			tx_done.no_ack = true;
diff --git a/drivers/net/wireless/ath/ath10k/txrx.c b/drivers/net/wireless/ath/ath10k/txrx.c
index 7579de8..7464d82 100644
--- a/drivers/net/wireless/ath/ath10k/txrx.c
+++ b/drivers/net/wireless/ath/ath10k/txrx.c
@@ -55,8 +55,10 @@ void ath10k_txrx_tx_unref(struct ath10k_htt *htt,
 
 	lockdep_assert_held(&htt->tx_lock);
 
-	ath10k_dbg(ar, ATH10K_DBG_HTT, "htt tx completion msdu_id %u discard %d no_ack %d\n",
-		   tx_done->msdu_id, !!tx_done->discard, !!tx_done->no_ack);
+	ath10k_dbg(ar, ATH10K_DBG_HTT,
+		   "htt tx completion msdu_id %u discard %d no_ack %d success %d\n",
+		   tx_done->msdu_id, !!tx_done->discard,
+		   !!tx_done->no_ack, !!tx_done->success);
 
 	if (tx_done->msdu_id >= htt->max_num_pending_tx) {
 		ath10k_warn(ar, "warning: msdu_id %d too big, ignoring\n",
@@ -91,6 +93,9 @@ void ath10k_txrx_tx_unref(struct ath10k_htt *htt,
 	if (tx_done->no_ack)
 		info->flags &= ~IEEE80211_TX_STAT_ACK;
 
+	if (tx_done->success && (info->flags & IEEE80211_TX_CTL_NO_ACK))
+		info->flags |= IEEE80211_TX_STAT_NOACK_TRANSMITTED;
+
 	ieee80211_tx_status(htt->ar->hw, msdu);
 	/* we do not own the msdu anymore */
 
-- 
2.1.3

Re: [PATCH 3/4] ath10k: Fix no-ack frame status

[Kalle Valo]

Sujith Manoharan <sujith@msujith.org> writes:

> From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
>
> Use the new IEEE80211_TX_STAT_NOACK_TRANSMITTED flag
> to indicate successful transmission of no-ack frames.
> This fixes multicast frame accounting.
>
> Cc: ath10k@lists.infradead.org
> Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>

Johannes, are you planning to take this? Or should I take this once the
corresponding mac80211 patch trickles down to my tree?

-- 
Kalle Valo

Re: [PATCH 3/4] ath10k: Fix no-ack frame status

[Johannes Berg]

On Thu, 2014-12-11 at 12:10 +0200, Kalle Valo wrote:
> Sujith Manoharan <sujith@msujith.org> writes:
> 
> > From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> >
> > Use the new IEEE80211_TX_STAT_NOACK_TRANSMITTED flag
> > to indicate successful transmission of no-ack frames.
> > This fixes multicast frame accounting.
> >
> > Cc: ath10k@lists.infradead.org
> > Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> 
> Johannes, are you planning to take this? Or should I take this once the
> corresponding mac80211 patch trickles down to my tree?

I don't really want to take the driver patches, but I'll try to get to
the mac80211 ones today.

johannes

Re: [PATCH 3/4] ath10k: Fix no-ack frame status

[Sujith Manoharan]

Kalle Valo wrote:
> Johannes, are you planning to take this? Or should I take this once the
> corresponding mac80211 patch trickles down to my tree?

Kalle, can you please pick this one ? I'll rebase and send the ath9k patch
to John once he starts merging patches.

Sujith

Re: [PATCH 3/4] ath10k: Fix no-ack frame status

[Kalle Valo]

Sujith Manoharan <sujith@msujith.org> writes:

> Kalle Valo wrote:
>> Johannes, are you planning to take this? Or should I take this once the
>> corresponding mac80211 patch trickles down to my tree?
>
> Kalle, can you please pick this one ? I'll rebase and send the ath9k patch
> to John once he starts merging patches.

Yes, I'll take this patch once I have the mac80211 dependency in my
tree. I marked the patch as "Awaiting Upstream" in patchwork so that I
don't forget, hopefully :)

https://patchwork.kernel.org/patch/5470621/

-- 
Kalle Valo

Re: [PATCH 3/4] ath10k: Fix no-ack frame status

[Kalle Valo]

(Discussion from last December)

Kalle Valo <kvalo@qca.qualcomm.com> writes:

> Sujith Manoharan <sujith@msujith.org> writes:
>
>> Kalle Valo wrote:
>>> Johannes, are you planning to take this? Or should I take this once the
>>> corresponding mac80211 patch trickles down to my tree?
>>
>> Kalle, can you please pick this one ? I'll rebase and send the ath9k patch
>> to John once he starts merging patches.
>
> Yes, I'll take this patch once I have the mac80211 dependency in my
> tree. I marked the patch as "Awaiting Upstream" in patchwork so that I
> don't forget, hopefully :)
>
> https://patchwork.kernel.org/patch/5470621/

Naturally I did forget this :) But the patch is in pending branch now.

-- 
Kalle Valo

Re: [PATCH 3/4] ath10k: Fix no-ack frame status

[Sujith Manoharan]

Kalle Valo wrote:
> > https://patchwork.kernel.org/patch/5470621/
> 
> Naturally I did forget this :) But the patch is in pending branch now.

Thanks...

Sujith

Re: [PATCH 3/4] ath10k: Fix no-ack frame status

[Kalle Valo]

Sujith Manoharan <sujith@msujith.org> writes:

> From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
>
> Use the new IEEE80211_TX_STAT_NOACK_TRANSMITTED flag
> to indicate successful transmission of no-ack frames.
> This fixes multicast frame accounting.
>
> Cc: ath10k@lists.infradead.org
> Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>

Thanks, applied.

-- 
Kalle Valo

[PATCH 4/4] ath9k: Fix no-ack frame status

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

Check if the frame has been completed without any
error and use IEEE80211_TX_STAT_NOACK_TRANSMITTED to
indicate successful transmission of no-ack frames.

Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath9k/xmit.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 4caee66..c7ff9a8 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -2445,9 +2445,12 @@ static void ath_tx_complete(struct ath_softc *sc, struct sk_buff *skb,
 	if (sc->sc_ah->caldata)
 		set_bit(PAPRD_PACKET_SENT, &sc->sc_ah->caldata->cal_flags);
 
-	if (!(tx_flags & ATH_TX_ERROR))
-		/* Frame was ACKed */
-		tx_info->flags |= IEEE80211_TX_STAT_ACK;
+	if (!(tx_flags & ATH_TX_ERROR)) {
+		if (tx_info->flags & IEEE80211_TX_CTL_NO_ACK)
+			tx_info->flags |= IEEE80211_TX_STAT_NOACK_TRANSMITTED;
+		else
+			tx_info->flags |= IEEE80211_TX_STAT_ACK;
+	}
 
 	padpos = ieee80211_hdrlen(hdr->frame_control);
 	padsize = padpos & 3;
-- 
2.1.3

Re: [PATCH 1/4] mac80211: Move IEEE80211_TX_CTL_PS_RESPONSE

[Johannes Berg]

On Wed, 2014-12-10 at 21:26 +0530, Sujith Manoharan wrote:
> From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> 
> Move IEEE80211_TX_CTL_PS_RESPONSE to info->control.flags since
> this is used only in the TX path (by ath9k). This frees up
> a bit which can be used for other purposes.

Applied.

johannes