[RFC 1/2] ath9k: use ieee80211_free_txskb

[RFC 1/2] ath9k: use ieee80211_free_txskb

[Christian Lamparter]

With the new tx status API: "mac80211: implement wifi TX status"
All skb originating from mac80211 needs to be given back to mac80211.

Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
It's high time we change all calls in the tx-path from
dev_kfree_skb into ieee80211_free_txskb.

The call in ath9k_tx is straightforward, but the one in
ath_tx_setup_buffer gives me headaches. I'm not sure if
we even need to check bf->bf_state.bfs_paprd at this
stage since ath_tx_start_dma sets bf->bf_state.bfs_paprd
"after" calling ath_tx_setup_buffer?!

Regards,
	Chr
---
 drivers/net/wireless/ath/ath9k/main.c |    2 +-
 drivers/net/wireless/ath/ath9k/xmit.c |    6 +++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index c81304d..4471ed9 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1162,7 +1162,7 @@ static void ath9k_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
 
 	return;
 exit:
-	dev_kfree_skb_any(skb);
+	ieee80211_free_txskb(sc->hw, skb);
 }
 
 static void ath9k_stop(struct ieee80211_hw *hw)
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 3182408..0dadbc3 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -1858,7 +1858,11 @@ static struct ath_buf *ath_tx_setup_buffer(struct ath_softc *sc,
 	return bf;
 
 error:
-	dev_kfree_skb_any(skb);
+
+	if (bf->bf_state.bfs_paprd)
+		dev_kfree_skb_any(skb);
+	else
+		ieee80211_free_txskb(sc->hw, skb);
 	return NULL;
 }
 
-- 
1.7.9.1

[RFC 2/2] ath9k_htc: use ieee80211_free_txskb

[Christian Lamparter]

With the new tx status API: "mac80211: implement wifi TX status"
All skb originating from mac80211 needs to be given back to mac80211.

Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
It compiles, but not tested.
---
 drivers/net/wireless/ath/ath9k/htc_drv_txrx.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
