On 2/28/2025 6:46 PM, Johannes Berg wrote:
On Thu, 2025-02-13 at 22:46 +0530, Sarika Sharma wrote:
- if (!sta->deflink.pcpu_rx_stats)
+ if (link_id < 0)
+ link_sta_info = &sta->deflink;
+ else
+ link_sta_info =
+ rcu_dereference_protected(sta->link[link_id],
+ lockdep_is_held(&sta->local->hw.wiphy->mtx));
We have all kinds of helper macros for that? Even this very specific
case: link_sta_dereference_protected()
Sure, will check this and use link_sta_dereference_protected() instead.
+ stats = &link_sta_info->rx_stats;
Should you check that link_sta_info even exists, just in case some link
IDs get mixed up? Not sure.
No, here it is not required, as already lock is acquired and checked
existence of link_sta_info in it's caller.
-unsigned long ieee80211_sta_last_active(struct sta_info *sta)
+unsigned long ieee80211_sta_last_active(struct sta_info *sta, int link_id)
{
- struct ieee80211_sta_rx_stats *stats = sta_get_last_rx_stats(sta);
+ struct ieee80211_sta_rx_stats *stats = sta_get_last_rx_stats(sta, link_id);
+ struct link_sta_info *link_sta_info;
+
+ if (link_id < 0)
+ link_sta_info = &sta->deflink;
+ else
+ link_sta_info =
+ rcu_dereference_protected(sta->link[link_id],
+ lockdep_is_held(&sta->local->hw.wiphy->mtx));
- if (!sta->deflink.status_stats.last_ack ||
- time_after(stats->last_rx, sta->deflink.status_stats.last_ack))
+ if (!link_sta_info->status_stats.last_ack ||
+ time_after(stats->last_rx, link_sta_info->status_stats.last_ack))
return stats->last_rx;
- return sta->deflink.status_stats.last_ack;
+
+ return link_sta_info->status_stats.last_ack;
}
This seems wrong, if you ask for -1 you get deflink but that's no longer
updated at all, so you break the current/updated sta_set_sinfo() usage
with this since you just use -1 statically there now (with this patch.)
Yes, I updated the usage, passing -1 as of now with this patch.
But deflink is still filled whenever we use -1.
Example:
Here, in sta_set_tidstats() we check
if (link_id < 0)
+ link_sta_info = &sta->deflink;
static void sta_update_codel_params(struct sta_info *sta, u32 thr)
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 07b7ec39a52f..7e600c82a6e1 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -947,7 +947,7 @@ void ieee80211_sta_ps_deliver_wakeup(struct sta_info *sta);
void ieee80211_sta_ps_deliver_poll_response(struct sta_info *sta);
void ieee80211_sta_ps_deliver_uapsd(struct sta_info *sta);
-unsigned long ieee80211_sta_last_active(struct sta_info *sta);
+unsigned long ieee80211_sta_last_active(struct sta_info *sta, int link_id);
void ieee80211_sta_set_max_amsdu_subframes(struct sta_info *sta,
const u8 *ext_capab,
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index f6b631faf4f7..1e2cb33030da 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -3276,14 +3276,28 @@ int ieee80211_put_srates_elem(struct sk_buff *skb,
return 0;
}
-int ieee80211_ave_rssi(struct ieee80211_vif *vif)
+int ieee80211_ave_rssi(struct ieee80211_vif *vif, int link_id)
{
struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+ struct ieee80211_link_data *link_data;
+ int rssi;
if (WARN_ON_ONCE(sdata->vif.type != NL80211_IFTYPE_STATION))
return 0;
- return -ewma_beacon_signal_read(&sdata->deflink.u.mgd.ave_beacon_signal);
+ if (link_id < 0)
+ link_data = &sdata->deflink;
+ else
+ link_data =
+ rcu_dereference_protected(sdata->link[link_id],
+ lockdep_is_held(&sdata->local->hw.wiphy->mtx));
+
+ if (WARN_ON(!link_data))
+ return -99;
+
+ rssi = -ewma_beacon_signal_read(&link_data->u.mgd.ave_beacon_signal);
+
+ return rssi;
what's the point in the trivial intermediate 'rssi' variable? It's not
even for line length since "rssi = " is the same length as "return "?
Oops! sure, will use return directly.
johannes
