Re: [PATCH 1/4] ath10k: improve tx status reporting
Sergey Ryazanov wrote: > We use ieee80211_tx_status() to report each completed tx frame. > Internally, this function calls sta_info_get_by_addrs(), what has a > couple of drawbacks: > 1. additional station lookup causes a performance degradation; > 2. mac80211 can not properly account Ethernet encapsulated frames due >to the inability to properly determine the destination (station) MAC >address since ieee80211_tx_status() assumes the frame has a 802.11 >header. > > The latter is especially destructive if we want to use hardware frames > encapsulation. > > To fix both of these issues, replace ieee80211_tx_status() with > ieee80211_tx_status_ext() call and feed it station pointer from the tx > queue associated with the transmitted frame. > > Tested-on: QCA9888 hw2.0 PCI 10.4-3.9.0.2-00131 > Tested-on: QCA6174 hw3.2 PCI WLAN.RM.4.4.1-00157-QCARMSWPZ-1 > > Signed-off-by: Sergey Ryazanov > Tested-by: Oldřich Jedlička # TP-Link Archer C7 v4 & > v5 (QCA9563 + QCA9880) > Tested-by: Edward Matijevic # TP-Link Archer C2600 > (IPQ8064 + QCA9980 10.4.1.00030-1) > Tested-by: Edward Matijevic # QCA9377 PCI in Sta mode > Tested-by: Zhijun You # NETGEAR R7800 (QCA9984 > 10.4-3.9.0.2-00159) > Signed-off-by: Kalle Valo 4 patches applied to ath-next branch of ath.git, thanks. 2587d5198aa5 ath10k: improve tx status reporting 70f119fb82af ath10k: htt_tx: do not interpret Eth frames as WiFi a09740548275 ath10k: turn rawmode into frame_mode af6d8265c47e ath10k: add encapsulation offloading support -- https://patchwork.kernel.org/project/linux-wireless/patch/20220516032519.29831-2-ryazanov@gmail.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 1/4] ath10k: improve tx status reporting
On Wed, May 18, 2022 at 10:30 AM Kalle Valo wrote:
> Sergey Ryazanov writes:
>> On Mon, May 16, 2022 at 6:25 AM Sergey Ryazanov
>> wrote:
>>> --- a/drivers/net/wireless/ath/ath10k/txrx.c
>>> +++ b/drivers/net/wireless/ath/ath10k/txrx.c
>>> @@ -43,6 +43,7 @@ static void ath10k_report_offchan_tx(struct ath10k *ar,
>>> struct sk_buff *skb)
>>> int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
>>> const struct htt_tx_done *tx_done)
>>> {
>>> + struct ieee80211_tx_status status;
>>> struct ath10k *ar = htt->ar;
>>> struct device *dev = ar->dev;
>>> struct ieee80211_tx_info *info;
>>> @@ -128,7 +129,16 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
>>> info->status.flags |= IEEE80211_TX_STATUS_ACK_SIGNAL_VALID;
>>> }
>>>
>>> - ieee80211_tx_status(htt->ar->hw, msdu);
>>> + memset(, 0, sizeof(status));
>>> + status.skb = msdu;
>>> + status.info = info;
>>> +
>>> + rcu_read_lock();
>>> + if (txq && txq->sta)
>>> + status.sta = txq->sta;
>>
>> Just noticed that since we do not dereference the txq->sta pointer
>> here, the above code can be simplified to:
>>
>> if (txq)
>> status.sta = txq->sta;
>>
>> Kalle, should I send V2 or can you change this in your tree?
>
> I changed this in the pending branch, please check my changes:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending=1bd0c16e10229683fab1dd8adf8c4339992688b7
Exactly what I meant. Thank you!
--
Sergey
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 1/4] ath10k: improve tx status reporting
Sergey Ryazanov writes:
> On Mon, May 16, 2022 at 6:25 AM Sergey Ryazanov
> wrote:
>
>> --- a/drivers/net/wireless/ath/ath10k/txrx.c
>> +++ b/drivers/net/wireless/ath/ath10k/txrx.c
>> @@ -43,6 +43,7 @@ static void ath10k_report_offchan_tx(struct ath10k *ar,
>> struct sk_buff *skb)
>> int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
>> const struct htt_tx_done *tx_done)
>> {
>> + struct ieee80211_tx_status status;
>> struct ath10k *ar = htt->ar;
>> struct device *dev = ar->dev;
>> struct ieee80211_tx_info *info;
>> @@ -128,7 +129,16 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
>> info->status.flags |= IEEE80211_TX_STATUS_ACK_SIGNAL_VALID;
>> }
>>
>> - ieee80211_tx_status(htt->ar->hw, msdu);
>> + memset(, 0, sizeof(status));
>> + status.skb = msdu;
>> + status.info = info;
>> +
>> + rcu_read_lock();
>> + if (txq && txq->sta)
>> + status.sta = txq->sta;
>
> Just noticed that since we do not dereference the txq->sta pointer
> here, the above code can be simplified to:
>
> if (txq)
> status.sta = txq->sta;
>
> Kalle, should I send V2 or can you change this in your tree?
I changed this in the pending branch, please check my changes:
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending=1bd0c16e10229683fab1dd8adf8c4339992688b7
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 1/4] ath10k: improve tx status reporting
On Mon, May 16, 2022 at 6:25 AM Sergey Ryazanov wrote:
> We use ieee80211_tx_status() to report each completed tx frame.
> Internally, this function calls sta_info_get_by_addrs(), what has a
> couple of drawbacks:
> 1. additional station lookup causes a performance degradation;
> 2. mac80211 can not properly account Ethernet encapsulated frames due
>to the inability to properly determine the destination (station) MAC
>address since ieee80211_tx_status() assumes the frame has a 802.11
>header.
>
> The latter is especially destructive if we want to use hardware frames
> encapsulation.
>
> To fix both of these issues, replace ieee80211_tx_status() with
> ieee80211_tx_status_ext() call and feed it station pointer from the tx
> queue associated with the transmitted frame.
>
> Tested-on: QCA9888 hw 2.0 10.4-3.9.0.2-00131
> Tested-on: QCA6174 hw 3.2 PCI WLAN.RM.4.4.1-00157-QCARMSWPZ-1
> Signed-off-by: Sergey Ryazanov
> Tested-by: Oldřich Jedlička # TP-Link Archer C7 v4 &
> v5 (QCA9563 + QCA9880)
> Tested-by: Edward Matijevic # TP-Link Archer C2600
> (IPQ8064 + QCA9980 10.4.1.00030-1)
> Tested-by: Edward Matijevic # QCA9377 PCI in Sta mode
> Tested-by: Zhijun You # NETGEAR R7800 (QCA9984
> 10.4-3.9.0.2-00159)
> ---
>
> Changes since RFC:
> * new Tested-on and Tested-by tags
>
> drivers/net/wireless/ath/ath10k/txrx.c | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/txrx.c
> b/drivers/net/wireless/ath/ath10k/txrx.c
> index 10123974c3da..72540434c75b 100644
> --- a/drivers/net/wireless/ath/ath10k/txrx.c
> +++ b/drivers/net/wireless/ath/ath10k/txrx.c
> @@ -43,6 +43,7 @@ static void ath10k_report_offchan_tx(struct ath10k *ar,
> struct sk_buff *skb)
> int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
> const struct htt_tx_done *tx_done)
> {
> + struct ieee80211_tx_status status;
> struct ath10k *ar = htt->ar;
> struct device *dev = ar->dev;
> struct ieee80211_tx_info *info;
> @@ -128,7 +129,16 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
> info->status.flags |= IEEE80211_TX_STATUS_ACK_SIGNAL_VALID;
> }
>
> - ieee80211_tx_status(htt->ar->hw, msdu);
> + memset(, 0, sizeof(status));
> + status.skb = msdu;
> + status.info = info;
> +
> + rcu_read_lock();
> + if (txq && txq->sta)
> + status.sta = txq->sta;
Just noticed that since we do not dereference the txq->sta pointer
here, the above code can be simplified to:
if (txq)
status.sta = txq->sta;
Kalle, should I send V2 or can you change this in your tree?
> + ieee80211_tx_status_ext(htt->ar->hw, );
> + rcu_read_unlock();
> +
> /* we do not own the msdu anymore */
>
> return 0;
--
Sergey
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
[PATCH 1/4] ath10k: improve tx status reporting
We use ieee80211_tx_status() to report each completed tx frame.
Internally, this function calls sta_info_get_by_addrs(), what has a
couple of drawbacks:
1. additional station lookup causes a performance degradation;
2. mac80211 can not properly account Ethernet encapsulated frames due
to the inability to properly determine the destination (station) MAC
address since ieee80211_tx_status() assumes the frame has a 802.11
header.
The latter is especially destructive if we want to use hardware frames
encapsulation.
To fix both of these issues, replace ieee80211_tx_status() with
ieee80211_tx_status_ext() call and feed it station pointer from the tx
queue associated with the transmitted frame.
Tested-on: QCA9888 hw 2.0 10.4-3.9.0.2-00131
Tested-on: QCA6174 hw 3.2 PCI WLAN.RM.4.4.1-00157-QCARMSWPZ-1
Signed-off-by: Sergey Ryazanov
Tested-by: Oldřich Jedlička # TP-Link Archer C7 v4 & v5
(QCA9563 + QCA9880)
Tested-by: Edward Matijevic # TP-Link Archer C2600 (IPQ8064
+ QCA9980 10.4.1.00030-1)
Tested-by: Edward Matijevic # QCA9377 PCI in Sta mode
Tested-by: Zhijun You # NETGEAR R7800 (QCA9984
10.4-3.9.0.2-00159)
---
Changes since RFC:
* new Tested-on and Tested-by tags
drivers/net/wireless/ath/ath10k/txrx.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/txrx.c
b/drivers/net/wireless/ath/ath10k/txrx.c
index 10123974c3da..72540434c75b 100644
--- a/drivers/net/wireless/ath/ath10k/txrx.c
+++ b/drivers/net/wireless/ath/ath10k/txrx.c
@@ -43,6 +43,7 @@ static void ath10k_report_offchan_tx(struct ath10k *ar,
struct sk_buff *skb)
int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
const struct htt_tx_done *tx_done)
{
+ struct ieee80211_tx_status status;
struct ath10k *ar = htt->ar;
struct device *dev = ar->dev;
struct ieee80211_tx_info *info;
@@ -128,7 +129,16 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
info->status.flags |= IEEE80211_TX_STATUS_ACK_SIGNAL_VALID;
}
- ieee80211_tx_status(htt->ar->hw, msdu);
+ memset(, 0, sizeof(status));
+ status.skb = msdu;
+ status.info = info;
+
+ rcu_read_lock();
+ if (txq && txq->sta)
+ status.sta = txq->sta;
+ ieee80211_tx_status_ext(htt->ar->hw, );
+ rcu_read_unlock();
+
/* we do not own the msdu anymore */
return 0;
--
2.35.1
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
