This implements the Packet log feature for ath10k.
All tx and rx events and rx descriptor details are
posted to user space by tracepoints.
Rajkumar Manoharan (4):
ath10k: Add support to configure pktlog filter
ath10k: add tracing for ath10k_htt_pktlog
ath10k: add tracing for rx descriptor
ath10k: add tracing for tx msdu pktlog event
drivers/net/wireless/ath/ath10k/core.h | 2 +
drivers/net/wireless/ath/ath10k/debug.c | 80 +++++++++++++++++++++++++++-
drivers/net/wireless/ath/ath10k/debug.h | 15 +++++-
drivers/net/wireless/ath/ath10k/htt.h | 2 +-
drivers/net/wireless/ath/ath10k/htt_rx.c | 47 +++++++++++++++++
drivers/net/wireless/ath/ath10k/hw.h | 35 +++++++++++++
drivers/net/wireless/ath/ath10k/trace.h | 89 ++++++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/wmi.c | 33 ++++++++++++
drivers/net/wireless/ath/ath10k/wmi.h | 6 +++
9 files changed, 306 insertions(+), 3 deletions(-)
--
2.1.1
This is useful for collecting msdu statistics. For every msdu
entry in the pktlog message, corresponding msdu length is determined.
Signed-off-by: Rajkumar Manoharan <[email protected]>
---
drivers/net/wireless/ath/ath10k/htt_rx.c | 48 ++++++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/hw.h | 27 ++++++++++++++++++
drivers/net/wireless/ath/ath10k/trace.h | 33 ++++++++++++++++++++++
3 files changed, 108 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index be24071..22ef0f4 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1569,6 +1569,48 @@ static void ath10k_htt_rx_delba(struct ath10k *ar, struct htt_resp *resp)
spin_unlock_bh(&ar->data_lock);
}
+static void ath10k_htt_pktlog_msdu(struct ath10k *ar,
+ struct ath10k_pktlog_hdr *hdr)
+{
+ struct ath10k_htt *htt = &ar->htt;
+ struct sk_buff *msdu;
+ __le32 num_msdu;
+ __le16 *msdu_id;
+ u16 msdu_len[MAX_PKT_INFO_MSDU_ID], id;
+ u32 i;
+
+ num_msdu = __le32_to_cpu(*((u32 *)hdr->payload)) * 2;
+ if (num_msdu >= MAX_PKT_INFO_MSDU_ID)
+ return;
+
+ msdu_id = (u16 *)&hdr->payload[MSDU_ID_INFO_ID_OFFSET];
+ memset(msdu_len, 0, sizeof(msdu_len));
+
+ spin_lock_bh(&htt->tx_lock);
+ for (i = 0; i < num_msdu; i++) {
+ id = __le16_to_cpu(*msdu_id);
+ if (id >= htt->max_num_pending_tx) {
+ msdu_id++;
+ continue;
+ }
+
+ msdu = htt->pending_tx[id];
+ if (!msdu) {
+ msdu_id++;
+ continue;
+ }
+
+ msdu_len[i] =
+ (hdr->flags & ATH10K_PKTLOG_FLG_TYPE_CLONE_S) ?
+ CLONED_PKT_MSDU_LEN_DEFAULT : (u16)msdu->len;
+ msdu_id++;
+ }
+ spin_unlock_bh(&htt->tx_lock);
+ trace_ath10k_htt_pktlog_msdu(ar, hdr->payload,
+ sizeof(*hdr) + __le16_to_cpu(hdr->size),
+ msdu_len, sizeof(msdu_len));
+}
+
void ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
{
struct ath10k_htt *htt = &ar->htt;
@@ -1680,6 +1722,12 @@ void ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
struct ath10k_pktlog_hdr *hdr =
(struct ath10k_pktlog_hdr *)resp->pktlog_msg.payload;
+ if (__le16_to_cpu(hdr->log_type) ==
+ ATH10K_PKTLOG_TYPE_TX_MSDU_ID) {
+ ath10k_htt_pktlog_msdu(ar, hdr);
+ break;
+ }
+
trace_ath10k_htt_pktlog(ar, resp->pktlog_msg.payload,
sizeof(*hdr) +
__le16_to_cpu(hdr->size));
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 4b86ca3..57a4c72 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -80,6 +80,27 @@ enum ath10k_mcast2ucast_mode {
ATH10K_MCAST2UCAST_ENABLED = 1,
};
+/* Types of packet log events */
+enum ath10k_pktlog_type {
+ ATH10K_PKTLOG_TYPE_TX_CTRL = 1,
+ ATH10K_PKTLOG_TYPE_TX_STAT,
+ ATH10K_PKTLOG_TYPE_TX_MSDU_ID,
+ ATH10K_PKTLOG_TYPE_TX_FRM_HDR,
+ ATH10K_PKTLOG_TYPE_RX_STAT,
+ ATH10K_PKTLOG_TYPE_RC_FIND,
+ ATH10K_PKTLOG_TYPE_RC_UPDATE,
+ ATH10K_PKTLOG_TYPE_TX_VIRT_ADDR,
+ ATH10K_PKTLOG_TYPE_DBG_PRINT,
+ ATH10K_PKTLOG_TYPE_MAX,
+};
+
+enum ath10k_pktlog_flag_type {
+ ATH10K_PKTLOG_FLG_TYPE_LOCAL_S = 0,
+ ATH10K_PKTLOG_FLG_TYPE_REMOTE_S,
+ ATH10K_PKTLOG_FLG_TYPE_CLONE_S,
+ ATH10K_PKTLOG_FLG_TYPE_UNKNOWN_S
+};
+
struct ath10k_pktlog_hdr {
__le16 flags;
__le16 missed_cnt;
@@ -89,6 +110,12 @@ struct ath10k_pktlog_hdr {
u8 payload[0];
} __packed;
+#define MAX_PKT_INFO_MSDU_ID 192
+#define MSDU_ID_INFO_ID_OFFSET ((MAX_PKT_INFO_MSDU_ID >> 3) + 4)
+#define MSDU_ID_INFO_SIZE ((MAX_PKT_INFO_MSDU_ID >> 3) + 4 + \
+ (MAX_PKT_INFO_MSDU_ID * 2))
+#define CLONED_PKT_MSDU_LEN_DEFAULT 200
+
/* Target specific defines for MAIN firmware */
#define TARGET_NUM_VDEVS 8
#define TARGET_NUM_PEER_AST 2
diff --git a/drivers/net/wireless/ath/ath10k/trace.h b/drivers/net/wireless/ath/ath10k/trace.h
index 3dff799..4f8a20a 100644
--- a/drivers/net/wireless/ath/ath10k/trace.h
+++ b/drivers/net/wireless/ath/ath10k/trace.h
@@ -310,6 +310,39 @@ TRACE_EVENT(ath10k_rx_pktlog,
__entry->len
)
);
+
+TRACE_EVENT(ath10k_htt_pktlog_msdu,
+ TP_PROTO(struct ath10k *ar, void *buf, u16 buf_len,
+ void *msdu, u16 msdu_len),
+
+ TP_ARGS(ar, buf, buf_len, msdu, msdu_len),
+
+ TP_STRUCT__entry(
+ __string(device, dev_name(ar->dev))
+ __string(driver, dev_driver_string(ar->dev))
+ __field(u16, buf_len)
+ __dynamic_array(u8, pktlog, buf_len)
+ __field(u16, msdu_len)
+ __dynamic_array(u8, msdu, msdu_len)
+ ),
+
+ TP_fast_assign(
+ __assign_str(device, dev_name(ar->dev));
+ __assign_str(driver, dev_driver_string(ar->dev));
+ __entry->buf_len = buf_len;
+ memcpy(__get_dynamic_array(pktlog), buf, buf_len);
+ __entry->msdu_len = msdu_len;
+ memcpy(__get_dynamic_array(msdu), msdu, msdu_len);
+ ),
+
+ TP_printk(
+ "%s %s size %hu msdu size %hu",
+ __get_str(driver),
+ __get_str(device),
+ __entry->buf_len,
+ __entry->msdu_len
+ )
+);
#endif /* _TRACE_H_ || TRACE_HEADER_MULTI_READ*/
/* we don't want to use include/trace/events */
--
2.1.1
Upon the reception of frame, the descriptor status are reported
to user space by tracepoint. This is useful for collecting rx
statistics for pktlog.
Signed-off-by: Rajkumar Manoharan <[email protected]>
---
drivers/net/wireless/ath/ath10k/debug.c | 17 +++++++++++++++++
drivers/net/wireless/ath/ath10k/debug.h | 5 +++++
drivers/net/wireless/ath/ath10k/htt_rx.c | 2 ++
drivers/net/wireless/ath/ath10k/trace.h | 29 +++++++++++++++++++++++++++++
4 files changed, 53 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 48672da..29fa820 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -1439,6 +1439,23 @@ void ath10k_debug_unregister(struct ath10k *ar)
cancel_delayed_work_sync(&ar->debug.htt_stats_dwork);
}
+void ath10k_debug_pktlog_rx(struct ath10k *ar, struct sk_buff *msdu)
+{
+ struct htt_rx_desc *rxd;
+ u32 tsf;
+
+ if (!(ar->debug.pktlog_filter & ATH10K_PKTLOG_RX))
+ return;
+
+ while (msdu) {
+ rxd = (void *)msdu->data - sizeof(*rxd);
+ tsf = __le32_to_cpu(rxd->ppdu_end.tsf_timestamp);
+
+ trace_ath10k_rx_pktlog(ar, tsf, &rxd->attention,
+ sizeof(*rxd) - sizeof(u32));
+ msdu = msdu->next;
+ }
+}
#endif /* CONFIG_ATH10K_DEBUGFS */
#ifdef CONFIG_ATH10K_DEBUG
diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h
index aafdcab..389387c 100644
--- a/drivers/net/wireless/ath/ath10k/debug.h
+++ b/drivers/net/wireless/ath/ath10k/debug.h
@@ -71,6 +71,7 @@ ath10k_debug_get_new_fw_crash_data(struct ath10k *ar);
void ath10k_debug_dbglog_add(struct ath10k *ar, u8 *buffer, int len);
#define ATH10K_DFS_STAT_INC(ar, c) (ar->debug.dfs_stats.c++)
+void ath10k_debug_pktlog_rx(struct ath10k *ar, struct sk_buff *msdu);
#else
static inline int ath10k_debug_start(struct ath10k *ar)
{
@@ -121,6 +122,10 @@ ath10k_debug_get_new_fw_crash_data(struct ath10k *ar)
return NULL;
}
+static inline void ath10k_debug_pktlog_rx(struct ath10k *ar,
+ struct sk_buff *msdu)
+{
+}
#define ATH10K_DFS_STAT_INC(ar, c) do { } while (0)
#endif /* CONFIG_ATH10K_DEBUGFS */
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index a078451..be24071 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1295,6 +1295,8 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
continue;
}
+ ath10k_debug_pktlog_rx(ar, msdu_head);
+
rxd = container_of((void *)msdu_head->data,
struct htt_rx_desc,
msdu_payload);
diff --git a/drivers/net/wireless/ath/ath10k/trace.h b/drivers/net/wireless/ath/ath10k/trace.h
index 971ff23..3dff799 100644
--- a/drivers/net/wireless/ath/ath10k/trace.h
+++ b/drivers/net/wireless/ath/ath10k/trace.h
@@ -281,6 +281,35 @@ TRACE_EVENT(ath10k_htt_pktlog,
)
);
+TRACE_EVENT(ath10k_rx_pktlog,
+ TP_PROTO(struct ath10k *ar, u32 tsf, void *rxdesc, u16 len),
+
+ TP_ARGS(ar, tsf, rxdesc, len),
+
+ TP_STRUCT__entry(
+ __string(device, dev_name(ar->dev))
+ __string(driver, dev_driver_string(ar->dev))
+ __field(u32, tsf)
+ __field(u16, len)
+ __dynamic_array(u8, rxdesc, len)
+ ),
+
+ TP_fast_assign(
+ __assign_str(device, dev_name(ar->dev));
+ __assign_str(driver, dev_driver_string(ar->dev));
+ __entry->tsf = tsf;
+ __entry->len = len;
+ memcpy(__get_dynamic_array(rxdesc), rxdesc, len);
+ ),
+
+ TP_printk(
+ "%s %s %u len %hu",
+ __get_str(driver),
+ __get_str(device),
+ __entry->tsf,
+ __entry->len
+ )
+);
#endif /* _TRACE_H_ || TRACE_HEADER_MULTI_READ*/
/* we don't want to use include/trace/events */
--
2.1.1
Add support to configure packet log filters (tx, rx, rate control)
via debugfs. To disable htt pktlog events set the filters to 0.
ex:
To enable pktlog for all filters
echo 0x1f > /sys/kernel/debug/ieee80211/phy*/ath10k/pktlog_filter
To disable pktlog
echo 0 > /sys/kernel/debug/ieee80211/phy*/ath10k/pktlog_filter
Signed-off-by: Rajkumar Manoharan <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.h | 2 ++
drivers/net/wireless/ath/ath10k/debug.c | 63 ++++++++++++++++++++++++++++++++-
drivers/net/wireless/ath/ath10k/debug.h | 9 +++++
drivers/net/wireless/ath/ath10k/wmi.c | 33 +++++++++++++++++
drivers/net/wireless/ath/ath10k/wmi.h | 6 ++++
5 files changed, 112 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 674e38c..f361226 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -302,7 +302,9 @@ struct ath10k_debug {
struct ath10k_dfs_stats dfs_stats;
struct ath_dfs_pool_stats dfs_pool_stats;
+ /* protected by conf_mutex */
u32 fw_dbglog_mask;
+ u32 pktlog_filter;
u8 htt_max_amsdu;
u8 htt_max_ampdu;
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 680d508..48672da 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -1195,7 +1195,17 @@ int ath10k_debug_start(struct ath10k *ar)
ret);
}
- return 0;
+ if (ar->debug.pktlog_filter)
+ ret = ath10k_wmi_pdev_pktlog_enable(ar,
+ ar->debug.pktlog_filter);
+ else
+ ret = ath10k_wmi_pdev_pktlog_disable(ar);
+
+ if (ret)
+ ath10k_warn(ar, "failed to send pktlog command: %d filter %x",
+ ret, ar->debug.pktlog_filter);
+
+ return ret;
}
void ath10k_debug_stop(struct ath10k *ar)
@@ -1292,6 +1302,54 @@ static const struct file_operations fops_dfs_stats = {
.llseek = default_llseek,
};
+static ssize_t ath10k_write_pktlog_filter(struct file *file,
+ const char __user *ubuf,
+ size_t count, loff_t *ppos)
+{
+ struct ath10k *ar = file->private_data;
+ u32 filter;
+ int ret;
+
+ if (kstrtouint_from_user(ubuf, count, 0, &filter))
+ return -EINVAL;
+
+ mutex_lock(&ar->conf_mutex);
+ if (filter && (filter != ar->debug.pktlog_filter))
+ ret = ath10k_wmi_pdev_pktlog_enable(ar, filter);
+ else
+ ret = ath10k_wmi_pdev_pktlog_disable(ar);
+
+ ar->debug.pktlog_filter = filter;
+ mutex_unlock(&ar->conf_mutex);
+
+ if (ret)
+ ath10k_warn(ar, "failed to send pktlog command: %d filter %x",
+ ret, ar->debug.pktlog_filter);
+
+ return count;
+}
+
+static ssize_t ath10k_read_pktlog_filter(struct file *file, char __user *ubuf,
+ size_t count, loff_t *ppos)
+{
+ char buf[32];
+ struct ath10k *ar = file->private_data;
+ int len = 0;
+
+ mutex_lock(&ar->conf_mutex);
+ len = scnprintf(buf, sizeof(buf) - len, "%08x\n",
+ ar->debug.pktlog_filter);
+ mutex_unlock(&ar->conf_mutex);
+
+ return simple_read_from_buffer(ubuf, count, ppos, buf, len);
+}
+
+static const struct file_operations fops_pktlog_filter = {
+ .read = ath10k_read_pktlog_filter,
+ .write = ath10k_write_pktlog_filter,
+ .open = simple_open
+};
+
int ath10k_debug_create(struct ath10k *ar)
{
ar->debug.fw_crash_data = vzalloc(sizeof(*ar->debug.fw_crash_data));
@@ -1370,6 +1428,9 @@ int ath10k_debug_register(struct ath10k *ar)
&fops_dfs_stats);
}
+ debugfs_create_file("pktlog_filter", S_IRUGO | S_IWUSR,
+ ar->debug.debugfs_phy, ar, &fops_pktlog_filter);
+
return 0;
}
diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h
index efbcd29..794e6bb 100644
--- a/drivers/net/wireless/ath/ath10k/debug.h
+++ b/drivers/net/wireless/ath/ath10k/debug.h
@@ -38,6 +38,15 @@ enum ath10k_debug_mask {
ATH10K_DBG_ANY = 0xffffffff,
};
+enum ath10k_pktlog_filter {
+ ATH10K_PKTLOG_RX = 0x000000001,
+ ATH10K_PKTLOG_TX = 0x000000002,
+ ATH10K_PKTLOG_RCFIND = 0x000000004,
+ ATH10K_PKTLOG_RCUPDATE = 0x000000008,
+ ATH10K_PKTLOG_DBG_PRINT = 0x000000010,
+ ATH10K_PKTLOG_ANY = 0x00000001f,
+};
+
extern unsigned int ath10k_debug_mask;
__printf(2, 3) void ath10k_info(struct ath10k *ar, const char *fmt, ...);
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index f65032f..c145b98 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -4345,6 +4345,39 @@ int ath10k_wmi_dbglog_cfg(struct ath10k *ar, u32 module_enable)
return ath10k_wmi_cmd_send(ar, skb, ar->wmi.cmd->dbglog_cfg_cmdid);
}
+int ath10k_wmi_pdev_pktlog_enable(struct ath10k *ar, u32 ev_bitmap)
+{
+ struct wmi_pdev_pktlog_enable_cmd *cmd;
+ struct sk_buff *skb;
+
+ skb = ath10k_wmi_alloc_skb(ar, sizeof(*cmd));
+ if (!skb)
+ return -ENOMEM;
+
+ ev_bitmap &= ATH10K_PKTLOG_ANY;
+ ath10k_dbg(ar, ATH10K_DBG_WMI,
+ "wmi enable pktlog filter:%x\n", ev_bitmap);
+
+ cmd = (struct wmi_pdev_pktlog_enable_cmd *)skb->data;
+ cmd->ev_bitmap = __cpu_to_le32(ev_bitmap);
+ return ath10k_wmi_cmd_send(ar, skb,
+ ar->wmi.cmd->pdev_pktlog_enable_cmdid);
+}
+
+int ath10k_wmi_pdev_pktlog_disable(struct ath10k *ar)
+{
+ struct sk_buff *skb;
+
+ skb = ath10k_wmi_alloc_skb(ar, 0);
+ if (!skb)
+ return -ENOMEM;
+
+ ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi disable pktlog\n");
+
+ return ath10k_wmi_cmd_send(ar, skb,
+ ar->wmi.cmd->pdev_pktlog_disable_cmdid);
+}
+
int ath10k_wmi_attach(struct ath10k *ar)
{
if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features)) {
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 6243dbe..a38d788 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -2787,6 +2787,10 @@ struct wmi_pdev_set_channel_cmd {
struct wmi_channel chan;
} __packed;
+struct wmi_pdev_pktlog_enable_cmd {
+ __le32 ev_bitmap;
+} __packed;
+
/* Customize the DSCP (bit) to TID (0-7) mapping for QOS */
#define WMI_DSCP_MAP_MAX (64)
struct wmi_pdev_set_dscp_tid_map_cmd {
@@ -4647,5 +4651,7 @@ int ath10k_wmi_mgmt_tx(struct ath10k *ar, struct sk_buff *skb);
int ath10k_wmi_dbglog_cfg(struct ath10k *ar, u32 module_enable);
int ath10k_wmi_pull_fw_stats(struct ath10k *ar, struct sk_buff *skb,
struct ath10k_fw_stats *stats);
+int ath10k_wmi_pdev_pktlog_enable(struct ath10k *ar, u32 ev_list);
+int ath10k_wmi_pdev_pktlog_disable(struct ath10k *ar);
#endif /* _WMI_H_ */
--
2.1.1
On 2 October 2014 14:53, Rajkumar Manoharan <[email protected]> wrote:
> On Thu, Oct 02, 2014 at 02:37:13PM +0200, Michal Kazior wrote:
>> On 2 October 2014 14:12, Rajkumar Manoharan <[email protected]> wrote:
>> > Add support to configure packet log filters (tx, rx, rate control)
>> > via debugfs. To disable htt pktlog events set the filters to 0.
>> [...]
>> > +static ssize_t ath10k_write_pktlog_filter(struct file *file,
>> > + const char __user *ubuf,
>> > + size_t count, loff_t *ppos)
>> > +{
>> > + struct ath10k *ar = file->private_data;
>> > + u32 filter;
>> > + int ret;
>> > +
>> > + if (kstrtouint_from_user(ubuf, count, 0, &filter))
>> > + return -EINVAL;
>> > +
>> > + mutex_lock(&ar->conf_mutex);
>> > + if (filter && (filter != ar->debug.pktlog_filter))
>> > + ret = ath10k_wmi_pdev_pktlog_enable(ar, filter);
>> > + else
>> > + ret = ath10k_wmi_pdev_pktlog_disable(ar);
>>
>> You're not checking if ar->state is ON.
>>
> How about updating pktlog_filter alone and not sending WMI command when
> state != ON. So that in the next drv_start pktlog will be either be
> disabled or enabled based on filter value at debug_start.
Sounds good to me.
>> Even if I assume the above doesn't crash the driver/system this has
>> strange semantics. On one hand the driver has to be after drv_start
>> (e.g. at least one interface must be up) before it's possible to set
>> it but on the other hand pktlog_filter is retained across
>> drv_stop/start. I'd expect it to be possible to set the pktlog_filter
>> regardless of driver state but maybe that's just me.
>>
> So pktlog should be disabed at ath10k_debug_stop. isn't it?
Actually my bad. You set ar->debug.pktlog_filter regardless of wmi
command result which is fine.
Michał
On Thu, Oct 02, 2014 at 02:58:43PM +0200, Michal Kazior wrote:
> On 2 October 2014 14:53, Rajkumar Manoharan <[email protected]> wrote:
> > On Thu, Oct 02, 2014 at 02:37:13PM +0200, Michal Kazior wrote:
> >> On 2 October 2014 14:12, Rajkumar Manoharan <[email protected]> wrote:
> >> > Add support to configure packet log filters (tx, rx, rate control)
> >> > via debugfs. To disable htt pktlog events set the filters to 0.
> >> [...]
> >> > +static ssize_t ath10k_write_pktlog_filter(struct file *file,
> >> > + const char __user *ubuf,
> >> > + size_t count, loff_t *ppos)
> >> > +{
> >> > + struct ath10k *ar = file->private_data;
> >> > + u32 filter;
> >> > + int ret;
> >> > +
> >> > + if (kstrtouint_from_user(ubuf, count, 0, &filter))
> >> > + return -EINVAL;
> >> > +
> >> > + mutex_lock(&ar->conf_mutex);
> >> > + if (filter && (filter != ar->debug.pktlog_filter))
> >> > + ret = ath10k_wmi_pdev_pktlog_enable(ar, filter);
> >> > + else
> >> > + ret = ath10k_wmi_pdev_pktlog_disable(ar);
> >>
> >> You're not checking if ar->state is ON.
> >>
> > How about updating pktlog_filter alone and not sending WMI command when
> > state != ON. So that in the next drv_start pktlog will be either be
> > disabled or enabled based on filter value at debug_start.
>
> Sounds good to me.
>
>
> >> Even if I assume the above doesn't crash the driver/system this has
> >> strange semantics. On one hand the driver has to be after drv_start
> >> (e.g. at least one interface must be up) before it's possible to set
> >> it but on the other hand pktlog_filter is retained across
> >> drv_stop/start. I'd expect it to be possible to set the pktlog_filter
> >> regardless of driver state but maybe that's just me.
> >>
> > So pktlog should be disabed at ath10k_debug_stop. isn't it?
>
> Actually my bad. You set ar->debug.pktlog_filter regardless of wmi
> command result which is fine.
>
Thanks for your quick feedback.
-Rajkumar
On 2 October 2014 14:12, Rajkumar Manoharan <[email protected]> wrote:
> Add support to configure packet log filters (tx, rx, rate control)
> via debugfs. To disable htt pktlog events set the filters to 0.
[...]
> +static ssize_t ath10k_write_pktlog_filter(struct file *file,
> + const char __user *ubuf,
> + size_t count, loff_t *ppos)
> +{
> + struct ath10k *ar = file->private_data;
> + u32 filter;
> + int ret;
> +
> + if (kstrtouint_from_user(ubuf, count, 0, &filter))
> + return -EINVAL;
> +
> + mutex_lock(&ar->conf_mutex);
> + if (filter && (filter != ar->debug.pktlog_filter))
> + ret = ath10k_wmi_pdev_pktlog_enable(ar, filter);
> + else
> + ret = ath10k_wmi_pdev_pktlog_disable(ar);
You're not checking if ar->state is ON.
Even if I assume the above doesn't crash the driver/system this has
strange semantics. On one hand the driver has to be after drv_start
(e.g. at least one interface must be up) before it's possible to set
it but on the other hand pktlog_filter is retained across
drv_stop/start. I'd expect it to be possible to set the pktlog_filter
regardless of driver state but maybe that's just me.
Michał
This is useful for collecting pktlog statistics of tx, rx
and rate information, so add tracing for the API call.
Signed-off-by: Rajkumar Manoharan <[email protected]>
---
drivers/net/wireless/ath/ath10k/debug.h | 1 -
drivers/net/wireless/ath/ath10k/htt.h | 2 +-
drivers/net/wireless/ath/ath10k/htt_rx.c | 9 +++++++++
drivers/net/wireless/ath/ath10k/hw.h | 9 +++++++++
drivers/net/wireless/ath/ath10k/trace.h | 27 +++++++++++++++++++++++++++
5 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h
index 794e6bb..aafdcab 100644
--- a/drivers/net/wireless/ath/ath10k/debug.h
+++ b/drivers/net/wireless/ath/ath10k/debug.h
@@ -69,7 +69,6 @@ struct ath10k_fw_crash_data *
ath10k_debug_get_new_fw_crash_data(struct ath10k *ar);
void ath10k_debug_dbglog_add(struct ath10k *ar, u8 *buffer, int len);
-
#define ATH10K_DFS_STAT_INC(ar, c) (ar->debug.dfs_stats.c++)
#else
diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 3b44217..15c58e8 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -725,7 +725,7 @@ static inline u8 *htt_rx_test_get_chars(struct htt_rx_test *rx_test)
*/
struct htt_pktlog_msg {
u8 pad[3];
- __le32 payload[1 /* or more */];
+ u8 payload[0];
} __packed;
struct htt_dbg_stats_rx_reorder_stats {
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 60d40a0..a078451 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1674,6 +1674,15 @@ void ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
case HTT_T2H_MSG_TYPE_RX_DELBA:
ath10k_htt_rx_delba(ar, resp);
break;
+ case HTT_T2H_MSG_TYPE_PKTLOG: {
+ struct ath10k_pktlog_hdr *hdr =
+ (struct ath10k_pktlog_hdr *)resp->pktlog_msg.payload;
+
+ trace_ath10k_htt_pktlog(ar, resp->pktlog_msg.payload,
+ sizeof(*hdr) +
+ __le16_to_cpu(hdr->size));
+ break;
+ }
case HTT_T2H_MSG_TYPE_RX_FLUSH: {
/* Ignore this event because mac80211 takes care of Rx
* aggregation reordering.
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 006a9cb..4b86ca3 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -80,6 +80,15 @@ enum ath10k_mcast2ucast_mode {
ATH10K_MCAST2UCAST_ENABLED = 1,
};
+struct ath10k_pktlog_hdr {
+ __le16 flags;
+ __le16 missed_cnt;
+ __le16 log_type;
+ __le16 size;
+ __le32 timestamp;
+ u8 payload[0];
+} __packed;
+
/* Target specific defines for MAIN firmware */
#define TARGET_NUM_VDEVS 8
#define TARGET_NUM_PEER_AST 2
diff --git a/drivers/net/wireless/ath/ath10k/trace.h b/drivers/net/wireless/ath/ath10k/trace.h
index 574b75a..971ff23 100644
--- a/drivers/net/wireless/ath/ath10k/trace.h
+++ b/drivers/net/wireless/ath/ath10k/trace.h
@@ -254,6 +254,33 @@ TRACE_EVENT(ath10k_wmi_dbglog,
)
);
+TRACE_EVENT(ath10k_htt_pktlog,
+ TP_PROTO(struct ath10k *ar, void *buf, u16 buf_len),
+
+ TP_ARGS(ar, buf, buf_len),
+
+ TP_STRUCT__entry(
+ __string(device, dev_name(ar->dev))
+ __string(driver, dev_driver_string(ar->dev))
+ __field(u16, buf_len)
+ __dynamic_array(u8, pktlog, buf_len)
+ ),
+
+ TP_fast_assign(
+ __assign_str(device, dev_name(ar->dev));
+ __assign_str(driver, dev_driver_string(ar->dev));
+ __entry->buf_len = buf_len;
+ memcpy(__get_dynamic_array(pktlog), buf, buf_len);
+ ),
+
+ TP_printk(
+ "%s %s size %hu",
+ __get_str(driver),
+ __get_str(device),
+ __entry->buf_len
+ )
+);
+
#endif /* _TRACE_H_ || TRACE_HEADER_MULTI_READ*/
/* we don't want to use include/trace/events */
--
2.1.1
On Thu, Oct 02, 2014 at 02:37:13PM +0200, Michal Kazior wrote:
> On 2 October 2014 14:12, Rajkumar Manoharan <[email protected]> wrote:
> > Add support to configure packet log filters (tx, rx, rate control)
> > via debugfs. To disable htt pktlog events set the filters to 0.
> [...]
> > +static ssize_t ath10k_write_pktlog_filter(struct file *file,
> > + const char __user *ubuf,
> > + size_t count, loff_t *ppos)
> > +{
> > + struct ath10k *ar = file->private_data;
> > + u32 filter;
> > + int ret;
> > +
> > + if (kstrtouint_from_user(ubuf, count, 0, &filter))
> > + return -EINVAL;
> > +
> > + mutex_lock(&ar->conf_mutex);
> > + if (filter && (filter != ar->debug.pktlog_filter))
> > + ret = ath10k_wmi_pdev_pktlog_enable(ar, filter);
> > + else
> > + ret = ath10k_wmi_pdev_pktlog_disable(ar);
>
> You're not checking if ar->state is ON.
>
How about updating pktlog_filter alone and not sending WMI command when
state != ON. So that in the next drv_start pktlog will be either be
disabled or enabled based on filter value at debug_start.
> Even if I assume the above doesn't crash the driver/system this has
> strange semantics. On one hand the driver has to be after drv_start
> (e.g. at least one interface must be up) before it's possible to set
> it but on the other hand pktlog_filter is retained across
> drv_stop/start. I'd expect it to be possible to set the pktlog_filter
> regardless of driver state but maybe that's just me.
>
So pktlog should be disabed at ath10k_debug_stop. isn't it?
-Rajkumar