Re: [bug report] ath11k: Fix pktlog lite rx events

[Anilkumar Kolli <akolli@codeaurora.org>]

On 2021-12-21 20:22, Kalle Valo wrote:
> Anilkumar Kolli <akolli@codeaurora.org> writes:
> 
>> On 2021-11-30 16:12, Dan Carpenter wrote:
>>> Hello Anilkumar Kolli,
>>> 
>>> The patch ab18e3bc1c13: "ath11k: Fix pktlog lite rx events" from Sep
>>> 28, 2021, leads to the following Smatch static checker warning:
>>> 
>>> 	drivers/net/wireless/ath/ath11k/dp_rx.c:3105
>>> ath11k_dp_rx_process_mon_status()
>>> 	error: uninitialized symbol 'rx_buf_sz'.
>>> 
>>> drivers/net/wireless/ath/ath11k/dp_rx.c
>>>     3043 int ath11k_dp_rx_process_mon_status(struct ath11k_base *ab,
>>> int mac_id,
>>>     3044                                     struct napi_struct 
>>> *napi,
>>> int budget)
>>>     3045 {
>>>     3046         struct ath11k *ar = ath11k_ab_to_ar(ab, mac_id);
>>>     3047         enum hal_rx_mon_status hal_status;
>>>     3048         struct sk_buff *skb;
>>>     3049         struct sk_buff_head skb_list;
>>>     3050         struct hal_rx_mon_ppdu_info ppdu_info;
>>>     3051         struct ath11k_peer *peer;
>>>     3052         struct ath11k_sta *arsta;
>>>     3053         int num_buffs_reaped = 0;
>>>     3054         u32 rx_buf_sz;
>>>                  ^^^^^^^^^^^^^
>>> 
>>>     3055         u16 log_type = 0;
>>>     3056
>>>     3057         __skb_queue_head_init(&skb_list);
>>>     3058
>>>     3059         num_buffs_reaped =
>>> ath11k_dp_rx_reap_mon_status_ring(ab, mac_id, &budget,
>>>     3060
>>> &skb_list);
>>>     3061         if (!num_buffs_reaped)
>>>     3062                 goto exit;
>>>     3063
>>>     3064         while ((skb = __skb_dequeue(&skb_list))) {
>>>     3065                 memset(&ppdu_info, 0, sizeof(ppdu_info));
>>>     3066                 ppdu_info.peer_id = HAL_INVALID_PEERID;
>>>     3067
>>>     3068                 if
>>> (ath11k_debugfs_is_pktlog_lite_mode_enabled(ar)) {
>>>     3069                         log_type = 
>>> ATH11K_PKTLOG_TYPE_LITE_RX;
>>>     3070                         rx_buf_sz = DP_RX_BUFFER_SIZE_LITE;
>>>     3071                 } else if
>>> (ath11k_debugfs_is_pktlog_rx_stats_enabled(ar)) {
>>>     3072                         log_type =
>>> ATH11K_PKTLOG_TYPE_RX_STATBUF;
>>>     3073                         rx_buf_sz = DP_RX_BUFFER_SIZE;
>>>     3074                 }
>>> 
>>> No else statement.
>>> 
>>>     3075
>>>     3076                 if (log_type)
>>>     3077                         trace_ath11k_htt_rxdesc(ar,
>>> skb->data, log_type, rx_buf_sz);
>>>     3078
>>>     3079                 hal_status =
>>> ath11k_hal_rx_parse_mon_status(ab, &ppdu_info, skb);
>>>     3080
>>>     3081                 if (ppdu_info.peer_id == HAL_INVALID_PEERID 
>>> ||
>>>     3082                     hal_status !=
>>> HAL_RX_MON_STATUS_PPDU_DONE) {
>>>     3083                         dev_kfree_skb_any(skb);
>>>     3084                         continue;
>>>     3085                 }
>>>     3086
>>>     3087                 rcu_read_lock();
>>>     3088                 spin_lock_bh(&ab->base_lock);
>>>     3089                 peer = ath11k_peer_find_by_id(ab,
>>> ppdu_info.peer_id);
>>>     3090
>>>     3091                 if (!peer || !peer->sta) {
>>>     3092                         ath11k_dbg(ab, ATH11K_DBG_DATA,
>>>     3093                                    "failed to find the peer
>>> with peer_id %d\n",
>>>     3094                                    ppdu_info.peer_id);
>>>     3095                         spin_unlock_bh(&ab->base_lock);
>>>     3096                         rcu_read_unlock();
>>>     3097                         dev_kfree_skb_any(skb);
>>>     3098                         continue;
>>>     3099                 }
>>>     3100
>>>     3101                 arsta = (struct ath11k_sta
>>> *)peer->sta->drv_priv;
>>>     3102                 ath11k_dp_rx_update_peer_stats(arsta,
>>> &ppdu_info);
>>>     3103
>>>     3104                 if (ath11k_debugfs_is_pktlog_peer_valid(ar,
>>> peer->addr))
>>> --> 3105                         trace_ath11k_htt_rxdesc(ar,
>>> skb->data, log_type, rx_buf_sz);
>>> 
>>>            ^^^^^^^^^
>>> Warning here.
>>> 
>>>     3106
>>>     3107                 spin_unlock_bh(&ab->base_lock);
>>>     3108                 rcu_read_unlock();
>>>     3109
>>>     3110                 dev_kfree_skb_any(skb);
>>>     3111         }
>>>     3112 exit:
>>>     3113         return num_buffs_reaped;
>>>     3114 }
>>> 
>>> regards,
>>> dan carpenter
>> 
>> Hi Dan carpenter
>> 
>> Thanks for reporting issue. Could you please help in reproducing the
>> issue,
>> 
>> I tried to reproduce the warnings using command, but could not
>>     make C=1 CHECK="smatch -p=kernel" -C.
>> drivers/net/wireless/ath/ath11k/ modules
> 
> I don't see why you need to reproduce the error. rx_buf_sz is
> uninitialised as there's no else statement, like Dan mentioned above.
> 

I thought this script is useful for ath11k.

> Anil, can you send a patch?

Sure Kalle, I will submit a patch.

- Anil.

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k