On 2/10/2025 7:20 PM, Karthikeyan Periyasamy wrote:
>
>
> On 2/10/2025 4:23 PM, Mahendran P wrote:
>> On 2/6/2025 7:08 AM, Karthikeyan Periyasamy wrote:
>>> Currently, monitor is not enabled. However, in the future, the monitor
>>> will be enabled. Therefore, add the necessary HAL_PHYRX_GENERIC_EHT_SIG
>>> TLV parsing support in monitor Rx path, which help to populate the EHT
>>> radiotap data.
>>>
>>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1
>>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>>
>>> Co-developed-by: P Praneesh <quic_ppranees@xxxxxxxxxxx>
>>> Signed-off-by: P Praneesh <quic_ppranees@xxxxxxxxxxx>
>>> Reviewed-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@xxxxxxxxxxxxxxxx>
>>> Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@xxxxxxxxxxx>
>>> ---
>>> drivers/net/wireless/ath/ath12k/dp_mon.c | 548 +++++++++++++++++++++--
>>> drivers/net/wireless/ath/ath12k/hal_rx.h | 136 +++++-
>>> 2 files changed, 638 insertions(+), 46 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath12k/dp_mon.c b/drivers/net/wireless/ath/ath12k/dp_mon.c
>>> index 61fd481b7a3b..dd17607d470d 100644
>>> --- a/drivers/net/wireless/ath/ath12k/dp_mon.c
>>> +++ b/drivers/net/wireless/ath/ath12k/dp_mon.c
>>> @@ -13,6 +13,9 @@
>>> #define ATH12K_LE32_DEC_ENC(value, dec_bits, enc_bits) \
>>> u32_encode_bits(le32_get_bits(value, dec_bits), enc_bits)
>>> +#define ATH12K_LE64_DEC_ENC(value, dec_bits, enc_bits) \
>>> + u32_encode_bits(le64_get_bits(value, dec_bits), enc_bits)
>>
>> u64_encode_bits?
>>
>
> No, Its not needed. Here we need to decode from the u64 and get assign to u32 as defined by eht radiotap header.
Okay. Using le32_encode_bits in the ATH12K_LE64_DEC_ENC macro is likely sufficient with the current mask usage
however it is good to add a comment so that this macro can be used acoordingly in future
>
>
>>> @@ -836,6 +1239,9 @@ ath12k_dp_mon_rx_parse_status_tlv(struct ath12k *ar,
>>> case HAL_RX_PREAMBLE_11AX:
>>> ppdu_info->he_flags = 1;
>>> break;
>>> + case HAL_RX_PREAMBLE_11BE:
>>> + ppdu_info->is_eht = true;
>>> + break;
>>> default:
>>> break;
>>> }
>>> @@ -961,6 +1367,21 @@ ath12k_dp_mon_rx_parse_status_tlv(struct ath12k *ar,
>>> case HAL_PHYRX_GENERIC_U_SIG:
>>> ath12k_dp_mon_hal_rx_parse_u_sig_hdr(tlv_data, ppdu_info);
>>> break;
>>> + case HAL_PHYRX_GENERIC_EHT_SIG:
>>> + /* Handle the case where aggregation is in progress
>>> + * or the current TLV is one of the TLVs which should be
>>> + * aggregated
>>> + */
>>> + if (!ppdu_info->tlv_aggr.in_progress) {
>>> + ppdu_info->tlv_aggr.in_progress = true;
>>> + ppdu_info->tlv_aggr.tlv_tag = tlv_tag;
>>> + ppdu_info->tlv_aggr.cur_len = 0;
>>> + }
>>> +
>>> + ppdu_info->is_eht = true;
>>> +
>>> + ath12k_dp_mon_hal_aggr_tlv(ppdu_info, tlv_len, tlv_data);
>>> + break;
>>> case HAL_DUMMY:
>>> return HAL_RX_MON_STATUS_BUF_DONE;
>>> case HAL_RX_PPDU_END_STATUS_DONE:
>>> @@ -1158,22 +1579,59 @@ static void ath12k_dp_mon_update_radiotap(struct ath12k *ar,
>>> rxs->ampdu_reference = ampdu_id;
>>> }
>>> - if (ppduinfo->eht_usig) {
>>> + if (ppduinfo->is_eht || ppduinfo->eht_usig) {
>>> struct ieee80211_radiotap_tlv *tlv;
>>> + struct ieee80211_radiotap_eht *eht;
>>> struct ieee80211_radiotap_eht_usig *usig;
>>> - u16 len = sizeof(*usig);
>>> + u16 len = 0, i, eht_len, usig_len;
>>> + u8 user;
>>> +
>>> + if (ppduinfo->is_eht) {
>>> + eht_len = struct_size(eht,
>>> + user_info,
>>> + ppduinfo->eht_info.num_user_info);
>>> + len += sizeof(*tlv) + eht_len;
>>> + }
>>> +
>>> + if (ppduinfo->eht_usig) {
>>> + usig_len = sizeof(*usig);
>>> + len += sizeof(*tlv) + usig_len;
>>> + }
>>> rxs->flag |= RX_FLAG_RADIOTAP_TLV_AT_END;
>>> rxs->encoding = RX_ENC_EHT;
>>> skb_reset_mac_header(mon_skb);
>>> - tlv = skb_push(mon_skb, sizeof(*tlv) + len);
>>> - tlv->type = cpu_to_le16(IEEE80211_RADIOTAP_EHT_USIG);
>>> - tlv->len = cpu_to_le16(len);
>>> + tlv = skb_push(mon_skb, len);
>>> +
>>> + if (ppduinfo->is_eht) {
>>> + tlv->type = cpu_to_le16(IEEE80211_RADIOTAP_EHT);
>>> + tlv->len = cpu_to_le16(eht_len);
>>> - usig = (struct ieee80211_radiotap_eht_usig *)tlv->data;
>>> - *usig = ppduinfo->usig;
>>> + eht = (struct ieee80211_radiotap_eht *)tlv->data;
>>> + eht->known = ppduinfo->eht_info.eht.known;
>>> +
>>> + for (i = 0;
>>> + i < ARRAY_SIZE(eht->data) &&
>>> + i < ARRAY_SIZE(ppduinfo->eht_info.eht.data);
>>> + i++)
>>> + eht->data[i] = ppduinfo->eht_info.eht.data[i];
>>> +
>>> + for (user = 0; user < ppduinfo->eht_info.num_user_info; user++)
>>> + put_unaligned_le32(ppduinfo->eht_info.user_info[user],
>>> + &eht->user_info[user]);
>>> +
>>> + tlv = (struct ieee80211_radiotap_tlv *)&tlv->data[eht_len];
>>> + }
>>> +
>>> + if (ppduinfo->eht_usig) {
>>> + tlv->type = cpu_to_le16(IEEE80211_RADIOTAP_EHT_USIG);
>>> + tlv->len = cpu_to_le16(usig_len);
>>> +
>>> + usig = (struct ieee80211_radiotap_eht_usig *)tlv->data;
>>> + *usig = ppduinfo->u_sig_info.usig;
>>> + }
>>> } else if (ppduinfo->he_mu_flags) {
>>> rxs->flag |= RX_FLAG_RADIOTAP_HE_MU;
>>> rxs->encoding = RX_ENC_HE;
>>> diff --git a/drivers/net/wireless/ath/ath12k/hal_rx.h b/drivers/net/wireless/ath/ath12k/hal_rx.h
>>> index 2da16f27e76c..959f2283294c 100644
>>> --- a/drivers/net/wireless/ath/ath12k/hal_rx.h
>>> +++ b/drivers/net/wireless/ath/ath12k/hal_rx.h
>>> @@ -71,6 +71,8 @@ enum hal_rx_preamble {
>>> HAL_RX_PREAMBLE_11N,
>>> HAL_RX_PREAMBLE_11AC,
>>> HAL_RX_PREAMBLE_11AX,
>>> + HAL_RX_PREAMBLE_11BA,
>>
>> is it needed?
>>
>
> It will be good to have the defined values in the enum instead of initialise the HAL_RX_PREAMBLE_11BE = 6 directly.
got it.
>
>>> + HAL_RX_PREAMBLE_11BE,
>>> HAL_RX_PREAMBLE_MAX,
>>> };
>>>
>
