On 6/2/2024 9:42 PM, Ramya Gnanasekar wrote:
> From: Dinesh Karthikeyan <quic_dinek@xxxxxxxxxxx>
>
> Add extended htt stats parser and print the corresponding TLVs associated
> with the requested htt_stats_type.
> Add support for TX PDEV related htt stats.
>
while reviewing some other patches in the internal pipeline I discovered I had
overlooked some things in this series, so pointing them out now...
> ---
> .../wireless/ath/ath12k/debugfs_htt_stats.c | 315 ++++++++++++++++++
> .../wireless/ath/ath12k/debugfs_htt_stats.h | 211 ++++++++++++
> drivers/net/wireless/ath/ath12k/dp_rx.c | 10 +-
> drivers/net/wireless/ath/ath12k/dp_rx.h | 4 +
> 4 files changed, 536 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c
> index d98b971cde7d..e0a0acd5eec8 100644
> --- a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c
> +++ b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c
> @@ -9,6 +9,321 @@
> #include "debug.h"
> #include "debugfs_htt_stats.h"
> #include "dp_tx.h"
> +#include "dp_rx.h"
> +
> +static u32
> +print_array_to_buf(u8 *buf, u32 offset, const char *header,
> + const __le32 *array, u32 array_len)
> +{
> + int index = 0;
> + u8 i;
> +
> + if (header) {
> + index += scnprintf(buf + offset,
> + ATH12K_HTT_STATS_BUF_SIZE - offset,
> + "%s = ", header);
> + }
> + for (i = 0; i < array_len; i++) {
> + index += scnprintf(buf + offset + index,
> + (ATH12K_HTT_STATS_BUF_SIZE - offset) - index,
> + " %u:%u,", i, le32_to_cpu(array[i]));
> + }
note that this will end up with a trailing comma, so suggest you add:
/* have the next print overwrite the last trailing comma */
index--;
> + index += scnprintf(buf + offset + index,
> + (ATH12K_HTT_STATS_BUF_SIZE - offset) - index,
> + "\n\n");
> + return index;
> +}
> +
> +static void
> +htt_print_tx_pdev_stats_cmn_tlv(const void *tag_buf, u16 tag_len,
> + struct debug_htt_stats_req *stats_req)
> +{
> + const struct ath12k_htt_tx_pdev_stats_cmn_tlv *htt_stats_buf = tag_buf;
> + u8 *buf = stats_req->buf;
> + u32 len = stats_req->buf_len;
> + u32 buf_len = ATH12K_HTT_STATS_BUF_SIZE;
> + u32 mac_id_word = le32_to_cpu(htt_stats_buf->mac_id__word);
this dereference has to occur after the length check, otherwise you may have a
buffer overread
> +
> + if (tag_len < sizeof(struct ath12k_htt_tx_pdev_stats_cmn_tlv))
sizeof(*htt_stats_buf) is preferred
> + return;
> +
that concludes my new comments
