[PATCH v4] wifi: ath12k: Add support for link specific datapath stats

[Jeff Johnson]

On 2/4/2025 10:37 PM, Tamizh Chelvam Raja (QUIC) wrote:
...
>> +static int ath12k_open_link_stats(struct inode *inode, struct file
>> +*file) {
>> +       struct ath12k_vif *ahvif = inode->i_private;
>> +       size_t len = 0, buf_len = (PAGE_SIZE * 2);
>> +       struct ath12k_link_stats linkstat;
>> +       struct ath12k_link_vif *arvif;
>> +       unsigned long links_map;
>> +       struct wiphy *wiphy;
>> +       int link_id, i;
>> +       char *buf;
>> +
>> +       if (!ahvif)
>> +               return -EINVAL;
>> +
>> +       buf = kzalloc(buf_len, GFP_KERNEL);
>> +       if (!buf)
>> +               return -ENOMEM;
>> +
>> +       wiphy = ahvif->ah->hw->wiphy;
>> +       wiphy_lock(wiphy);
>> +
>> +       links_map = ahvif->links_map;
>> +       for_each_set_bit(link_id, &links_map,
>> +                        IEEE80211_MLD_MAX_NUM_LINKS) {
>> +               arvif = rcu_dereference_protected(ahvif->link[link_id],
>> +
>> + lockdep_is_held(&wiphy->mtx));
>> +
> 
> Here arvif can be NULL, so it would be good to check before using it.
> 
>> +               spin_lock_bh(&arvif->link_stats_lock);
>> +               linkstat = arvif->link_stats;
>> +               spin_unlock_bh(&arvif->link_stats_lock);
>> +
>> +               len += scnprintf(buf + len, buf_len - len,
>> +                                "link[%d] Tx Unicast Frames Enqueued  = %d\n",
>> +                                link_id, linkstat.tx_enqueued);
>> +               len += scnprintf(buf + len, buf_len - len,
>> +                                "link[%d] Tx Broadcast Frames Enqueued = %d\n",
>> +                                link_id, linkstat.tx_bcast_mcast);
>> +               len += scnprintf(buf + len, buf_len - len,
>> +                                "link[%d] Tx Frames Completed = %d\n",
>> +                                link_id, linkstat.tx_completed);
>> +               len += scnprintf(buf + len, buf_len - len,
>> +                                "link[%d] Tx Frames Dropped = %d\n",
>> +                                link_id, linkstat.tx_dropped);
>> +
>> +               len += scnprintf(buf + len, buf_len - len,
>> +                                "link[%d] Tx Frame descriptor Encap Type = ",
>> +                                link_id);
>> +
>> +               len += scnprintf(buf + len, buf_len - len,
>> +                                        " raw:%d",
>> +                                        linkstat.tx_encap_type[0]);
>> +
>> +               len += scnprintf(buf + len, buf_len - len,
>> +                                        " native_wifi:%d",
>> +                                        linkstat.tx_encap_type[1]);
>> +
>> +               len += scnprintf(buf + len, buf_len - len,
>> +                                        " ethernet:%d",
>> +                                        linkstat.tx_encap_type[2]);
> 
> Like encrypt type stats below this also can be put it in a loop.
> 
>> +
>> +               len += scnprintf(buf + len, buf_len - len,
>> +                                "\nlink[%d] Tx Frame descriptor Encrypt Type = ",
>> +                                link_id);
>> +
>> +               for (i = 0; i < HAL_ENCRYPT_TYPE_MAX; i++) {
>> +                       len += scnprintf(buf + len, buf_len - len,
>> +                                        " %d:%d", i,
>> +                                        linkstat.tx_encrypt_type[i]);
>> +               }
>> +               len += scnprintf(buf + len, buf_len - len,
>> +                                "\nlink[%d] Tx Frame descriptor Type = buffer:%d
>> extension:%d\n",
>> +                                link_id, linkstat.tx_desc_type[0],
>> +                                linkstat.tx_desc_type[1]);
>> +
>> +               len += scnprintf(buf + len, buf_len - len,
>> +                               "------------------------------------------------------\n");
>> +       }
>> +
>> +       wiphy_unlock(wiphy);
>> +
>> +       file->private_data = buf;
>> +
>> +       return 0;
>> +}

Hi Maha,

I didn't see any response to the two feedback comments given.

I'd like to clear this patch from my backlog, so either need a response why
the feedback isn't applicable or need a v5 which incorporates the feedback.

Thanks!
/jeff