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()
> + 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.
> -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.)
> 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 "?
johannes