index 3e40a64..e958704 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
@@ -451,7 +451,7 @@ static void ath9k_htc_tx_process(struct ath9k_htc_priv *priv,
 
 	slot = strip_drv_header(priv, skb);
 	if (slot < 0) {
-		dev_kfree_skb_any(skb);
+		ieee80211_free_txskb(priv->hw, skb);
 		return;
 	}
 
@@ -687,7 +687,7 @@ void ath9k_htc_txep(void *drv_priv, struct sk_buff *skb,
 
 	epid_queue = get_htc_epid_queue(priv, ep_id);
 	if (!epid_queue) {
-		dev_kfree_skb_any(skb);
+		ieee80211_free_txskb(priv->hw, skb);
 		return;
 	}
 
-- 
1.7.9.1

Re: [RFC 2/2] ath9k_htc: use ieee80211_free_txskb

[Mohammed Shafi Shajakhan]

Hi Christian,


On Monday 27 February 2012 10:02 PM, Christian Lamparter wrote:
> With the new tx status API: "mac80211: implement wifi TX status"
> All skb originating from mac80211 needs to be given back to mac80211.
>
> Signed-off-by: Christian Lamparter<chunkeey@googlemail.com>
> ---
> It compiles, but not tested.
> ---
>   drivers/net/wireless/ath/ath9k/htc_drv_txrx.c |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> index 3e40a64..e958704 100644
> --- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> +++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> @@ -451,7 +451,7 @@ static void ath9k_htc_tx_process(struct ath9k_htc_priv *priv,
>
>   	slot = strip_drv_header(priv, skb);
>   	if (slot<  0) {
> -		dev_kfree_skb_any(skb);
> +		ieee80211_free_txskb(priv->hw, skb);
>   		return;
>   	}
>
> @@ -687,7 +687,7 @@ void ath9k_htc_txep(void *drv_priv, struct sk_buff *skb,
>
>   	epid_queue = get_htc_epid_queue(priv, ep_id);
>   	if (!epid_queue) {
> -		dev_kfree_skb_any(skb);
> +		ieee80211_free_txskb(priv->hw, skb);
>   		return;
>   	}
>

also we should do it ath9k_htc_tx in htc_drv_main.c, i don't know very 
much about ath9k_htc.


-- 
thanks,
shafi

Re: [RFC 1/2] ath9k: use ieee80211_free_txskb

[Mohammed Shafi Shajakhan]

Hi Christian,

On Monday 27 February 2012 09:40 PM, Christian Lamparter wrote:
> With the new tx status API: "mac80211: implement wifi TX status"
> All skb originating from mac80211 needs to be given back to mac80211.
>
> Signed-off-by: Christian Lamparter<chunkeey@googlemail.com>
> ---
> It's high time we change all calls in the tx-path from
> dev_kfree_skb into ieee80211_free_txskb.
>
> The call in ath9k_tx is straightforward, but the one in
> ath_tx_setup_buffer gives me headaches. I'm not sure if
> we even need to check bf->bf_state.bfs_paprd at this
> stage since ath_tx_start_dma sets bf->bf_state.bfs_paprd
> "after" calling ath_tx_setup_buffer?!

yes, i think its a problem, we cannot exactly say its the skb from 
mac80211 (or) its our PAPRD skb. though with the current code(as the 
PAPRD) is disabled the skb would be always from mac80211. let us see if 
we have some good solution.

>
> Regards,
> 	Chr
> ---
>   drivers/net/wireless/ath/ath9k/main.c |    2 +-
>   drivers/net/wireless/ath/ath9k/xmit.c |    6 +++++-
>   2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index c81304d..4471ed9 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -1162,7 +1162,7 @@ static void ath9k_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
>
>   	return;
>   exit:
> -	dev_kfree_skb_any(skb);
> +	ieee80211_free_txskb(sc->hw, skb);
>   }
>
>   static void ath9k_stop(struct ieee80211_hw *hw)
> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> index 3182408..0dadbc3 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -1858,7 +1858,11 @@ static struct ath_buf *ath_tx_setup_buffer(struct ath_softc *sc,
>   	return bf;
>
>   error:
> -	dev_kfree_skb_any(skb);
> +
> +	if (bf->bf_state.bfs_paprd)
> +		dev_kfree_skb_any(skb);
> +	else
> +		ieee80211_free_txskb(sc->hw, skb);
>   	return NULL;
>   }
>


-- 
thanks,
shafi

Re: [RFC 1/2] ath9k: use ieee80211_free_txskb

[Mohammed Shafi Shajakhan]

[-- Attachment #1: Type: text/plain, Size: 2992 bytes --]

On Thursday 01 March 2012 11:09 AM, Mohammed Shafi Shajakhan wrote:
> Hi Christian,
>
> On Monday 27 February 2012 09:40 PM, Christian Lamparter wrote:
>> With the new tx status API: "mac80211: implement wifi TX status"
>> All skb originating from mac80211 needs to be given back to mac80211.
>>
>> Signed-off-by: Christian Lamparter<chunkeey@googlemail.com>
>> ---
>> It's high time we change all calls in the tx-path from
>> dev_kfree_skb into ieee80211_free_txskb.
>>
>> The call in ath9k_tx is straightforward, but the one in
>> ath_tx_setup_buffer gives me headaches. I'm not sure if
>> we even need to check bf->bf_state.bfs_paprd at this
>> stage since ath_tx_start_dma sets bf->bf_state.bfs_paprd
>> "after" calling ath_tx_setup_buffer?!
>
> yes, i think its a problem, we cannot exactly say its the skb from
> mac80211 (or) its our PAPRD skb. though with the current code(as the
> PAPRD) is disabled the skb would be always from mac80211. let us see if
> we have some good solution.


got some idea, need to make sure this works perfectly, paprd frames are 
not aggregated, so we can look only at the non-aggregated path.

lets ignore aggregated path:

*ath_tx_form_aggr
*ath_tx_send_ampdu


non-aggregated path :

*ath_tx_start_dma
*ath_send_normal

->  'ath_tx_setup_buffer' called from 'ath_tx_start_dma' can have a 
'is_paprd' check based on 'txctl->paprd' which holds which chain we are 
sending PAPRD frame.

-> 'ath_send_normal' called from
                                  -ath_tx_start_dma ( then 'bf' should 
be valid and we would not have called 'ath_tx_setup_buffer'
				 -ath_tx_flush_tid ( which is an aggregation path) ignore it.


attached rough patch for ath9k, need to be compiled and tested. i will 
also analyze if this can be done in a better way or any flaws in it


>>
>> Regards,
>> Chr
>> ---
>> drivers/net/wireless/ath/ath9k/main.c | 2 +-
>> drivers/net/wireless/ath/ath9k/xmit.c | 6 +++++-
>> 2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/main.c
>> b/drivers/net/wireless/ath/ath9k/main.c
>> index c81304d..4471ed9 100644
>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> @@ -1162,7 +1162,7 @@ static void ath9k_tx(struct ieee80211_hw *hw,
>> struct sk_buff *skb)
>>
>> return;
>> exit:
>> - dev_kfree_skb_any(skb);
>> + ieee80211_free_txskb(sc->hw, skb);
>> }
>>
>> static void ath9k_stop(struct ieee80211_hw *hw)
>> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c
>> b/drivers/net/wireless/ath/ath9k/xmit.c
>> index 3182408..0dadbc3 100644
>> --- a/drivers/net/wireless/ath/ath9k/xmit.c
>> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
>> @@ -1858,7 +1858,11 @@ static struct ath_buf
>> *ath_tx_setup_buffer(struct ath_softc *sc,
>> return bf;
>>
>> error:
>> - dev_kfree_skb_any(skb);
>> +
>> + if (bf->bf_state.bfs_paprd)
>> + dev_kfree_skb_any(skb);
>> + else
>> + ieee80211_free_txskb(sc->hw, skb);
>> return NULL;
>> }
>>
>
>


-- 
thanks,
shafi

[-- Attachment #2: use-mac80211-api-freeing-skb.patch --]
[-- Type: text/x-patch, Size: 2865 bytes --]

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 02e95c8..1a37a43 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1148,7 +1148,7 @@ static void ath9k_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
 
 	return;
 exit:
-	dev_kfree_skb_any(skb);
+	ieee80211_free_txskb(sc->hw, skb);
 }
 
 static void ath9k_stop(struct ieee80211_hw *hw)
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 5dd27d2..699086c 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -811,7 +811,7 @@ static enum ATH_AGGR_STATUS ath_tx_form_aggr(struct ath_softc *sc,
 		fi = get_frame_info(skb);
 		bf = fi->bf;
 		if (!fi->bf)
-			bf = ath_tx_setup_buffer(sc, txq, tid, skb);
+			bf = ath_tx_setup_buffer(sc, txq, tid, skb, false);
 
 		if (!bf)
 			continue;
@@ -1728,7 +1728,7 @@ static void ath_tx_send_ampdu(struct ath_softc *sc, struct ath_atx_tid *tid,
 		return;
 	}
 
-	bf = ath_tx_setup_buffer(sc, txctl->txq, tid, skb);
+	bf = ath_tx_setup_buffer(sc, txctl->txq, tid, skb, false);
 	if (!bf)
 		return;
 
@@ -1752,10 +1752,12 @@ static void ath_tx_send_normal(struct ath_softc *sc, struct ath_txq *txq,
 	struct ath_frame_info *fi = get_frame_info(skb);
 	struct list_head bf_head;
 	struct ath_buf *bf;
+	bool is_paprd = false;
 
 	bf = fi->bf;
+
 	if (!bf)
-		bf = ath_tx_setup_buffer(sc, txq, tid, skb);
+		bf = ath_tx_setup_buffer(sc, txq, tid, skb, false);
 
 	if (!bf)
 		return;
@@ -1816,7 +1818,8 @@ u8 ath_txchainmask_reduction(struct ath_softc *sc, u8 chainmask, u32 rate)
 static struct ath_buf *ath_tx_setup_buffer(struct ath_softc *sc,
 					   struct ath_txq *txq,
 					   struct ath_atx_tid *tid,
-					   struct sk_buff *skb)
+					   struct sk_buff *skb,
+					   bool is_paprd)
 {
 	struct ath_common *common = ath9k_hw_common(sc->sc_ah);
 	struct ath_frame_info *fi = get_frame_info(skb);
@@ -1857,7 +1860,11 @@ static struct ath_buf *ath_tx_setup_buffer(struct ath_softc *sc,
 	return bf;
 
 error:
-	dev_kfree_skb_any(skb);
+	if (is_paprd)
+		dev_kfree_skb_any(skb);
+	else
+		ieee80211_free_txskb(sc->hw, skb);
+
 	return NULL;
 }
 
@@ -1870,6 +1877,7 @@ static void ath_tx_start_dma(struct ath_softc *sc, struct sk_buff *skb,
 	struct ath_atx_tid *tid = NULL;
 	struct ath_buf *bf;
 	u8 tidno;
+	bool is_paprd = false;
 
 	if ((sc->sc_flags & SC_OP_TXAGGR) && txctl->an &&
 		ieee80211_is_data_qos(hdr->frame_control)) {
@@ -1887,7 +1895,10 @@ static void ath_tx_start_dma(struct ath_softc *sc, struct sk_buff *skb,
 		 */
 		ath_tx_send_ampdu(sc, tid, skb, txctl);
 	} else {
-		bf = ath_tx_setup_buffer(sc, txctl->txq, tid, skb);
+		if (txctl->paprd)
+			is_paprd = true;
+
+		bf = ath_tx_setup_buffer(sc, txctl->txq, tid, skb, is_paprd);
 		if (!bf)
 			return;
 

Re: [RFC 1/2] ath9k: use ieee80211_free_txskb

[Mohammed Shafi Shajakhan]

[-- Attachment #1: Type: text/plain, Size: 2034 bytes --]

On Thursday 01 March 2012 05:12 PM, Mohammed Shafi Shajakhan wrote:
> On Thursday 01 March 2012 11:09 AM, Mohammed Shafi Shajakhan wrote:
>> Hi Christian,
>>
>> On Monday 27 February 2012 09:40 PM, Christian Lamparter wrote:
>>> With the new tx status API: "mac80211: implement wifi TX status"
>>> All skb originating from mac80211 needs to be given back to mac80211.
>>>
>>> Signed-off-by: Christian Lamparter<chunkeey@googlemail.com>
>>> ---
>>> It's high time we change all calls in the tx-path from
>>> dev_kfree_skb into ieee80211_free_txskb.
>>>
>>> The call in ath9k_tx is straightforward, but the one in
>>> ath_tx_setup_buffer gives me headaches. I'm not sure if
>>> we even need to check bf->bf_state.bfs_paprd at this
>>> stage since ath_tx_start_dma sets bf->bf_state.bfs_paprd
>>> "after" calling ath_tx_setup_buffer?!
>>
>> yes, i think its a problem, we cannot exactly say its the skb from
>> mac80211 (or) its our PAPRD skb. though with the current code(as the
>> PAPRD) is disabled the skb would be always from mac80211. let us see if
>> we have some good solution.
>
>
> got some idea, need to make sure this works perfectly, paprd frames are
> not aggregated, so we can look only at the non-aggregated path.
>
> lets ignore aggregated path:
>
> *ath_tx_form_aggr
> *ath_tx_send_ampdu
>
>
> non-aggregated path :
>
> *ath_tx_start_dma
> *ath_send_normal
>
> -> 'ath_tx_setup_buffer' called from 'ath_tx_start_dma' can have a
> 'is_paprd' check based on 'txctl->paprd' which holds which chain we are
> sending PAPRD frame.
>
> -> 'ath_send_normal' called from
> -ath_tx_start_dma ( then 'bf' should be valid and we would not have
> called 'ath_tx_setup_buffer'
> -ath_tx_flush_tid ( which is an aggregation path) ignore it.
>
>
> attached rough patch for ath9k, need to be compiled and tested. i will
> also analyze if this can be done in a better way or any flaws in it
>

compilation fails with previous patch, forgot to declare the field in 
the declaration, attached patch passes compilation.


-- 
thanks,
shafi

[-- Attachment #2: use-mac80211-api-freeing-skb.patch --]
[-- Type: text/x-patch, Size: 3092 bytes --]

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 02e95c8..1a37a43 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1148,7 +1148,7 @@ static void ath9k_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
 
 	return;
 exit:
-	dev_kfree_skb_any(skb);
+	ieee80211_free_txskb(sc->hw, skb);
 }
 
 static void ath9k_stop(struct ieee80211_hw *hw)
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 5dd27d2..7b9f1ba 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -64,7 +64,8 @@ static void ath_tx_update_baw(struct ath_softc *sc, struct ath_atx_tid *tid,
 static struct ath_buf *ath_tx_setup_buffer(struct ath_softc *sc,
 					   struct ath_txq *txq,
 					   struct ath_atx_tid *tid,
-					   struct sk_buff *skb);
+					   struct sk_buff *skb,
+					   bool is_paprd);
 
 enum {
 	MCS_HT20,
@@ -811,7 +812,7 @@ static enum ATH_AGGR_STATUS ath_tx_form_aggr(struct ath_softc *sc,
 		fi = get_frame_info(skb);
 		bf = fi->bf;
 		if (!fi->bf)
-			bf = ath_tx_setup_buffer(sc, txq, tid, skb);
+			bf = ath_tx_setup_buffer(sc, txq, tid, skb, false);
 
 		if (!bf)
 			continue;
@@ -1728,7 +1729,7 @@ static void ath_tx_send_ampdu(struct ath_softc *sc, struct ath_atx_tid *tid,
 		return;
 	}
 
-	bf = ath_tx_setup_buffer(sc, txctl->txq, tid, skb);
+	bf = ath_tx_setup_buffer(sc, txctl->txq, tid, skb, false);
 	if (!bf)
 		return;
 
@@ -1754,8 +1755,9 @@ static void ath_tx_send_normal(struct ath_softc *sc, struct ath_txq *txq,
 	struct ath_buf *bf;
 
 	bf = fi->bf;
+
 	if (!bf)
-		bf = ath_tx_setup_buffer(sc, txq, tid, skb);
+		bf = ath_tx_setup_buffer(sc, txq, tid, skb, false);
 
 	if (!bf)
 		return;
@@ -1816,7 +1818,8 @@ u8 ath_txchainmask_reduction(struct ath_softc *sc, u8 chainmask, u32 rate)
 static struct ath_buf *ath_tx_setup_buffer(struct ath_softc *sc,
 					   struct ath_txq *txq,
 					   struct ath_atx_tid *tid,
-					   struct sk_buff *skb)
+					   struct sk_buff *skb,
+					   bool is_paprd)
 {
 	struct ath_common *common = ath9k_hw_common(sc->sc_ah);
 	struct ath_frame_info *fi = get_frame_info(skb);
@@ -1857,7 +1860,11 @@ static struct ath_buf *ath_tx_setup_buffer(struct ath_softc *sc,
 	return bf;
 
 error:
-	dev_kfree_skb_any(skb);
+	if (is_paprd)
+		dev_kfree_skb_any(skb);
+	else
+		ieee80211_free_txskb(sc->hw, skb);
+
 	return NULL;
 }
 
@@ -1870,6 +1877,7 @@ static void ath_tx_start_dma(struct ath_softc *sc, struct sk_buff *skb,
 	struct ath_atx_tid *tid = NULL;
 	struct ath_buf *bf;
 	u8 tidno;
+	bool is_paprd = false;
 
 	if ((sc->sc_flags & SC_OP_TXAGGR) && txctl->an &&
 		ieee80211_is_data_qos(hdr->frame_control)) {
@@ -1887,7 +1895,10 @@ static void ath_tx_start_dma(struct ath_softc *sc, struct sk_buff *skb,
 		 */
 		ath_tx_send_ampdu(sc, tid, skb, txctl);
 	} else {
-		bf = ath_tx_setup_buffer(sc, txctl->txq, tid, skb);
+		if (txctl->paprd)
+			is_paprd = true;
+
+		bf = ath_tx_setup_buffer(sc, txctl->txq, tid, skb, is_paprd);
 		if (!bf)
 			return;
 

Re: [RFC 1/2] ath9k: use ieee80211_free_txskb

[Mohammed Shafi Shajakhan]

Hi Christian,

ping

On Thursday 01 March 2012 07:16 PM, Mohammed Shafi Shajakhan wrote:
> On Thursday 01 March 2012 05:12 PM, Mohammed Shafi Shajakhan wrote:
>> On Thursday 01 March 2012 11:09 AM, Mohammed Shafi Shajakhan wrote:
>>> Hi Christian,
>>>
>>> On Monday 27 February 2012 09:40 PM, Christian Lamparter wrote:
>>>> With the new tx status API: "mac80211: implement wifi TX status"
>>>> All skb originating from mac80211 needs to be given back to mac80211.
>>>>
>>>> Signed-off-by: Christian Lamparter<chunkeey@googlemail.com>
>>>> ---
>>>> It's high time we change all calls in the tx-path from
>>>> dev_kfree_skb into ieee80211_free_txskb.
>>>>
>>>> The call in ath9k_tx is straightforward, but the one in
>>>> ath_tx_setup_buffer gives me headaches. I'm not sure if
>>>> we even need to check bf->bf_state.bfs_paprd at this
>>>> stage since ath_tx_start_dma sets bf->bf_state.bfs_paprd
>>>> "after" calling ath_tx_setup_buffer?!
>>>
>>> yes, i think its a problem, we cannot exactly say its the skb from
>>> mac80211 (or) its our PAPRD skb. though with the current code(as the
>>> PAPRD) is disabled the skb would be always from mac80211. let us see if
>>> we have some good solution.
>>
>>
>> got some idea, need to make sure this works perfectly, paprd frames are
>> not aggregated, so we can look only at the non-aggregated path.
>>
>> lets ignore aggregated path:
>>
>> *ath_tx_form_aggr
>> *ath_tx_send_ampdu
>>
>>
>> non-aggregated path :
>>
>> *ath_tx_start_dma
>> *ath_send_normal
>>
>> -> 'ath_tx_setup_buffer' called from 'ath_tx_start_dma' can have a
>> 'is_paprd' check based on 'txctl->paprd' which holds which chain we are
>> sending PAPRD frame.
>>
>> -> 'ath_send_normal' called from
>> -ath_tx_start_dma ( then 'bf' should be valid and we would not have
>> called 'ath_tx_setup_buffer'
>> -ath_tx_flush_tid ( which is an aggregation path) ignore it.
>>
>>
>> attached rough patch for ath9k, need to be compiled and tested. i will
>> also analyze if this can be done in a better way or any flaws in it
>>
>
> compilation fails with previous patch, forgot to declare the field in
> the declaration, attached patch passes compilation.
>
>

would you like me to test this patch, i can just ran a test parallel y 
enabling PAPRD,with this proposed patch.

-- 
thanks,
shafi

Re: [RFC 1/2] ath9k: use ieee80211_free_txskb

[Mohammed Shafi Shajakhan]

Hi Christian,

ping

On Thursday 01 March 2012 07:16 PM, Mohammed Shafi Shajakhan wrote:
> On Thursday 01 March 2012 05:12 PM, Mohammed Shafi Shajakhan wrote:
>> On Thursday 01 March 2012 11:09 AM, Mohammed Shafi Shajakhan wrote:
>>> Hi Christian,
>>>
>>> On Monday 27 February 2012 09:40 PM, Christian Lamparter wrote:
>>>> With the new tx status API: "mac80211: implement wifi TX status"
>>>> All skb originating from mac80211 needs to be given back to mac80211.
>>>>
>>>> Signed-off-by: Christian Lamparter<chunkeey@googlemail.com>
>>>> ---
>>>> It's high time we change all calls in the tx-path from
>>>> dev_kfree_skb into ieee80211_free_txskb.
>>>>
>>>> The call in ath9k_tx is straightforward, but the one in
>>>> ath_tx_setup_buffer gives me headaches. I'm not sure if
>>>> we even need to check bf->bf_state.bfs_paprd at this
>>>> stage since ath_tx_start_dma sets bf->bf_state.bfs_paprd
>>>> "after" calling ath_tx_setup_buffer?!
>>>
>>> yes, i think its a problem, we cannot exactly say its the skb from
>>> mac80211 (or) its our PAPRD skb. though with the current code(as the
>>> PAPRD) is disabled the skb would be always from mac80211. let us see if
>>> we have some good solution.
>>
>>
>> got some idea, need to make sure this works perfectly, paprd frames are
>> not aggregated, so we can look only at the non-aggregated path.
>>
>> lets ignore aggregated path:
>>
>> *ath_tx_form_aggr
>> *ath_tx_send_ampdu
>>
>>
>> non-aggregated path :
>>
>> *ath_tx_start_dma
>> *ath_send_normal
>>
>> -> 'ath_tx_setup_buffer' called from 'ath_tx_start_dma' can have a
>> 'is_paprd' check based on 'txctl->paprd' which holds which chain we are
>> sending PAPRD frame.
>>
>> -> 'ath_send_normal' called from
>> -ath_tx_start_dma ( then 'bf' should be valid and we would not have
>> called 'ath_tx_setup_buffer'
>> -ath_tx_flush_tid ( which is an aggregation path) ignore it.
>>
>>
>> attached rough patch for ath9k, need to be compiled and tested. i will
>> also analyze if this can be done in a better way or any flaws in it
>>
>
> compilation fails with previous patch, forgot to declare the field in
> the declaration, attached patch passes compilation.
>
>

would you like me to test this patch, i can just ran a bidirectional 
traffic test parallel y enabling PAPRD,with this proposed patch.

-- 
thanks,
shafi

Re: [RFC 1/2] ath9k: use ieee80211_free_txskb

[Christian Lamparter]

On Monday, March 05, 2012 04:01:17 PM Mohammed Shafi Shajakhan wrote:
> ping
pong !

> On Thursday 01 March 2012 07:16 PM, Mohammed Shafi Shajakhan wrote:
> > On Thursday 01 March 2012 05:12 PM, Mohammed Shafi Shajakhan wrote:
> >> On Thursday 01 March 2012 11:09 AM, Mohammed Shafi Shajakhan wrote:
> >>> Hi Christian,
> >>>
> >>> On Monday 27 February 2012 09:40 PM, Christian Lamparter wrote:
> >>>> With the new tx status API: "mac80211: implement wifi TX status"
> >>>> All skb originating from mac80211 needs to be given back to mac80211.
> >>>>
> >>>> Signed-off-by: Christian Lamparter<chunkeey@googlemail.com>
> >>>> ---
> >>>> It's high time we change all calls in the tx-path from
> >>>> dev_kfree_skb into ieee80211_free_txskb.
> >>>>
> >>>> The call in ath9k_tx is straightforward, but the one in
> >>>> ath_tx_setup_buffer gives me headaches. I'm not sure if
> >>>> we even need to check bf->bf_state.bfs_paprd at this
> >>>> stage since ath_tx_start_dma sets bf->bf_state.bfs_paprd
> >>>> "after" calling ath_tx_setup_buffer?!
> >>>
> >>> yes, i think its a problem, we cannot exactly say its the skb from
> >>> mac80211 (or) its our PAPRD skb. though with the current code(as the
> >>> PAPRD) is disabled the skb would be always from mac80211. let us see if
> >>> we have some good solution.
> >>
> >>
> >> got some idea, need to make sure this works perfectly, paprd frames are
> >> not aggregated, so we can look only at the non-aggregated path.
> >>
> >> lets ignore aggregated path:
> >>
> >> *ath_tx_form_aggr
> >> *ath_tx_send_ampdu
> >>
> >>
> >> non-aggregated path :
> >>
> >> *ath_tx_start_dma
> >> *ath_send_normal
> >>
> >> -> 'ath_tx_setup_buffer' called from 'ath_tx_start_dma' can have a
> >> 'is_paprd' check based on 'txctl->paprd' which holds which chain we are
> >> sending PAPRD frame.
> >>
> >> -> 'ath_send_normal' called from
> >> -ath_tx_start_dma ( then 'bf' should be valid and we would not have
> >> called 'ath_tx_setup_buffer'
> >> -ath_tx_flush_tid ( which is an aggregation path) ignore it.
> >>
> >>
> >> attached rough patch for ath9k, need to be compiled and tested. i will
> >> also analyze if this can be done in a better way or any flaws in it
> >>
> >
> > compilation fails with previous patch, forgot to declare the field in
> > the declaration, attached patch passes compilation.
> >
> >
> 
> would you like me to test this patch,
? Sure, I would like someone with a AR9300+ to test the patch.
Currently, I'm stuck with a puny AR9287 [in a router].
So, I can't run tests all day :(.

> i can just ran a bidirectional traffic test parallel y enabling
> PAPRD,with this proposed patch.
Actually, the changes are all within ath9k's tx error paths. So,
I'm afraid just continuous traffic testing won't do, unless of course
you can churn out all sorts of tx (dma) errors at the same time. 

Regards,
	Chr

Re: [RFC 1/2] ath9k: use ieee80211_free_txskb

[Mohammed Shafi Shajakhan]

>> would you like me to test this patch,
> ? Sure, I would like someone with a AR9300+ to test the patch.
> Currently, I'm stuck with a puny AR9287 [in a router].
> So, I can't run tests all day :(.
>
>> i can just ran a bidirectional traffic test parallel y enabling
>> PAPRD,with this proposed patch.
> Actually, the changes are all within ath9k's tx error paths. So,
> I'm afraid just continuous traffic testing won't do, unless of course
> you can churn out all sorts of tx (dma) errors at the same time.

yes, i can just a start stress test that can run few days in a noisy 
channel, with the AR9300 card.

>
> Regards,
> 	Chr


-- 
thanks,
shafi