RE: [PATCH 15/50] wifi: ath12k: add dp_rx.c

Sriram R (QUIC)
2022-10-13T05:55:06Z

>> +static void ath12k_dp_rx_process_received_packets(struct ath12k_base
>*ab,
>> + struct napi_struct *napi,
>> + struct sk_buff_head *msdu_list,
>> + int ring_id) {
>> + struct ieee80211_rx_status rx_status = {0};
>
>initializing just on entry seems wrong, see below
We reset the whole struct once, and reset the necessary fields alone for every loop within ath12k_dp_rx_h_ppdu or similar places as necessary
and pass the status to mac80211.
The initial reset is necessary so as to reset all unused fields. This is an optimization to not memset the struct for every loop.
>
>> + struct ath12k_skb_rxcb *rxcb;
>> + struct sk_buff *msdu;
>> + struct ath12k *ar;
>> + u8 mac_id;
>> + int ret;
>> +
>> + if (skb_queue_empty(msdu_list))
>> + return;
>> +
>> + rcu_read_lock();
>> +
>> + while ((msdu = __skb_dequeue(msdu_list))) {
>> + rxcb = ATH12K_SKB_RXCB(msdu);
>> + mac_id = rxcb->mac_id;
>> + ar = ab->pdevs[mac_id].ar;
>> + if (!rcu_dereference(ab->pdevs_active[mac_id])) {
>> + dev_kfree_skb_any(msdu);
>> + continue;
>> + }
>> +
>> + if (test_bit(ATH12K_CAC_RUNNING, &ar->dev_flags)) {
>> + dev_kfree_skb_any(msdu);
>> + continue;
>> + }
>> +
>> + ret = ath12k_dp_rx_process_msdu(ar, msdu, msdu_list,
>> + &rx_status);
>
>you don't need to reinitialize rx_status for every iteration of the loop?
Same as above comment.
>
>> + if (ret) {
>> + ath12k_dbg(ab, ATH12K_DBG_DATA,
>> + "Unable to process msdu %d", ret);
>> + dev_kfree_skb_any(msdu);
>> + continue;
>> + }
>> +
>> + ath12k_dp_rx_deliver_msdu(ar, napi, msdu, &rx_status);
>> + }
>> +
>> + rcu_read_unlock();
>> +}
Thanks,
Sriram.R