Re: ath10k: set a-mpdu reference number in ath10k (receiver)

[Kalle Valo]

Matthias Frei <mf@frei.media> wrote:

> Set the a-mpdu reference number in ath10k to make it accessible in the
> receivers radiotap header. Implemented as in ath9k.  The reference number is
> needed for troubleshooting and research at the receivers site (e.g. to 
> identify
> mpdu's that were aggregated in an a-mpdu)
> 
> Signed-off-by: Matthias Frei <mf@frei.media>
> [kv...@qca.qualcomm.com: fix checkpatch warning, commit log cleanup]
> Signed-off-by: Kalle Valo <kv...@qca.qualcomm.com>

Patch applied to ath-next branch of ath.git, thanks.

47cc0ca91c9e ath10k: set a-mpdu receiver reference number

-- 
https://patchwork.kernel.org/patch/9826931/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] ath10k: set a-mpdu reference number in ath10k (receiver)

[Kalle Valo]

Matthias Frei  writes:

> Set the a-mpdu reference number in ath10k to make it accessible in the 
> receivers radiotap header. Implemented as in ath9k. 
> The reference number is needed for troubleshooting and research at the 
> receivers site (e.g. to identify mpdu's that were aggregated in an a-mpdu)
>
> Signed-off-by: Matthias Frei 

I did some changes in pending branch, please review them:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending=f722728460a5c9e9200a7f1362fa605a714c1968

> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -989,6 +989,9 @@ struct ath10k {
>   u32 reg_ack_cts_timeout_orig;
>   } fw_coverage;
>  
> + /* AMPDU */
> + u32 ampdu_ref;

The comment is not providing any extra value, I'll remove that.

What about locking? How is this protected or doesn't it need anything?

Also I renamed this to ampdu_reference just to be consistent with the
mac80211 name.

> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -877,16 +877,24 @@ static void ath10k_htt_rx_h_ppdu(struct ath10k *ar,
>   status->nss = 0;
>   status->encoding = RX_ENC_LEGACY;
>   status->bw = RATE_INFO_BW_20;
> - status->flag &= ~RX_FLAG_MACTIME_END;
> - status->flag |= RX_FLAG_NO_SIGNAL_VAL;
> + status->flag &= ~(RX_FLAG_MACTIME_END | RX_FLAG_AMPDU_IS_LAST);
> + status->flag |= RX_FLAG_NO_SIGNAL_VAL | RX_FLAG_AMPDU_DETAILS | 
> RX_FLAG_AMPDU_LAST_KNOWN;

This added a new warning:

drivers/net/wireless/ath/ath10k/htt_rx.c:894: line over 90 characters

I fixed that by separating the setting of ampdu flags.

> + /* set ampdu ref */
> + status->ampdu_reference = ar->ampdu_ref;

The comment is not telling anything new so removed it.

> - if (is_last_ppdu)
> + if (is_last_ppdu) {
>   ath10k_htt_rx_h_mactime(ar, status, rxd);
> +
> + /* set ampdu last segment flag */
> + status->flag |= RX_FLAG_AMPDU_IS_LAST;
> + ar->ampdu_ref++;
> + }

So this counter is per wiphy, not per vdev. Is that ok?

-- 
Kalle Valo

[PATCH] ath10k: set a-mpdu reference number in ath10k (receiver)

[Matthias Frei]

Set the a-mpdu reference number in ath10k to make it accessible in the 
receivers radiotap header. Implemented as in ath9k. 
The reference number is needed for troubleshooting and research at the 
receivers site (e.g. to identify mpdu's that were aggregated in an a-mpdu)

Signed-off-by: Matthias Frei 
---
 drivers/net/wireless/ath/ath10k/core.h   |  3 +++
 drivers/net/wireless/ath/ath10k/htt_rx.c | 14 +++---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h 
b/drivers/net/wireless/ath/ath10k/core.h
index bf09151..82adf55 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -989,6 +989,9 @@ struct ath10k {
u32 reg_ack_cts_timeout_orig;
} fw_coverage;
 
+   /* AMPDU */
+   u32 ampdu_ref;
+
/* must be last */
u8 drv_priv[0] __aligned(sizeof(void *));
 };
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 84b6067..a4de02b 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -877,16 +877,24 @@ static void ath10k_htt_rx_h_ppdu(struct ath10k *ar,
status->nss = 0;
status->encoding = RX_ENC_LEGACY;
status->bw = RATE_INFO_BW_20;
-   status->flag &= ~RX_FLAG_MACTIME_END;
-   status->flag |= RX_FLAG_NO_SIGNAL_VAL;
+   status->flag &= ~(RX_FLAG_MACTIME_END | RX_FLAG_AMPDU_IS_LAST);
+   status->flag |= RX_FLAG_NO_SIGNAL_VAL | RX_FLAG_AMPDU_DETAILS | 
RX_FLAG_AMPDU_LAST_KNOWN;
+
+   /* set ampdu ref */
+   status->ampdu_reference = ar->ampdu_ref;
 
ath10k_htt_rx_h_signal(ar, status, rxd);
ath10k_htt_rx_h_channel(ar, status, rxd, vdev_id);
ath10k_htt_rx_h_rates(ar, status, rxd);
}
 
-   if (is_last_ppdu)
+   if (is_last_ppdu) {
ath10k_htt_rx_h_mactime(ar, status, rxd);
+
+   /* set ampdu last segment flag */
+   status->flag |= RX_FLAG_AMPDU_IS_LAST;
+   ar->ampdu_ref++;
+   }
 }
 
 static const char * const tid_to_ac[] = {
-- 
2.7.4