[PATCH wireless-next v8 00/10] wifi: cfg80211/mac80211: add support to handle per link statistics of multi-link station

[PATCH wireless-next v8 00/10] wifi: cfg80211/mac80211: add support to handle per link statistics of multi-link station

[Sarika Sharma]

Current implementation of NL80211_CMD_GET_STATION does not work
for multi-link operation(MLO) since in case of MLO only deflink
(or one of the links) is considered and not all links.

Hence, add the link_sinfo structure to provide infrastructure
for link-level station statistics for multi-link operation(MLO).

Additionally, accumulated stats for MLO are included in a concise
manner to provide a comprehensive overview of the ML stations.

V8:
 - Fix locking issue.

V7:
 - Redesign the approach according to provided comments( mentioned the
   current implemented approach below in "Purposed Flow").

V6:
 - Fix compilation failure.
 - Add expected_throughput, beacon_loss_count, rx_dropped_misc at MLO
   level.
 - Removed is_per_link_stats_support boolean from sinfo structure.
 - Used ether_addr_copy() instead of memcpy().

V5:
 - Rebased the patches.

V4:
 - Update helper function for link_sta derefrence.                    
 - Instead of using unnecessary rssi variable directly use return.
 - Correct commit tittle and message for some patches.
 - Corrected cfg80211_sinfo_release_content() unnecessary logic.
 - Split the patches for cfg80211/mac80211.
 - Add additional fields at MLO level.
 - Remove Ath12k patche from this series.
 - Remove unnecessary if condition check from "add additional MLO
   statistics".

V3:
 - Convert RFC patch to actual PATCH with each patch bisectable.
 - Add logic for MLO level signal and rates.

V2:
 - Update cover letter to give more details on structural changes.
 - Split the patch(1/7) in two patches.
 - Do the required changes for MLO bringup before as seperate patches.
 - Remove link_sinfo naming to sinfo for better clarity on changes.
 - Add accumulated stats logic in cfg80211.
 - Add flag to indicate driver supports per-link station statistics or not.

Current flow:
  cfg80211:
  - Structure station_info is used to fill all station information (station
    level as well link level).
  - Nl80211_get_station() - locally define sinfo, call -.get_station() ops

  mac80211:
  - Sta_set_sinfo() - fill station information and call mac80211
    ops - .sta_statistics()(to fill the station_info structure).

Purposed Flow:
 cfg80211:
 - introduce link_station_info structure for link level statistics
    structure link_station_info {
      filled
      rates
      ...etc // all link specific fields
    }
  - add link_station_info for MAX links in station_info structure
    structure station_info {
      filled
      packets
      sta_flags
      ... etc 

      valid_links
      link_station_info *links[IEEE80211_MLD_MAX_NUM_LINKS]
    }

 - Station_info structure is used for non-ML station statistics and
   update the sinfo_structure for MLO, for accumulated, best, least
   among all valid links in cfg80211_sta_set_mld_sinfo().
 - Allocate and free memory for link_sinfo for all links.
 - Embed NL message to include link level data, if valid_links are present.

mac80211:
  - Sta_set_sinfo() - fill station specific information as filled before
    if non-ML:
    - call mac80211 ops .sta_statistics() to fill station_info structure.
    if MLO:
    - call sta_set_link_sinfo(), for all valid links to fill
      link_station_info in *links[link_id].
    - introduce and call mac80211 ops for each link- .link_sta_statistics()
      to fill link statistics from drivers.
    - Maintain accumulated data for removed links in sta_info structure
      and fill in to station_info structure for consistency.

Sarika Sharma (10):
  wifi: mac80211: add support towards MLO handling of station statistics
  wifi: cfg80211: add link_station_info structure to support MLO
    statistics
  wifi: cfg80211: extend to embed link level statistics in NL message
  wifi: cfg80211: add statistics for providing overview for MLO station
  wifi: cfg80211: allocate memory for link_station info structure
  wifi: mac80211: add support to accumulate removed link statistics
  wifi: cfg80211: reset sinfo->filled for MLO station statistics
  wifi: mac80211: extend support to fill link level sinfo structure
  wifi: mac80211: add link_sta_statistics ops to fill link station
    statistics
  wifi: mac80211: correct RX stats packet increment for multi-link

 drivers/net/wireless/intel/iwlwifi/dvm/lib.c |   2 +-
 include/net/cfg80211.h                       | 124 ++++++
 include/net/mac80211.h                       |  16 +-
 net/mac80211/cfg.c                           |  13 +
 net/mac80211/driver-ops.h                    |  19 +
 net/mac80211/ibss.c                          |   4 +-
 net/mac80211/rx.c                            |  15 +-
 net/mac80211/sta_info.c                      | 393 +++++++++++++++--
 net/mac80211/sta_info.h                      |  36 +-
 net/mac80211/trace.h                         |   7 +
 net/mac80211/util.c                          |  14 +-
 net/wireless/nl80211.c                       | 434 ++++++++++++++++++-
 net/wireless/util.c                          |  12 +
 13 files changed, 1051 insertions(+), 38 deletions(-)


base-commit: 63a9a727d373fa5b8ce509eef50dbc45e0f745b9
-- 
2.34.1

[PATCH wireless-next v8 01/10] wifi: mac80211: add support towards MLO handling of station statistics

[Sarika Sharma]

Currently, in supporting API's to fill sinfo structure from sta
structure, is mapped to fill the fields from sta->deflink. However,
for multi-link (ML) station, sinfo structure should be filled from
corresponding link_id.

Therefore, add  link_id as an additional argument in supporting API's
for filling sinfo structure correctly. Link_id is set to -1 for non-ML
station and corresponding link_id for ML stations. In supporting API's
for filling sinfo structure, check for link_id, if link_id < 0, fill
the sinfo structure from sta-> deflink, otherwise fill from
sta->link[link_id].

Current, changes are done at the deflink level i.e, pass -1 as link_id.
Actual link_id will be added in subsequent patches to support
station statistics for MLO.

Signed-off-by: Sarika Sharma <quic_sarishar@quicinc.com>
---
 drivers/net/wireless/intel/iwlwifi/dvm/lib.c |  2 +-
 include/net/mac80211.h                       |  3 +-
 net/mac80211/ibss.c                          |  4 +-
 net/mac80211/sta_info.c                      | 79 ++++++++++++++------
 net/mac80211/sta_info.h                      |  2 +-
 net/mac80211/util.c                          | 14 +++-
 6 files changed, 73 insertions(+), 31 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/lib.c b/drivers/net/wireless/intel/iwlwifi/dvm/lib.c
index 1dc974e2c511..48711dbcfa5a 100644
--- a/drivers/net/wireless/intel/iwlwifi/dvm/lib.c
+++ b/drivers/net/wireless/intel/iwlwifi/dvm/lib.c
@@ -586,7 +586,7 @@ static bool iwlagn_fill_txpower_mode(struct iwl_priv *priv,
 		return false;
 	}
 
-	ave_rssi = ieee80211_ave_rssi(ctx->vif);
+	ave_rssi = ieee80211_ave_rssi(ctx->vif, -1);
 	if (!ave_rssi) {
 		/* no rssi data, no changes to reduce tx power */
 		IWL_DEBUG_COEX(priv, "no rssi data available\n");
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 82617579d910..a305e7f9c6b2 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -7242,13 +7242,14 @@ void ieee80211_disable_rssi_reports(struct ieee80211_vif *vif);
  * ieee80211_ave_rssi - report the average RSSI for the specified interface
  *
  * @vif: the specified virtual interface
+ * @link_id: the link ID for MLO, or -1 for non-MLO
  *
  * Note: This function assumes that the given vif is valid.
  *
  * Return: The average RSSI value for the requested interface, or 0 if not
  * applicable.
  */
-int ieee80211_ave_rssi(struct ieee80211_vif *vif);
+int ieee80211_ave_rssi(struct ieee80211_vif *vif, int link_id);
 
 /**
  * ieee80211_report_wowlan_wakeup - report WoWLAN wakeup
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index a6e7b7ba6a01..d521308734c4 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -635,7 +635,7 @@ static int ieee80211_sta_active_ibss(struct ieee80211_sub_if_data *sdata)
 	rcu_read_lock();
 
 	list_for_each_entry_rcu(sta, &local->sta_list, list) {
-		unsigned long last_active = ieee80211_sta_last_active(sta);
+		unsigned long last_active = ieee80211_sta_last_active(sta, -1);
 
 		if (sta->sdata == sdata &&
 		    time_is_after_jiffies(last_active +
@@ -1228,7 +1228,7 @@ static void ieee80211_ibss_sta_expire(struct ieee80211_sub_if_data *sdata)
 	lockdep_assert_wiphy(local->hw.wiphy);
 
 	list_for_each_entry_safe(sta, tmp, &local->sta_list, list) {
-		unsigned long last_active = ieee80211_sta_last_active(sta);
+		unsigned long last_active = ieee80211_sta_last_active(sta, -1);
 
 		if (sdata != sta->sdata)
 			continue;
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 84b18be1f0b1..5cc28eb1005b 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1650,7 +1650,7 @@ void ieee80211_sta_expire(struct ieee80211_sub_if_data *sdata,
 	lockdep_assert_wiphy(local->hw.wiphy);
 
 	list_for_each_entry_safe(sta, tmp, &local->sta_list, list) {
-		unsigned long last_active = ieee80211_sta_last_active(sta);
+		unsigned long last_active = ieee80211_sta_last_active(sta, -1);
 
 		if (sdata != sta->sdata)
 			continue;
@@ -2419,18 +2419,27 @@ void ieee80211_sta_update_pending_airtime(struct ieee80211_local *local,
 }
 
 static struct ieee80211_sta_rx_stats *
-sta_get_last_rx_stats(struct sta_info *sta)
+sta_get_last_rx_stats(struct sta_info *sta, int link_id)
 {
-	struct ieee80211_sta_rx_stats *stats = &sta->deflink.rx_stats;
+	struct ieee80211_sta_rx_stats *stats;
+	struct link_sta_info *link_sta_info;
 	int cpu;
 
-	if (!sta->deflink.pcpu_rx_stats)
+	if (link_id < 0)
+		link_sta_info = &sta->deflink;
+	else
+		link_sta_info = wiphy_dereference(sta->local->hw.wiphy,
+						  sta->link[link_id]);
+
+	stats = &link_sta_info->rx_stats;
+
+	if (!link_sta_info->pcpu_rx_stats)
 		return stats;
 
 	for_each_possible_cpu(cpu) {
 		struct ieee80211_sta_rx_stats *cpustats;
 
-		cpustats = per_cpu_ptr(sta->deflink.pcpu_rx_stats, cpu);
+		cpustats = per_cpu_ptr(link_sta_info->pcpu_rx_stats, cpu);
 
 		if (time_after(cpustats->last_rx, stats->last_rx))
 			stats = cpustats;
@@ -2498,9 +2507,10 @@ static void sta_stats_decode_rate(struct ieee80211_local *local, u32 rate,
 	}
 }
 
-static int sta_set_rate_info_rx(struct sta_info *sta, struct rate_info *rinfo)
+static int sta_set_rate_info_rx(struct sta_info *sta, struct rate_info *rinfo,
+				int link_id)
 {
-	u32 rate = READ_ONCE(sta_get_last_rx_stats(sta)->last_rate);
+	u32 rate = READ_ONCE(sta_get_last_rx_stats(sta, link_id)->last_rate);
 
 	if (rate == STA_STATS_RATE_INVALID)
 		return -EINVAL;
@@ -2525,20 +2535,28 @@ static inline u64 sta_get_tidstats_msdu(struct ieee80211_sta_rx_stats *rxstats,
 
 static void sta_set_tidstats(struct sta_info *sta,
 			     struct cfg80211_tid_stats *tidstats,
-			     int tid)
+			     int tid, int link_id)
 {
 	struct ieee80211_local *local = sta->local;
+	struct link_sta_info *link_sta_info;
 	int cpu;
 
+	if (link_id < 0)
+		link_sta_info = &sta->deflink;
+	else
+		link_sta_info = wiphy_dereference(sta->local->hw.wiphy,
+						  sta->link[link_id]);
+
 	if (!(tidstats->filled & BIT(NL80211_TID_STATS_RX_MSDU))) {
-		tidstats->rx_msdu += sta_get_tidstats_msdu(&sta->deflink.rx_stats,
-							   tid);
+		tidstats->rx_msdu +=
+			sta_get_tidstats_msdu(&link_sta_info->rx_stats,
+					      tid);
 
 		if (sta->deflink.pcpu_rx_stats) {
 			for_each_possible_cpu(cpu) {
 				struct ieee80211_sta_rx_stats *cpurxs;
 
-				cpurxs = per_cpu_ptr(sta->deflink.pcpu_rx_stats,
+				cpurxs = per_cpu_ptr(link_sta_info->pcpu_rx_stats,
 						     cpu);
 				tidstats->rx_msdu +=
 					sta_get_tidstats_msdu(cpurxs, tid);
@@ -2550,19 +2568,21 @@ static void sta_set_tidstats(struct sta_info *sta,
 
 	if (!(tidstats->filled & BIT(NL80211_TID_STATS_TX_MSDU))) {
 		tidstats->filled |= BIT(NL80211_TID_STATS_TX_MSDU);
-		tidstats->tx_msdu = sta->deflink.tx_stats.msdu[tid];
+		tidstats->tx_msdu = link_sta_info->tx_stats.msdu[tid];
 	}
 
 	if (!(tidstats->filled & BIT(NL80211_TID_STATS_TX_MSDU_RETRIES)) &&
 	    ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS)) {
 		tidstats->filled |= BIT(NL80211_TID_STATS_TX_MSDU_RETRIES);
-		tidstats->tx_msdu_retries = sta->deflink.status_stats.msdu_retries[tid];
+		tidstats->tx_msdu_retries =
+			link_sta_info->status_stats.msdu_retries[tid];
 	}
 
 	if (!(tidstats->filled & BIT(NL80211_TID_STATS_TX_MSDU_FAILED)) &&
 	    ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS)) {
 		tidstats->filled |= BIT(NL80211_TID_STATS_TX_MSDU_FAILED);
-		tidstats->tx_msdu_failed = sta->deflink.status_stats.msdu_failed[tid];
+		tidstats->tx_msdu_failed =
+			link_sta_info->status_stats.msdu_failed[tid];
 	}
 
 	if (tid < IEEE80211_NUM_TIDS) {
@@ -2633,7 +2653,7 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
 	int i, ac, cpu;
 	struct ieee80211_sta_rx_stats *last_rxstats;
 
-	last_rxstats = sta_get_last_rx_stats(sta);
+	last_rxstats = sta_get_last_rx_stats(sta, -1);
 
 	sinfo->generation = sdata->local->sta_generation;
 
@@ -2661,7 +2681,7 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
 	sinfo->connected_time = ktime_get_seconds() - sta->last_connected;
 	sinfo->assoc_at = sta->assoc_at;
 	sinfo->inactive_time =
-		jiffies_to_msecs(jiffies - ieee80211_sta_last_active(sta));
+		jiffies_to_msecs(jiffies - ieee80211_sta_last_active(sta, -1));
 
 	if (!(sinfo->filled & (BIT_ULL(NL80211_STA_INFO_TX_BYTES64) |
 			       BIT_ULL(NL80211_STA_INFO_TX_BYTES)))) {
@@ -2750,7 +2770,8 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
 	    !(sdata->vif.driver_flags & IEEE80211_VIF_BEACON_FILTER)) {
 		sinfo->filled |= BIT_ULL(NL80211_STA_INFO_BEACON_RX) |
 				 BIT_ULL(NL80211_STA_INFO_BEACON_SIGNAL_AVG);
-		sinfo->rx_beacon_signal_avg = ieee80211_ave_rssi(&sdata->vif);
+		sinfo->rx_beacon_signal_avg =
+			ieee80211_ave_rssi(&sdata->vif, -1);
 	}
 
 	if (ieee80211_hw_check(&sta->local->hw, SIGNAL_DBM) ||
@@ -2799,13 +2820,13 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
 
 	if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_RX_BITRATE)) &&
 	    !sta->sta.valid_links) {
-		if (sta_set_rate_info_rx(sta, &sinfo->rxrate) == 0)
+		if (sta_set_rate_info_rx(sta, &sinfo->rxrate, -1) == 0)
 			sinfo->filled |= BIT_ULL(NL80211_STA_INFO_RX_BITRATE);
 	}
 
 	if (tidstats && !cfg80211_sinfo_alloc_tid_stats(sinfo, GFP_KERNEL)) {
 		for (i = 0; i < IEEE80211_NUM_TIDS + 1; i++)
-			sta_set_tidstats(sta, &sinfo->pertid[i], i);
+			sta_set_tidstats(sta, &sinfo->pertid[i], i, -1);
 	}
 
 #ifdef CONFIG_MAC80211_MESH
@@ -2888,14 +2909,24 @@ u32 sta_get_expected_throughput(struct sta_info *sta)
 	return thr;
 }
 
-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;
+	struct link_sta_info *link_sta_info;
+
+	stats = sta_get_last_rx_stats(sta, link_id);
 
-	if (!sta->deflink.status_stats.last_ack ||
-	    time_after(stats->last_rx, sta->deflink.status_stats.last_ack))
+	if (link_id < 0)
+		link_sta_info = &sta->deflink;
+	else
+		link_sta_info = wiphy_dereference(sta->local->hw.wiphy,
+						  sta->link[link_id]);
+
+	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;
 }
 
 int ieee80211_sta_allocate_link(struct sta_info *sta, unsigned int link_id)
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 7a95d8d34fca..e5b91e60405b 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -936,7 +936,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 27d414efa3fd..85297faf659c 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -3265,14 +3265,24 @@ 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;
 
 	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 = wiphy_dereference(sdata->local->hw.wiphy,
+					      sdata->link[link_id]);
+
+	if (WARN_ON_ONCE(!link_data))
+		return -99;
+
+	return -ewma_beacon_signal_read(&link_data->u.mgd.ave_beacon_signal);
 }
 EXPORT_SYMBOL_GPL(ieee80211_ave_rssi);
 
-- 
2.34.1

[PATCH wireless-next v8 02/10] wifi: cfg80211: add link_station_info structure to support MLO statistics

[Sarika Sharma]

Current implementation of NL80211_GET_STATION does not work for
multi-link operation(MLO) since in case of MLO only deflink (or one
of the links) is considered and not all links.

Therefore to support for MLO, add link_station_info structure
to account link level statistics for station.

Additionally, add valid_links in station_info structure to indicate
bitmap of valid links for MLO. This will be helpful to check the link
related statistics during MLO.

Signed-off-by: Sarika Sharma <quic_sarishar@quicinc.com>
---
 include/net/cfg80211.h | 104 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index d1848dc8ec99..48096a23deb2 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2017,6 +2017,102 @@ struct cfg80211_tid_stats {
 
 #define IEEE80211_MAX_CHAINS	4
 
+/**
+ * struct link_station_info - link station information
+ *
+ * Link station information filled by driver for get_station() and
+ *	dump_station().
+ * @link_id: Link ID uniquely identifying the link STA. This is -1 for non-ML
+ * @filled: bit flag of flags using the bits of &enum nl80211_sta_info to
+ *	indicate the relevant values in this struct for them
+ * @connected_time: time(in secs) since a link of station is last connected
+ * @inactive_time: time since last activity for link station(tx/rx)
+ *	in milliseconds
+ * @assoc_at: bootime (ns) of the last association of link of station
+ * @rx_bytes: bytes (size of MPDUs) received from this link of station
+ * @tx_bytes: bytes (size of MPDUs) transmitted to this link of station
+ * @signal: The signal strength, type depends on the wiphy's signal_type.
+ *	For CFG80211_SIGNAL_TYPE_MBM, value is expressed in _dBm_.
+ * @signal_avg: Average signal strength, type depends on the wiphy's
+ *	signal_type. For CFG80211_SIGNAL_TYPE_MBM, value is expressed in _dBm_
+ * @chains: bitmask for filled values in @chain_signal, @chain_signal_avg
+ * @chain_signal: per-chain signal strength of last received packet in dBm
+ * @chain_signal_avg: per-chain signal strength average in dBm
+ * @txrate: current unicast bitrate from this link of station
+ * @rxrate: current unicast bitrate to this link of station
+ * @rx_packets: packets (MSDUs & MMPDUs) received from this link of station
+ * @tx_packets: packets (MSDUs & MMPDUs) transmitted to this link of station
+ * @tx_retries: cumulative retry counts (MPDUs) for this link of station
+ * @tx_failed: number of failed transmissions (MPDUs) (retries exceeded, no ACK)
+ * @rx_dropped_misc:  Dropped for un-specified reason.
+ * @bss_param: current BSS parameters
+ * @beacon_loss_count: Number of times beacon loss event has triggered.
+ * @expected_throughput: expected throughput in kbps (including 802.11 headers)
+ *	towards this station.
+ * @rx_beacon: number of beacons received from this peer
+ * @rx_beacon_signal_avg: signal strength average (in dBm) for beacons received
+ *	from this peer
+ * @rx_duration: aggregate PPDU duration(usecs) for all the frames from a peer
+ * @tx_duration: aggregate PPDU duration(usecs) for all the frames to a peer
+ * @airtime_weight: current airtime scheduling weight
+ * @pertid: per-TID statistics, see &struct cfg80211_tid_stats, using the last
+ *	(IEEE80211_NUM_TIDS) index for MSDUs not encapsulated in QoS-MPDUs.
+ *	Note that this doesn't use the @filled bit, but is used if non-NULL.
+ * @ack_signal: signal strength (in dBm) of the last ACK frame.
+ * @avg_ack_signal: average rssi value of ack packet for the no of msdu's has
+ *	been sent.
+ * @rx_mpdu_count: number of MPDUs received from this station
+ * @fcs_err_count: number of packets (MPDUs) received from this station with
+ *	an FCS error. This counter should be incremented only when TA of the
+ *	received packet with an FCS error matches the peer MAC address.
+ * @addr: For MLO STA connection, filled with address of the link of station.
+ **/
+
+struct link_station_info {
+	int link_id;
+	u64 filled;
+	u32 connected_time;
+	u32 inactive_time;
+	u64 assoc_at;
+	u64 rx_bytes;
+	u64 tx_bytes;
+	s8 signal;
+	s8 signal_avg;
+
+	u8 chains;
+	s8 chain_signal[IEEE80211_MAX_CHAINS];
+	s8 chain_signal_avg[IEEE80211_MAX_CHAINS];
+
+	struct rate_info txrate;
+	struct rate_info rxrate;
+	u32 rx_packets;
+	u32 tx_packets;
+	u32 tx_retries;
+	u32 tx_failed;
+	u32 rx_dropped_misc;
+	struct sta_bss_parameters bss_param;
+
+	u32 beacon_loss_count;
+
+	u32 expected_throughput;
+
+	u64 tx_duration;
+	u64 rx_duration;
+	u64 rx_beacon;
+	u8 rx_beacon_signal_avg;
+
+	u16 airtime_weight;
+
+	s8 ack_signal;
+	s8 avg_ack_signal;
+	struct cfg80211_tid_stats *pertid;
+
+	u32 rx_mpdu_count;
+	u32 fcs_err_count;
+
+	u8 addr[ETH_ALEN] __aligned(2);
+};
+
 /**
  * struct station_info - station information
  *
@@ -2101,6 +2197,11 @@ struct cfg80211_tid_stats {
  *	dump_station() callbacks. User space needs this information to determine
  *	the accepted and rejected affiliated links of the connected station.
  * @assoc_resp_ies_len: Length of @assoc_resp_ies buffer in octets.
+ * @valid_links: bitmap of valid links, or 0 for non-MLO. Drivers fill this
+ *	information in cfg80211_new_sta(), cfg80211_del_sta_sinfo(),
+ *	get_station() and dump_station() callbacks.
+ * @links: reference to Link sta entries for MLO STA, all link specific
+ *	information is accessed through links[link_id].
  */
 struct station_info {
 	u64 filled;
@@ -2165,6 +2266,9 @@ struct station_info {
 	u8 mld_addr[ETH_ALEN] __aligned(2);
 	const u8 *assoc_resp_ies;
 	size_t assoc_resp_ies_len;
+
+	u16 valid_links;
+	struct link_station_info *links[IEEE80211_MLD_MAX_NUM_LINKS];
 };
 
 /**
-- 
2.34.1

[PATCH wireless-next v8 03/10] wifi: cfg80211: extend to embed link level statistics in NL message

[Sarika Sharma]

Currently, statistics is supported at deflink( or one of the links)
level for station. This has problems when applied to multi-link(ML)
connections.

Hence, add changes to support link level statistics to embed NL message
with link related information if valid links are present.

This will be helpful to check the link related statistics during MLO.

The statistics will be embedded into NL message as below:
For non-ML:
cmd->
    NL80211_ATTR_IFINDEX
    NL80211_ATTR_MAC
    NL80211_ATTR_GENERATION
    ....etc
    NL80211_ATTR_STA_INFO | nested
        NL80211_STA_INFO_CONNECTED_TIME,
        NL80211_STA_INFO_STA_FLAGS,
        NL80211_STA_INFO_RX_BYTES,
        NL80211_STA_INFO_TX_BYTES,
        .........etc

For MLO:
cmd ->
    NL80211_ATTR_IFINDEX
    NL80211_ATTR_MAC
    NL80211_ATTR_GENERATION
    .......etc
    NL80211_ATTR_STA_INFO | nested
        NL80211_STA_INFO_CONNECTED_TIME,
        NL80211_STA_INFO_STA_FLAGS,
        ........etc
    NL80211_ATTR_MLO_LINK_ID,
    NL80211_ATTR_MLD_ADDR,
    NL80211_ATTR_MLO_LINKS | nested
        link_id-1 | nested
            NL80211_ATTR_MLO_LINK_ID,
            NL80211_ATTR_MAC,
            NL80211_ATTR_STA_INFO | nested
                NL80211_STA_INFO_RX_BYTES,
                NL80211_STA_INFO_TX_BYTES,
               NL80211_STA_INFO_CONNECTED_TIME,
               ..........etc.
        link_id-2 | nested
            NL80211_ATTR_MLO_LINK_ID,
            NL80211_ATTR_MAC,
            NL80211_ATTR_STA_INFO | nested
                NL80211_STA_INFO_RX_BYTES,
                NL80211_STA_INFO_TX_BYTES,
               NL80211_STA_INFO_CONNECTED_TIME,
                .........etc

Signed-off-by: Sarika Sharma <quic_sarishar@quicinc.com>
---
 net/wireless/nl80211.c | 215 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 214 insertions(+), 1 deletion(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index fd5f79266471..7cf1db9b012d 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -6728,6 +6728,185 @@ static bool nl80211_put_signal(struct sk_buff *msg, u8 mask, s8 *signal,
 	return true;
 }
 
+static int nl80211_fill_link_station(struct sk_buff *msg,
+				     struct cfg80211_registered_device *rdev,
+				     struct link_station_info *link_sinfo)
+{
+	struct nlattr *bss_param, *link_sinfoattr;
+
+#define PUT_LINK_SINFO(attr, memb, type) do {				\
+	BUILD_BUG_ON(sizeof(type) == sizeof(u64));			\
+	if (link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_ ## attr) &&	\
+	    nla_put_ ## type(msg, NL80211_STA_INFO_ ## attr,		\
+			     link_sinfo->memb))				\
+		goto nla_put_failure;					\
+	} while (0)
+#define PUT_LINK_SINFO_U64(attr, memb) do {				\
+	if (link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_ ## attr) &&	\
+	    nla_put_u64_64bit(msg, NL80211_STA_INFO_ ## attr,		\
+			      link_sinfo->memb, NL80211_STA_INFO_PAD))	\
+		goto nla_put_failure;					\
+	} while (0)
+
+	link_sinfoattr = nla_nest_start_noflag(msg, NL80211_ATTR_STA_INFO);
+	if (!link_sinfoattr)
+		goto nla_put_failure;
+
+	PUT_LINK_SINFO(INACTIVE_TIME, inactive_time, u32);
+
+	if (link_sinfo->filled & (BIT_ULL(NL80211_STA_INFO_RX_BYTES) |
+			     BIT_ULL(NL80211_STA_INFO_RX_BYTES64)) &&
+	    nla_put_u32(msg, NL80211_STA_INFO_RX_BYTES,
+			(u32)link_sinfo->rx_bytes))
+		goto nla_put_failure;
+
+	if (link_sinfo->filled & (BIT_ULL(NL80211_STA_INFO_TX_BYTES) |
+			     BIT_ULL(NL80211_STA_INFO_TX_BYTES64)) &&
+	    nla_put_u32(msg, NL80211_STA_INFO_TX_BYTES,
+			(u32)link_sinfo->tx_bytes))
+		goto nla_put_failure;
+
+	PUT_LINK_SINFO_U64(RX_BYTES64, rx_bytes);
+	PUT_LINK_SINFO_U64(TX_BYTES64, tx_bytes);
+	PUT_LINK_SINFO_U64(RX_DURATION, rx_duration);
+	PUT_LINK_SINFO_U64(TX_DURATION, tx_duration);
+
+	if (wiphy_ext_feature_isset(&rdev->wiphy,
+				    NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
+		PUT_LINK_SINFO(AIRTIME_WEIGHT, airtime_weight, u16);
+
+	switch (rdev->wiphy.signal_type) {
+	case CFG80211_SIGNAL_TYPE_MBM:
+		PUT_LINK_SINFO(SIGNAL, signal, u8);
+		PUT_LINK_SINFO(SIGNAL_AVG, signal_avg, u8);
+		break;
+	default:
+		break;
+	}
+	if (link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_CHAIN_SIGNAL)) {
+		if (!nl80211_put_signal(msg, link_sinfo->chains,
+					link_sinfo->chain_signal,
+					NL80211_STA_INFO_CHAIN_SIGNAL))
+			goto nla_put_failure;
+	}
+	if (link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_CHAIN_SIGNAL_AVG)) {
+		if (!nl80211_put_signal(msg, link_sinfo->chains,
+					link_sinfo->chain_signal_avg,
+					NL80211_STA_INFO_CHAIN_SIGNAL_AVG))
+			goto nla_put_failure;
+	}
+	if (link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_BITRATE)) {
+		if (!nl80211_put_sta_rate(msg, &link_sinfo->txrate,
+					  NL80211_STA_INFO_TX_BITRATE))
+			goto nla_put_failure;
+	}
+	if (link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_RX_BITRATE)) {
+		if (!nl80211_put_sta_rate(msg, &link_sinfo->rxrate,
+					  NL80211_STA_INFO_RX_BITRATE))
+			goto nla_put_failure;
+	}
+
+	PUT_LINK_SINFO(RX_PACKETS, rx_packets, u32);
+	PUT_LINK_SINFO(TX_PACKETS, tx_packets, u32);
+	PUT_LINK_SINFO(TX_RETRIES, tx_retries, u32);
+	PUT_LINK_SINFO(TX_FAILED, tx_failed, u32);
+	PUT_LINK_SINFO(EXPECTED_THROUGHPUT, expected_throughput, u32);
+	PUT_LINK_SINFO(BEACON_LOSS, beacon_loss_count, u32);
+
+	if (link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_BSS_PARAM)) {
+		bss_param = nla_nest_start_noflag(msg,
+						  NL80211_STA_INFO_BSS_PARAM);
+		if (!bss_param)
+			goto nla_put_failure;
+
+		if (((link_sinfo->bss_param.flags &
+		      BSS_PARAM_FLAGS_CTS_PROT) &&
+		     nla_put_flag(msg, NL80211_STA_BSS_PARAM_CTS_PROT)) ||
+		    ((link_sinfo->bss_param.flags &
+		      BSS_PARAM_FLAGS_SHORT_PREAMBLE) &&
+		     nla_put_flag(msg,
+				  NL80211_STA_BSS_PARAM_SHORT_PREAMBLE)) ||
+		    ((link_sinfo->bss_param.flags &
+		      BSS_PARAM_FLAGS_SHORT_SLOT_TIME) &&
+		     nla_put_flag(msg,
+				  NL80211_STA_BSS_PARAM_SHORT_SLOT_TIME)) ||
+		    nla_put_u8(msg, NL80211_STA_BSS_PARAM_DTIM_PERIOD,
+			       link_sinfo->bss_param.dtim_period) ||
+		    nla_put_u16(msg, NL80211_STA_BSS_PARAM_BEACON_INTERVAL,
+				link_sinfo->bss_param.beacon_interval))
+			goto nla_put_failure;
+
+		nla_nest_end(msg, bss_param);
+	}
+
+	PUT_LINK_SINFO_U64(RX_DROP_MISC, rx_dropped_misc);
+	PUT_LINK_SINFO_U64(BEACON_RX, rx_beacon);
+	PUT_LINK_SINFO(BEACON_SIGNAL_AVG, rx_beacon_signal_avg, u8);
+	PUT_LINK_SINFO(RX_MPDUS, rx_mpdu_count, u32);
+	PUT_LINK_SINFO(FCS_ERROR_COUNT, fcs_err_count, u32);
+	if (wiphy_ext_feature_isset(&rdev->wiphy,
+				    NL80211_EXT_FEATURE_ACK_SIGNAL_SUPPORT)) {
+		PUT_LINK_SINFO(ACK_SIGNAL, ack_signal, u8);
+		PUT_LINK_SINFO(ACK_SIGNAL_AVG, avg_ack_signal, s8);
+	}
+
+#undef PUT_LINK_SINFO
+#undef PUT_LINK_SINFO_U64
+
+	if (link_sinfo->pertid) {
+		struct nlattr *tidsattr;
+		int tid;
+
+		tidsattr = nla_nest_start_noflag(msg,
+						 NL80211_STA_INFO_TID_STATS);
+		if (!tidsattr)
+			goto nla_put_failure;
+
+		for (tid = 0; tid < IEEE80211_NUM_TIDS + 1; tid++) {
+			struct cfg80211_tid_stats *tidstats;
+			struct nlattr *tidattr;
+
+			tidstats = &link_sinfo->pertid[tid];
+
+			if (!tidstats->filled)
+				continue;
+
+			tidattr = nla_nest_start_noflag(msg, tid + 1);
+			if (!tidattr)
+				goto nla_put_failure;
+
+#define PUT_TIDVAL_U64(attr, memb) do {					\
+	if (tidstats->filled & BIT(NL80211_TID_STATS_ ## attr) &&	\
+	    nla_put_u64_64bit(msg, NL80211_TID_STATS_ ## attr,		\
+			      tidstats->memb, NL80211_TID_STATS_PAD))	\
+		goto nla_put_failure;					\
+	} while (0)
+
+			PUT_TIDVAL_U64(RX_MSDU, rx_msdu);
+			PUT_TIDVAL_U64(TX_MSDU, tx_msdu);
+			PUT_TIDVAL_U64(TX_MSDU_RETRIES, tx_msdu_retries);
+			PUT_TIDVAL_U64(TX_MSDU_FAILED, tx_msdu_failed);
+
+#undef PUT_TIDVAL_U64
+			if ((tidstats->filled &
+			     BIT(NL80211_TID_STATS_TXQ_STATS)) &&
+			    !nl80211_put_txq_stats(msg, &tidstats->txq_stats,
+						   NL80211_TID_STATS_TXQ_STATS))
+				goto nla_put_failure;
+
+			nla_nest_end(msg, tidattr);
+		}
+
+		nla_nest_end(msg, tidsattr);
+	}
+
+	nla_nest_end(msg, link_sinfoattr);
+	return 0;
+
+nla_put_failure:
+	return -EMSGSIZE;
+}
+
 static int nl80211_send_station(struct sk_buff *msg, u32 cmd, u32 portid,
 				u32 seq, int flags,
 				struct cfg80211_registered_device *rdev,
@@ -6736,6 +6915,9 @@ static int nl80211_send_station(struct sk_buff *msg, u32 cmd, u32 portid,
 {
 	void *hdr;
 	struct nlattr *sinfoattr, *bss_param;
+	struct link_station_info *link_sinfo;
+	struct nlattr *links, *link;
+	int link_id;
 
 	hdr = nl80211hdr_put(msg, portid, seq, flags, cmd);
 	if (!hdr) {
@@ -6880,7 +7062,8 @@ static int nl80211_send_station(struct sk_buff *msg, u32 cmd, u32 portid,
 #undef PUT_SINFO
 #undef PUT_SINFO_U64
 
-	if (sinfo->pertid) {
+	if ((sinfo->filled & BIT_ULL(NL80211_STA_INFO_TID_STATS)) &&
+	    sinfo->pertid) {
 		struct nlattr *tidsattr;
 		int tid;
 
@@ -6950,6 +7133,36 @@ static int nl80211_send_station(struct sk_buff *msg, u32 cmd, u32 portid,
 			goto nla_put_failure;
 	}
 
+	if (sinfo->valid_links) {
+		links = nla_nest_start(msg, NL80211_ATTR_MLO_LINKS);
+		if (!links)
+			goto nla_put_failure;
+
+		for_each_valid_link(sinfo, link_id) {
+			link_sinfo = sinfo->links[link_id];
+			link = nla_nest_start(msg, link_id + 1);
+			if (!link)
+				goto nla_put_failure;
+
+			if (nla_put_u8(msg, NL80211_ATTR_MLO_LINK_ID,
+				       link_id))
+				goto nla_put_failure;
+
+			if (link_sinfo &&
+			    !is_zero_ether_addr(link_sinfo->addr) &&
+			    nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN,
+				    link_sinfo->addr))
+				goto nla_put_failure;
+
+			if (link_sinfo && nl80211_fill_link_station(msg, rdev,
+								    link_sinfo))
+				goto nla_put_failure;
+
+			nla_nest_end(msg, link);
+		}
+		nla_nest_end(msg, links);
+	}
+
 	cfg80211_sinfo_release_content(sinfo);
 	genlmsg_end(msg, hdr);
 	return 0;
-- 
2.34.1

[PATCH wireless-next v8 04/10] wifi: cfg80211: add statistics for providing overview for MLO station

[Sarika Sharma]

Currently statistics are handled at link level for multi-link
operation(MLO). There is no provision to check accumulated statistics
for a multi-link(ML) station. Other statistics, such as signal, rates,
are also managed at the link level only.

Statistics such as packets, bytes, signal, rates, etc are useful to
provide overall overview for the ML stations.

Statistics such as packets, bytes are accumulated statistics at MLO level.
However, MLO statistics for rates and signal can not be accumulated since
it won't make much sense. Hence, handle other statistics such as signal,
rates, etc bit differently at MLO level.

The signal could be the best of all links-
e.g. if Link 1 has a signal strength of -70 dBm and Link 2 has -65 dBm,
the signal for MLO will be -65 dBm.

The rate could be determined based on the most recently updated link-
e.g. if link 1 has a rate of 300 Mbps and link 2 has a rate of 450 Mbps,
the MLO rate can be calculated based on the inactivity of each link.
If the inactive time for link 1 is 20 seconds and for link 2 is 10 seconds,
the MLO rate will be the most recently updated rate, which is link 2's
rate of 450 Mbps.

The inactive time, dtim_period and beacon_interval can be taken as the
least value of field from link level.

Similarly, other MLO level applicable fields are handled and the fields
which don't make much sense at MLO level, a subsequent change will handle
to embed NL message.

Hence, add accumulated and other statistics for MLO station if valid links
are present to represent comprehensive overview for the ML stations.

Signed-off-by: Sarika Sharma <quic_sarishar@quicinc.com>
---
 net/wireless/nl80211.c | 185 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 185 insertions(+)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 7cf1db9b012d..9095a8304d1d 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -7173,6 +7173,185 @@ static int nl80211_send_station(struct sk_buff *msg, u32 cmd, u32 portid,
 	return -EMSGSIZE;
 }
 
+static void cfg80211_sta_set_mld_rate_info(struct rate_info *sinfo_rate,
+					   struct rate_info *link_sinfo_rate)
+{
+	sinfo_rate->flags = link_sinfo_rate->flags;
+	sinfo_rate->legacy = link_sinfo_rate->legacy;
+	sinfo_rate->mcs = link_sinfo_rate->mcs;
+	sinfo_rate->nss = link_sinfo_rate->nss;
+	sinfo_rate->bw = link_sinfo_rate->bw;
+	sinfo_rate->he_gi = link_sinfo_rate->he_gi;
+	sinfo_rate->he_dcm = link_sinfo_rate->he_dcm;
+	sinfo_rate->he_ru_alloc = link_sinfo_rate->he_ru_alloc;
+	sinfo_rate->n_bonded_ch = link_sinfo_rate->n_bonded_ch;
+	sinfo_rate->eht_gi = link_sinfo_rate->eht_gi;
+	sinfo_rate->eht_ru_alloc = link_sinfo_rate->eht_ru_alloc;
+}
+
+static void cfg80211_sta_set_mld_sinfo(struct station_info *sinfo)
+{
+	struct link_station_info *link_sinfo;
+	int link_id, init = 0;
+	u32 link_inactive_time;
+
+	sinfo->signal = -99;
+
+	for_each_valid_link(sinfo, link_id) {
+		link_sinfo = sinfo->links[link_id];
+		if (!link_sinfo)
+			continue;
+
+		if ((link_sinfo->filled &
+		     BIT_ULL(NL80211_STA_INFO_TX_PACKETS))) {
+			sinfo->tx_packets += link_sinfo->tx_packets;
+			sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_PACKETS);
+		}
+
+		if ((link_sinfo->filled &
+		     BIT_ULL(NL80211_STA_INFO_RX_PACKETS))) {
+			sinfo->rx_packets += link_sinfo->rx_packets;
+			sinfo->filled |= BIT_ULL(NL80211_STA_INFO_RX_PACKETS);
+		}
+
+		if (link_sinfo->filled &
+		    (BIT_ULL(NL80211_STA_INFO_TX_BYTES) |
+		     BIT_ULL(NL80211_STA_INFO_TX_BYTES64))) {
+			sinfo->tx_bytes += link_sinfo->tx_bytes;
+			sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_BYTES);
+		}
+
+		if (link_sinfo->filled &
+		    (BIT_ULL(NL80211_STA_INFO_RX_BYTES) |
+		     BIT_ULL(NL80211_STA_INFO_TX_BYTES64))) {
+			sinfo->rx_bytes += link_sinfo->rx_bytes;
+			sinfo->filled |= BIT_ULL(NL80211_STA_INFO_RX_BYTES);
+		}
+
+		if (link_sinfo->filled &
+		    BIT_ULL(NL80211_STA_INFO_TX_RETRIES)) {
+			sinfo->tx_retries += link_sinfo->tx_retries;
+			sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_RETRIES);
+		}
+
+		if (link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_FAILED)) {
+			sinfo->tx_failed += link_sinfo->tx_failed;
+			sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_FAILED);
+		}
+
+		if (link_sinfo->filled &
+		    BIT_ULL(NL80211_STA_INFO_RX_DROP_MISC)) {
+			sinfo->rx_dropped_misc += link_sinfo->rx_dropped_misc;
+			sinfo->filled |=
+				BIT_ULL(NL80211_STA_INFO_RX_DROP_MISC);
+		}
+
+		if (link_sinfo->filled &
+		    BIT_ULL(NL80211_STA_INFO_BEACON_LOSS)) {
+			sinfo->beacon_loss_count +=
+				link_sinfo->beacon_loss_count;
+			sinfo->filled |= BIT_ULL(NL80211_STA_INFO_BEACON_LOSS);
+		}
+
+		if (link_sinfo->filled &
+		    BIT_ULL(NL80211_STA_INFO_EXPECTED_THROUGHPUT)) {
+			sinfo->expected_throughput +=
+				link_sinfo->expected_throughput;
+			sinfo->filled |=
+				BIT_ULL(NL80211_STA_INFO_EXPECTED_THROUGHPUT);
+		}
+
+		if (link_sinfo->filled &
+		    BIT_ULL(NL80211_STA_INFO_TX_DURATION)) {
+			sinfo->tx_duration += link_sinfo->tx_duration;
+			sinfo->filled |=
+				BIT_ULL(NL80211_STA_INFO_TX_DURATION);
+		}
+
+		if (link_sinfo->filled &
+		    BIT_ULL(NL80211_STA_INFO_RX_DURATION)) {
+			sinfo->rx_duration += link_sinfo->rx_duration;
+			sinfo->filled |=
+				BIT_ULL(NL80211_STA_INFO_RX_DURATION);
+		}
+
+		if (link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_RX_MPDUS)) {
+			sinfo->rx_mpdu_count += link_sinfo->rx_mpdu_count;
+			sinfo->filled |= BIT_ULL(NL80211_STA_INFO_RX_MPDUS);
+		}
+
+		if (link_sinfo->filled &
+		    BIT_ULL(NL80211_STA_INFO_FCS_ERROR_COUNT)) {
+			sinfo->fcs_err_count += link_sinfo->fcs_err_count;
+			sinfo->filled |=
+				BIT_ULL(NL80211_STA_INFO_FCS_ERROR_COUNT);
+		}
+
+		if (link_sinfo->filled &
+		    BIT_ULL(NL80211_STA_INFO_BEACON_RX)) {
+			sinfo->rx_beacon += link_sinfo->rx_beacon;
+			sinfo->filled |= BIT_ULL(NL80211_STA_INFO_BEACON_RX);
+		}
+
+		/* Update MLO signal, signal_avg as best among links */
+		if ((link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_SIGNAL)) &&
+		    link_sinfo->signal > sinfo->signal) {
+			sinfo->signal = link_sinfo->signal;
+			sinfo->filled |= BIT_ULL(NL80211_STA_INFO_SIGNAL);
+		}
+
+		if ((link_sinfo->filled &
+			BIT_ULL(NL80211_STA_INFO_SIGNAL_AVG)) &&
+		    link_sinfo->signal_avg > sinfo->signal_avg) {
+			sinfo->signal_avg = link_sinfo->signal_avg;
+			sinfo->filled |= BIT_ULL(NL80211_STA_INFO_SIGNAL_AVG);
+		}
+
+		/* Update MLO inactive_time, bss_param based on least
+		 * value for corresponding field of link.
+		 */
+		if ((link_sinfo->filled &
+		     BIT_ULL(NL80211_STA_INFO_INACTIVE_TIME)) &&
+		    (!init ||
+		     link_inactive_time > link_sinfo->inactive_time)) {
+			link_inactive_time = link_sinfo->inactive_time;
+			sinfo->inactive_time = link_sinfo->inactive_time;
+			sinfo->filled |= NL80211_STA_INFO_INACTIVE_TIME;
+		}
+
+		if (link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_BSS_PARAM) &&
+		    (!init ||
+		     sinfo->bss_param.dtim_period >
+		      link_sinfo->bss_param.dtim_period)) {
+			sinfo->bss_param.dtim_period =
+				link_sinfo->bss_param.dtim_period;
+			sinfo->filled |= NL80211_STA_BSS_PARAM_DTIM_PERIOD;
+			sinfo->bss_param.beacon_interval =
+				link_sinfo->bss_param.beacon_interval;
+			sinfo->filled |= NL80211_STA_BSS_PARAM_BEACON_INTERVAL;
+		}
+
+		/* Update MLO rates as per last updated link rate */
+		if ((link_sinfo->filled &
+		     BIT_ULL(NL80211_STA_INFO_TX_BITRATE)) &&
+		    (!init ||
+		     link_inactive_time > link_sinfo->inactive_time)) {
+			cfg80211_sta_set_mld_rate_info(&sinfo->txrate,
+						       &link_sinfo->txrate);
+			sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_BITRATE);
+		}
+		if ((link_sinfo->filled &
+		     BIT_ULL(NL80211_STA_INFO_RX_BITRATE)) &&
+		    (!init ||
+		     link_inactive_time > link_sinfo->inactive_time)) {
+			cfg80211_sta_set_mld_rate_info(&sinfo->rxrate,
+						       &link_sinfo->rxrate);
+			sinfo->filled |= BIT_ULL(NL80211_STA_INFO_RX_BITRATE);
+		}
+		init++;
+	}
+}
+
 static int nl80211_dump_station(struct sk_buff *skb,
 				struct netlink_callback *cb)
 {
@@ -7208,6 +7387,9 @@ static int nl80211_dump_station(struct sk_buff *skb,
 		if (err)
 			goto out_err;
 
+		if (sinfo.valid_links)
+			cfg80211_sta_set_mld_sinfo(&sinfo);
+
 		if (nl80211_send_station(skb, NL80211_CMD_NEW_STATION,
 				NETLINK_CB(cb->skb).portid,
 				cb->nlh->nlmsg_seq, NLM_F_MULTI,
@@ -7256,6 +7438,9 @@ static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info)
 		return -ENOMEM;
 	}
 
+	if (sinfo.valid_links)
+		cfg80211_sta_set_mld_sinfo(&sinfo);
+
 	if (nl80211_send_station(msg, NL80211_CMD_NEW_STATION,
 				 info->snd_portid, info->snd_seq, 0,
 				 rdev, dev, mac_addr, &sinfo) < 0) {
-- 
2.34.1

[PATCH wireless-next v8 05/10] wifi: cfg80211: allocate memory for link_station info structure

[Sarika Sharma]

Currently, station_info structure is passed to fill station statistics
from mac80211/drivers. After NL message send to user space for requested
station statistics, memory for station statistics is freed in cfg80211.
Therefore, memory allocation/free for link station statistics should
also happen in cfg80211 only.

Hence, allocate the memory for link_station structure for all
possible links and free in cfg80211_sinfo_release_content().

Signed-off-by: Sarika Sharma <quic_sarishar@quicinc.com>
---
 include/net/cfg80211.h |  9 +++++++++
 net/wireless/nl80211.c | 27 ++++++++++++++++++++++++---
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 48096a23deb2..65a1a6511172 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -8579,7 +8579,16 @@ int cfg80211_sinfo_alloc_tid_stats(struct station_info *sinfo, gfp_t gfp);
  */
 static inline void cfg80211_sinfo_release_content(struct station_info *sinfo)
 {
+	int link_id;
+
 	kfree(sinfo->pertid);
+
+	for_each_valid_link(sinfo, link_id) {
+		if (sinfo->links[link_id]) {
+			kfree(sinfo->links[link_id]->pertid);
+			kfree(sinfo->links[link_id]);
+		}
+	}
 }
 
 /**
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 9095a8304d1d..cc0b89d0578c 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -7360,7 +7360,7 @@ static int nl80211_dump_station(struct sk_buff *skb,
 	struct wireless_dev *wdev;
 	u8 mac_addr[ETH_ALEN];
 	int sta_idx = cb->args[2];
-	int err;
+	int err, i;
 
 	err = nl80211_prepare_wdev_dump(cb, &rdev, &wdev, NULL);
 	if (err)
@@ -7380,6 +7380,16 @@ static int nl80211_dump_station(struct sk_buff *skb,
 
 	while (1) {
 		memset(&sinfo, 0, sizeof(sinfo));
+
+		for (i = 0; i < IEEE80211_MLD_MAX_NUM_LINKS; i++) {
+			sinfo.links[i] =
+				kzalloc(sizeof(*sinfo.links[0]), GFP_KERNEL);
+			if (!sinfo.links[i]) {
+				err = -ENOMEM;
+				goto out_err;
+			}
+		}
+
 		err = rdev_dump_station(rdev, wdev->netdev, sta_idx,
 					mac_addr, &sinfo);
 		if (err == -ENOENT)
@@ -7404,6 +7414,7 @@ static int nl80211_dump_station(struct sk_buff *skb,
 	cb->args[2] = sta_idx;
 	err = skb->len;
  out_err:
+	cfg80211_sinfo_release_content(&sinfo);
 	wiphy_unlock(&rdev->wiphy);
 
 	return err;
@@ -7416,7 +7427,7 @@ static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info)
 	struct station_info sinfo;
 	struct sk_buff *msg;
 	u8 *mac_addr = NULL;
-	int err;
+	int err, i;
 
 	memset(&sinfo, 0, sizeof(sinfo));
 
@@ -7428,9 +7439,19 @@ static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info)
 	if (!rdev->ops->get_station)
 		return -EOPNOTSUPP;
 
+	for (i = 0; i < IEEE80211_MLD_MAX_NUM_LINKS; i++) {
+		sinfo.links[i] = kzalloc(sizeof(*sinfo.links[0]), GFP_KERNEL);
+		if (!sinfo.links[i]) {
+			cfg80211_sinfo_release_content(&sinfo);
+			return -ENOMEM;
+		}
+	}
+
 	err = rdev_get_station(rdev, dev, mac_addr, &sinfo);
-	if (err)
+	if (err) {
+		cfg80211_sinfo_release_content(&sinfo);
 		return err;
+	}
 
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg) {
-- 
2.34.1

[PATCH wireless-next v8 06/10] wifi: mac80211: add support to accumulate removed link statistics

[Sarika Sharma]

Currently, if a link gets removed in between for a station then
directly accumulated data will fall down to sum of other active links.
This will bring inconsistency in station dump statistics.

For instance, let's take Tx packets
- at t=0-> link-0:2 link-1:3 Tx packets => accumulated = 5
- at t=1-> link-0:4 link-1:6 Tx packets => accumulated = 10
let say at t=2, link-0 went down => link-0:0 link-1:7 => accumulated = 7
Here, suddenly accumulated Tx packets will come down to 7 from 10.
This is showing inconsistency.

Therefore, store link-0 data when it went down and add to accumulated
Tx packet = 11.

Hence, store the removed link statistics data in sta structure and
add it in accumulated statistics for consistency.

Also, initialize accumulatable fields in sinfo structure to 0, to avoid
any additional packets account for MLO station.

Signed-off-by: Sarika Sharma <quic_sarishar@quicinc.com>
---
 net/mac80211/cfg.c      | 13 ++++++++++
 net/mac80211/sta_info.c | 54 +++++++++++++++++++++++++++++++++++++++++
 net/mac80211/sta_info.h | 34 ++++++++++++++++++++++++++
 3 files changed, 101 insertions(+)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 2cd8731d8275..a523a6a5db32 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -885,6 +885,13 @@ static int ieee80211_dump_station(struct wiphy *wiphy, struct net_device *dev,
 		ret = 0;
 		memcpy(mac, sta->sta.addr, ETH_ALEN);
 		sta_set_sinfo(sta, sinfo, true);
+
+		/* Add accumulated removed link data to sinfo data for
+		 * consistency for MLO
+		 */
+		if (sinfo->valid_links)
+			sta_set_accumulated_removed_links_sinfo(sta, sinfo);
+
 	}
 
 	return ret;
@@ -912,6 +919,12 @@ static int ieee80211_get_station(struct wiphy *wiphy, struct net_device *dev,
 	if (sta) {
 		ret = 0;
 		sta_set_sinfo(sta, sinfo, true);
+
+		/* Add accumulated removed link data to sinfo data for
+		 * consistency for MLO
+		 */
+		if (sinfo->valid_links)
+			sta_set_accumulated_removed_links_sinfo(sta, sinfo);
 	}
 
 	return ret;
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 5cc28eb1005b..709250ba37c9 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -355,6 +355,25 @@ static void sta_info_free_link(struct link_sta_info *link_sta)
 	free_percpu(link_sta->pcpu_rx_stats);
 }
 
+static void sta_accumulate_removed_link_stats(struct sta_info *sta, int link_id)
+{
+	struct link_sta_info *link_sta = wiphy_dereference(sta->local->hw.wiphy,
+							   sta->link[link_id]);
+	int ac;
+
+	for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
+		sta->rem_link_stats.tx_packets +=
+			link_sta->tx_stats.packets[ac];
+		sta->rem_link_stats.tx_bytes += link_sta->tx_stats.bytes[ac];
+	}
+
+	sta->rem_link_stats.rx_packets += link_sta->rx_stats.packets;
+	sta->rem_link_stats.rx_bytes += link_sta->rx_stats.bytes;
+	sta->rem_link_stats.tx_retries += link_sta->status_stats.retry_count;
+	sta->rem_link_stats.tx_failed += link_sta->status_stats.retry_failed;
+	sta->rem_link_stats.rx_dropped_misc += link_sta->rx_stats.dropped;
+}
+
 static void sta_remove_link(struct sta_info *sta, unsigned int link_id,
 			    bool unhash)
 {
@@ -377,6 +396,10 @@ static void sta_remove_link(struct sta_info *sta, unsigned int link_id,
 		alloc = container_of(link_sta, typeof(*alloc), info);
 
 	sta->sta.valid_links &= ~BIT(link_id);
+
+	/* store removed link info for accumulated stats consistency */
+	sta_accumulate_removed_link_stats(sta, link_id);
+
 	RCU_INIT_POINTER(sta->link[link_id], NULL);
 	RCU_INIT_POINTER(sta->sta.link[link_id], NULL);
 	if (alloc) {
@@ -2644,6 +2667,37 @@ static void sta_set_mesh_sinfo(struct sta_info *sta,
 }
 #endif
 
+void sta_set_accumulated_removed_links_sinfo(struct sta_info *sta,
+					     struct station_info *sinfo)
+{
+	/* Resetting the MLO statistics for accumulated fields, to
+	 * avoid duplication.
+	 */
+	sinfo->tx_packets = 0;
+	sinfo->rx_packets = 0;
+	sinfo->tx_bytes = 0;
+	sinfo->rx_bytes = 0;
+	sinfo->tx_retries = 0;
+	sinfo->tx_failed = 0;
+	sinfo->rx_dropped_misc = 0;
+	sinfo->beacon_loss_count = 0;
+	sinfo->expected_throughput = 0;
+	sinfo->rx_mpdu_count = 0;
+	sinfo->fcs_err_count = 0;
+	sinfo->rx_beacon = 0;
+	sinfo->rx_duration = 0;
+	sinfo->tx_duration = 0;
+
+	/* Accumulating the removed link statistics. */
+	sinfo->tx_packets += sta->rem_link_stats.tx_packets;
+	sinfo->rx_packets += sta->rem_link_stats.rx_packets;
+	sinfo->tx_bytes += sta->rem_link_stats.tx_bytes;
+	sinfo->rx_bytes += sta->rem_link_stats.rx_bytes;
+	sinfo->tx_retries += sta->rem_link_stats.tx_retries;
+	sinfo->tx_failed += sta->rem_link_stats.tx_failed;
+	sinfo->rx_dropped_misc += sta->rem_link_stats.rx_dropped_misc;
+}
+
 void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
 		   bool tidstats)
 {
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index e5b91e60405b..6851cf10a1da 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -568,6 +568,35 @@ struct link_sta_info {
 	struct ieee80211_link_sta *pub;
 };
 
+/**
+ * struct ieee80211_sta_removed_link_stats - Removed link sta data
+ *
+ * keep required accumulated removed link data for stats
+ *
+ * @rx_packets: accumulated packets (MSDUs & MMPDUs) received from
+ *	this station for removed links
+ * @tx_packets: accumulated packets (MSDUs & MMPDUs) transmitted to
+ *	this station for removed links
+ * @rx_bytes: accumulated bytes (size of MPDUs) received from this
+ *	station for removed links
+ * @tx_bytes: accumulated bytes (size of MPDUs) transmitted to this
+ *	station for removed links
+ * @tx_retries: cumulative retry counts (MPDUs) for removed links
+ * @tx_failed: accumulated number of failed transmissions (MPDUs)
+ *	(retries exceeded, no ACK) for removed links
+ * @rx_dropped_misc: accumulated dropped packets for un-specified reason
+ *	from this station for removed links
+ */
+struct ieee80211_sta_removed_link_stats {
+	u32 rx_packets;
+	u32 tx_packets;
+	u64 rx_bytes;
+	u64 tx_bytes;
+	u32 tx_retries;
+	u32 tx_failed;
+	u32 rx_dropped_misc;
+};
+
 /**
  * struct sta_info - STA information
  *
@@ -644,6 +673,7 @@ struct link_sta_info {
  *	@deflink address and remaining would be allocated and the address
  *	would be assigned to link[link_id] where link_id is the id assigned
  *	by the AP.
+ * @rem_link_stats: accumulated removed link stats
  */
 struct sta_info {
 	/* General information, mostly static */
@@ -718,6 +748,7 @@ struct sta_info {
 	struct ieee80211_sta_aggregates cur;
 	struct link_sta_info deflink;
 	struct link_sta_info __rcu *link[IEEE80211_MLD_MAX_NUM_LINKS];
+	struct ieee80211_sta_removed_link_stats rem_link_stats;
 
 	/* keep last! */
 	struct ieee80211_sta sta;
@@ -922,6 +953,9 @@ void sta_set_rate_info_tx(struct sta_info *sta,
 void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
 		   bool tidstats);
 
+void sta_set_accumulated_removed_links_sinfo(struct sta_info *sta,
+					     struct station_info *sinfo);
+
 u32 sta_get_expected_throughput(struct sta_info *sta);
 
 void ieee80211_sta_expire(struct ieee80211_sub_if_data *sdata,
-- 
2.34.1

[PATCH wireless-next v8 07/10] wifi: cfg80211: reset sinfo->filled for MLO station statistics

[Sarika Sharma]

Currently, sinfo->filled is for set in sta_set_sinfo() after filling
the corresponding fields in station_info structure for station statistics.

For non-ML stations, the fields are correctly filled from sta->deflink
and corresponding sinfo->filled bit are set, but for MLO any one of
link's data is filled and corresponding sinfo->filled bit is set.

For MLO before embed NL message, fields of sinfo structure like
bytes, packets, signal are updated with accumulated, best, least of all
links data. But some of fields like rssi, pertid don't make much sense
at MLO level.

Hence, to prevent misinterpretation, reset sinfo->filled for fields
which don't make much sense at MLO level. This will prevent filling the
unnecessary values in NL message.

Signed-off-by: Sarika Sharma <quic_sarishar@quicinc.com>
---
 net/wireless/nl80211.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index cc0b89d0578c..9a8bc08aa54b 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -7350,6 +7350,13 @@ static void cfg80211_sta_set_mld_sinfo(struct station_info *sinfo)
 		}
 		init++;
 	}
+
+	/* Resetting sinfo->filled bits to exclude fields which don't
+	 * make much sense at the MLO level.
+	 */
+	sinfo->filled &= ~(1 << NL80211_STA_INFO_CHAIN_SIGNAL);
+	sinfo->filled &= ~(1 << NL80211_STA_INFO_CHAIN_SIGNAL_AVG);
+	sinfo->filled &= ~(1 << NL80211_STA_INFO_TID_STATS);
 }
 
 static int nl80211_dump_station(struct sk_buff *skb,
-- 
2.34.1

[PATCH wireless-next v8 08/10] wifi: mac80211: extend support to fill link level sinfo structure

[Sarika Sharma]

Currently, sinfo structure is supported to fill information at
deflink( or one of the links) level for station. This has problems
when applied to fetch multi-link(ML) station information.

Hence, if valid_links are present, support filling link_station
structure for each link.

This will be helpful to check the link related statistics during MLO.

Signed-off-by: Sarika Sharma <quic_sarishar@quicinc.com>
---
 include/net/cfg80211.h  |  11 ++
 net/mac80211/sta_info.c | 262 +++++++++++++++++++++++++++++++++++++++-
 net/wireless/util.c     |  12 ++
 3 files changed, 284 insertions(+), 1 deletion(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 65a1a6511172..3dfab12dc870 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -8569,6 +8569,17 @@ void cfg80211_tx_mgmt_expired(struct wireless_dev *wdev, u64 cookie,
  */
 int cfg80211_sinfo_alloc_tid_stats(struct station_info *sinfo, gfp_t gfp);
 
+/**
+ * cfg80211_link_sinfo_alloc_tid_stats - allocate per-tid statistics.
+ *
+ * @link_sinfo: the link station information
+ * @gfp: allocation flags
+ *
+ * Return: 0 on success. Non-zero on error.
+ */
+int cfg80211_link_sinfo_alloc_tid_stats(struct link_station_info *link_sinfo,
+					gfp_t gfp);
+
 /**
  * cfg80211_sinfo_release_content - release contents of station info
  * @sinfo: the station information
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 709250ba37c9..906569651fbd 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -2698,17 +2698,257 @@ void sta_set_accumulated_removed_links_sinfo(struct sta_info *sta,
 	sinfo->rx_dropped_misc += sta->rem_link_stats.rx_dropped_misc;
 }
 
+static void sta_set_link_sinfo(struct sta_info *sta,
+			       struct link_station_info *link_sinfo,
+			       struct ieee80211_link_data *link_sdata,
+			       bool tidstats)
+{
+	struct ieee80211_sub_if_data *sdata = sta->sdata;
+	struct ieee80211_sta_rx_stats *last_rxstats;
+	struct link_sta_info *link_sta_info;
+	u32 thr = 0;
+	int i, ac, cpu, link_id;
+
+	link_id = link_sinfo->link_id;
+	last_rxstats = sta_get_last_rx_stats(sta, link_id);
+
+	link_sta_info = wiphy_dereference(sta->local->hw.wiphy,
+					  sta->link[link_id]);
+
+	/* do before driver, so beacon filtering drivers have a
+	 * chance to e.g. just add the number of filtered beacons
+	 * (or just modify the value entirely, of course)
+	 */
+	if (sdata->vif.type == NL80211_IFTYPE_STATION)
+		link_sinfo->rx_beacon = link_sdata->u.mgd.count_beacon_signal;
+
+	ether_addr_copy(link_sinfo->addr, link_sta_info->addr);
+
+	/* TODO: add drv_link_sta_statistics() ops to fill link_station
+	 * statistics of station.
+	 */
+
+	link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_INACTIVE_TIME) |
+			 BIT_ULL(NL80211_STA_INFO_BSS_PARAM) |
+			 BIT_ULL(NL80211_STA_INFO_RX_DROP_MISC);
+
+	if (sdata->vif.type == NL80211_IFTYPE_STATION) {
+		link_sinfo->beacon_loss_count =
+			link_sdata->u.mgd.beacon_loss_count;
+		link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_BEACON_LOSS);
+	}
+
+	link_sinfo->inactive_time =
+		jiffies_to_msecs(jiffies -
+		ieee80211_sta_last_active(sta, link_id));
+
+	if (!(link_sinfo->filled & (BIT_ULL(NL80211_STA_INFO_TX_BYTES64) |
+				    BIT_ULL(NL80211_STA_INFO_TX_BYTES)))) {
+		link_sinfo->tx_bytes = 0;
+		for (ac = 0; ac < IEEE80211_NUM_ACS; ac++)
+			link_sinfo->tx_bytes +=
+				link_sta_info->tx_stats.bytes[ac];
+		link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_BYTES64);
+	}
+
+	if (!(link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_PACKETS))) {
+		link_sinfo->tx_packets = 0;
+		for (ac = 0; ac < IEEE80211_NUM_ACS; ac++)
+			link_sinfo->tx_packets +=
+				link_sta_info->tx_stats.packets[ac];
+		link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_PACKETS);
+	}
+
+	if (!(link_sinfo->filled & (BIT_ULL(NL80211_STA_INFO_RX_BYTES64) |
+			       BIT_ULL(NL80211_STA_INFO_RX_BYTES)))) {
+		link_sinfo->rx_bytes +=
+			sta_get_stats_bytes(&link_sta_info->rx_stats);
+
+		if (link_sta_info->pcpu_rx_stats) {
+			for_each_possible_cpu(cpu) {
+				struct ieee80211_sta_rx_stats *cpurxs;
+
+				cpurxs = per_cpu_ptr(link_sta_info->pcpu_rx_stats,
+						     cpu);
+				link_sinfo->rx_bytes +=
+					sta_get_stats_bytes(cpurxs);
+			}
+		}
+
+		link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_RX_BYTES64);
+	}
+
+	if (!(link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_RX_PACKETS))) {
+		link_sinfo->rx_packets = link_sta_info->rx_stats.packets;
+		if (link_sta_info->pcpu_rx_stats) {
+			for_each_possible_cpu(cpu) {
+				struct ieee80211_sta_rx_stats *cpurxs;
+
+				cpurxs = per_cpu_ptr(link_sta_info->pcpu_rx_stats,
+						     cpu);
+				link_sinfo->rx_packets += cpurxs->packets;
+			}
+		}
+		link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_RX_PACKETS);
+	}
+
+	if (!(link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_RETRIES))) {
+		link_sinfo->tx_retries =
+			link_sta_info->status_stats.retry_count;
+		link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_RETRIES);
+	}
+
+	if (!(link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_FAILED))) {
+		link_sinfo->tx_failed =
+			link_sta_info->status_stats.retry_failed;
+		link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_FAILED);
+	}
+
+	if (!(link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_RX_DURATION))) {
+		for (ac = 0; ac < IEEE80211_NUM_ACS; ac++)
+			link_sinfo->rx_duration += sta->airtime[ac].rx_airtime;
+		link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_RX_DURATION);
+	}
+
+	if (!(link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_DURATION))) {
+		for (ac = 0; ac < IEEE80211_NUM_ACS; ac++)
+			link_sinfo->tx_duration += sta->airtime[ac].tx_airtime;
+		link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_DURATION);
+	}
+
+	if (!(link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_AIRTIME_WEIGHT))) {
+		link_sinfo->airtime_weight = sta->airtime_weight;
+		link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_AIRTIME_WEIGHT);
+	}
+
+	link_sinfo->rx_dropped_misc = link_sta_info->rx_stats.dropped;
+	if (link_sta_info->pcpu_rx_stats) {
+		for_each_possible_cpu(cpu) {
+			struct ieee80211_sta_rx_stats *cpurxs;
+
+			cpurxs = per_cpu_ptr(link_sta_info->pcpu_rx_stats,
+					     cpu);
+			link_sinfo->rx_dropped_misc += cpurxs->dropped;
+		}
+	}
+
+	if (sdata->vif.type == NL80211_IFTYPE_STATION &&
+	    !(sdata->vif.driver_flags & IEEE80211_VIF_BEACON_FILTER)) {
+		link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_BEACON_RX) |
+				 BIT_ULL(NL80211_STA_INFO_BEACON_SIGNAL_AVG);
+		link_sinfo->rx_beacon_signal_avg =
+			ieee80211_ave_rssi(&sdata->vif, -1);
+	}
+
+	if (ieee80211_hw_check(&sta->local->hw, SIGNAL_DBM) ||
+	    ieee80211_hw_check(&sta->local->hw, SIGNAL_UNSPEC)) {
+		if (!(link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_SIGNAL))) {
+			link_sinfo->signal = (s8)last_rxstats->last_signal;
+			link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_SIGNAL);
+		}
+
+		if (!link_sta_info->pcpu_rx_stats &&
+		    !(link_sinfo->filled &
+		       BIT_ULL(NL80211_STA_INFO_SIGNAL_AVG))) {
+			link_sinfo->signal_avg =
+				-ewma_signal_read(&link_sta_info->rx_stats_avg.signal);
+			link_sinfo->filled |=
+				BIT_ULL(NL80211_STA_INFO_SIGNAL_AVG);
+		}
+	}
+
+	/* for the average - if pcpu_rx_stats isn't set - rxstats must point to
+	 * the sta->rx_stats struct, so the check here is fine with and without
+	 * pcpu statistics
+	 */
+	if (last_rxstats->chains &&
+	    !(link_sinfo->filled & (BIT_ULL(NL80211_STA_INFO_CHAIN_SIGNAL) |
+			       BIT_ULL(NL80211_STA_INFO_CHAIN_SIGNAL_AVG)))) {
+		link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_CHAIN_SIGNAL);
+		if (!link_sta_info->pcpu_rx_stats)
+			link_sinfo->filled |=
+				BIT_ULL(NL80211_STA_INFO_CHAIN_SIGNAL_AVG);
+
+		link_sinfo->chains = last_rxstats->chains;
+
+		for (i = 0; i < ARRAY_SIZE(link_sinfo->chain_signal); i++) {
+			link_sinfo->chain_signal[i] =
+				last_rxstats->chain_signal_last[i];
+			link_sinfo->chain_signal_avg[i] =
+				-ewma_signal_read(
+					&link_sta_info->rx_stats_avg.chain_signal[i]);
+		}
+	}
+
+	if (!(link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_BITRATE)) &&
+	    ieee80211_rate_valid(&link_sta_info->tx_stats.last_rate)) {
+		sta_set_rate_info_tx(sta, &link_sta_info->tx_stats.last_rate,
+				     &link_sinfo->txrate);
+		link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_BITRATE);
+	}
+
+	if (!(link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_RX_BITRATE))) {
+		if (sta_set_rate_info_rx(sta, &link_sinfo->rxrate,
+					 link_id) == 0)
+			link_sinfo->filled |=
+				BIT_ULL(NL80211_STA_INFO_RX_BITRATE);
+	}
+
+	if (tidstats && !cfg80211_link_sinfo_alloc_tid_stats(link_sinfo,
+							     GFP_KERNEL)) {
+		for (i = 0; i < IEEE80211_NUM_TIDS + 1; i++)
+			sta_set_tidstats(sta, &link_sinfo->pertid[i], i,
+					 link_id);
+		link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TID_STATS);
+	}
+
+	link_sinfo->bss_param.flags = 0;
+	if (sdata->vif.bss_conf.use_cts_prot)
+		link_sinfo->bss_param.flags |= BSS_PARAM_FLAGS_CTS_PROT;
+	if (sdata->vif.bss_conf.use_short_preamble)
+		link_sinfo->bss_param.flags |= BSS_PARAM_FLAGS_SHORT_PREAMBLE;
+	if (sdata->vif.bss_conf.use_short_slot)
+		link_sinfo->bss_param.flags |= BSS_PARAM_FLAGS_SHORT_SLOT_TIME;
+	link_sinfo->bss_param.dtim_period = link_sdata->conf->dtim_period;
+	link_sinfo->bss_param.beacon_interval = link_sdata->conf->beacon_int;
+
+	thr = sta_get_expected_throughput(sta);
+
+	if (thr != 0) {
+		link_sinfo->filled |=
+			BIT_ULL(NL80211_STA_INFO_EXPECTED_THROUGHPUT);
+		link_sinfo->expected_throughput = thr;
+	}
+
+	if (!(link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL)) &&
+	    link_sta_info->status_stats.ack_signal_filled) {
+		link_sinfo->ack_signal =
+			link_sta_info->status_stats.last_ack_signal;
+		link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL);
+	}
+
+	if (!(link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL_AVG)) &&
+	    link_sta_info->status_stats.ack_signal_filled) {
+		link_sinfo->avg_ack_signal =
+			-(s8)ewma_avg_signal_read(
+				&link_sta_info->status_stats.avg_ack_signal);
+		link_sinfo->filled |=
+			BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL_AVG);
+	}
+}
+
 void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
 		   bool tidstats)
 {
 	struct ieee80211_sub_if_data *sdata = sta->sdata;
 	struct ieee80211_local *local = sdata->local;
 	u32 thr = 0;
-	int i, ac, cpu;
+	int i, ac, cpu, link_id;
 	struct ieee80211_sta_rx_stats *last_rxstats;
 
 	last_rxstats = sta_get_last_rx_stats(sta, -1);
 
+	sinfo->valid_links = sta->sta.valid_links;
 	sinfo->generation = sdata->local->sta_generation;
 
 	/* do before driver, so beacon filtering drivers have a
@@ -2881,6 +3121,7 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
 	if (tidstats && !cfg80211_sinfo_alloc_tid_stats(sinfo, GFP_KERNEL)) {
 		for (i = 0; i < IEEE80211_NUM_TIDS + 1; i++)
 			sta_set_tidstats(sta, &sinfo->pertid[i], i, -1);
+		sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TID_STATS);
 	}
 
 #ifdef CONFIG_MAC80211_MESH
@@ -2942,6 +3183,25 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
 		sinfo->filled |=
 			BIT_ULL(NL80211_STA_INFO_ACK_SIGNAL_AVG);
 	}
+
+	if (sinfo->valid_links) {
+		struct ieee80211_link_data *link_sdata;
+		struct link_sta_info *link_sta;
+
+		ether_addr_copy(sinfo->mld_addr, sta->addr);
+		for_each_valid_link(sinfo, link_id) {
+			link_sta = wiphy_dereference(sta->local->hw.wiphy,
+						     sta->link[link_id]);
+			if (!link_sta || !sinfo->links[link_id])
+				continue;
+
+			sinfo->links[link_id]->link_id = link_id;
+			link_sdata = wiphy_dereference(sdata->local->hw.wiphy,
+						       sdata->link[link_id]);
+			sta_set_link_sinfo(sta, sinfo->links[link_id],
+					   link_sdata, tidstats);
+		}
+	}
 }
 
 u32 sta_get_expected_throughput(struct sta_info *sta)
diff --git a/net/wireless/util.c b/net/wireless/util.c
index ed868c0f7ca8..3c8ff0b3b240 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -2626,6 +2626,18 @@ bool cfg80211_does_bw_fit_range(const struct ieee80211_freq_range *freq_range,
 	return false;
 }
 
+int cfg80211_link_sinfo_alloc_tid_stats(struct link_station_info *link_sinfo,
+					gfp_t gfp)
+{
+	link_sinfo->pertid = kcalloc(IEEE80211_NUM_TIDS + 1,
+				     sizeof(*link_sinfo->pertid), gfp);
+	if (!link_sinfo->pertid)
+		return -ENOMEM;
+
+	return 0;
+}
+EXPORT_SYMBOL(cfg80211_link_sinfo_alloc_tid_stats);
+
 int cfg80211_sinfo_alloc_tid_stats(struct station_info *sinfo, gfp_t gfp)
 {
 	sinfo->pertid = kcalloc(IEEE80211_NUM_TIDS + 1,
-- 
2.34.1

[PATCH wireless-next v8 09/10] wifi: mac80211: add link_sta_statistics ops to fill link station statistics

[Sarika Sharma]

Currently, link station statistics for MLO are filled by mac80211.
But there are some statistics that kept by mac80211 might not be
accurate, so let the driver pre-fill the link statistics. The driver
can fill the values (indicating which field is filled, by setting the
filled bitmapin in link_station structure).
Statistics that driver don't fill are filled by mac80211.

Hence, add link_sta_statistics callback to fill link station statistics for
MLO in sta_set_link_sinfo() by drivers.

Signed-off-by: Sarika Sharma <quic_sarishar@quicinc.com>
---
 include/net/mac80211.h    | 13 +++++++++++++
 net/mac80211/driver-ops.h | 19 +++++++++++++++++++
 net/mac80211/sta_info.c   |  4 +---
 net/mac80211/trace.h      |  7 +++++++
 4 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index a305e7f9c6b2..fc30e7b0708d 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -4133,6 +4133,15 @@ struct ieee80211_prep_tx_info {
  *	Statistics that the driver doesn't fill will be filled by mac80211.
  *	The callback can sleep.
  *
+ * @link_sta_statistics: Get link statistics for this station. For example with
+ *	beacon filtering, the statistics kept by mac80211 might not be
+ *	accurate, so let the driver pre-fill the statistics. The driver can
+ *	fill most of the values (indicating which by setting the filled
+ *	bitmap), but not all of them make sense - see the source for which
+ *	ones are possible.
+ *	Statistics that the driver doesn't fill will be filled by mac80211.
+ *	The callback can sleep.
+ *
  * @conf_tx: Configure TX queue parameters (EDCF (aifs, cw_min, cw_max),
  *	bursting) for a hardware TX queue.
  *	Returns a negative error code on failure.
@@ -4627,6 +4636,10 @@ struct ieee80211_ops {
 			   s64 offset);
 	void (*reset_tsf)(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
 	int (*tx_last_beacon)(struct ieee80211_hw *hw);
+	void (*link_sta_statistics)(struct ieee80211_hw *hw,
+				    struct ieee80211_vif *vif,
+				    struct ieee80211_sta *sta,
+				    struct link_station_info *link_sinfo);
 
 	/**
 	 * @ampdu_action:
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 307587c8a003..60940b92c4d1 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -631,6 +631,25 @@ static inline void drv_sta_statistics(struct ieee80211_local *local,
 	trace_drv_return_void(local);
 }
 
+static inline void drv_link_sta_statistics(struct ieee80211_local *local,
+					   struct ieee80211_sub_if_data *sdata,
+					   struct ieee80211_sta *sta,
+					   struct link_station_info *link_sinfo)
+{
+	might_sleep();
+	lockdep_assert_wiphy(local->hw.wiphy);
+
+	sdata = get_bss_sdata(sdata);
+	if (!check_sdata_in_driver(sdata))
+		return;
+
+	trace_drv_link_sta_statistics(local, sdata, sta);
+	if (local->ops->link_sta_statistics)
+		local->ops->link_sta_statistics(&local->hw, &sdata->vif, sta,
+						link_sinfo);
+	trace_drv_return_void(local);
+}
+
 int drv_conf_tx(struct ieee80211_local *local,
 		struct ieee80211_link_data *link, u16 ac,
 		const struct ieee80211_tx_queue_params *params);
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 906569651fbd..ec0512c6c50a 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -2724,9 +2724,7 @@ static void sta_set_link_sinfo(struct sta_info *sta,
 
 	ether_addr_copy(link_sinfo->addr, link_sta_info->addr);
 
-	/* TODO: add drv_link_sta_statistics() ops to fill link_station
-	 * statistics of station.
-	 */
+	drv_link_sta_statistics(sta->local, sdata, &sta->sta, link_sinfo);
 
 	link_sinfo->filled |= BIT_ULL(NL80211_STA_INFO_INACTIVE_TIME) |
 			 BIT_ULL(NL80211_STA_INFO_BSS_PARAM) |
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index 72fad8ea8bb9..72ed15d605f6 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -1002,6 +1002,13 @@ DEFINE_EVENT(sta_event, drv_sta_statistics,
 	TP_ARGS(local, sdata, sta)
 );
 
+DEFINE_EVENT(sta_event, drv_link_sta_statistics,
+	TP_PROTO(struct ieee80211_local *local,
+		 struct ieee80211_sub_if_data *sdata,
+		 struct ieee80211_sta *sta),
+	TP_ARGS(local, sdata, sta)
+);
+
 DEFINE_EVENT(sta_event, drv_sta_add,
 	TP_PROTO(struct ieee80211_local *local,
 		 struct ieee80211_sub_if_data *sdata,
-- 
2.34.1

[PATCH wireless-next v8 10/10] wifi: mac80211: correct RX stats packet increment for multi-link

[Sarika Sharma]

Currently, RX stats packets are incremented for deflink member for
non-ML and multi-link(ML) station case. However, for ML station,
packets should be incremented based on the specific link.

Therefore, if a valid link_id is present, fetch the corresponding
link station information and increment the RX packets for that link.
For non-MLO stations, the deflink will still be used.

Signed-off-by: Sarika Sharma <quic_sarishar@quicinc.com>
---
 net/mac80211/rx.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 09beb65d6108..a3222c0e86ac 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -231,8 +231,19 @@ static void __ieee80211_queue_skb_to_iface(struct ieee80211_sub_if_data *sdata,
 
 	skb_queue_tail(&sdata->skb_queue, skb);
 	wiphy_work_queue(sdata->local->hw.wiphy, &sdata->work);
-	if (sta)
-		sta->deflink.rx_stats.packets++;
+	if (sta) {
+		struct link_sta_info *link_sta_info;
+
+		if (link_id >= 0) {
+			link_sta_info = rcu_dereference(sta->link[link_id]);
+			if (!link_sta_info)
+				return;
+		} else {
+			link_sta_info = &sta->deflink;
+		}
+
+		link_sta_info->rx_stats.packets++;
+	}
 }
 
 static void ieee80211_queue_skb_to_iface(struct ieee80211_sub_if_data *sdata,
-- 
2.34.1

Re: [PATCH wireless-next v8 00/10] wifi: cfg80211/mac80211: add support to handle per link statistics of multi-link station

[Johannes Berg]

On Thu, 2025-05-15 at 11:18 +0530, Sarika Sharma wrote:
> Current implementation of NL80211_CMD_GET_STATION does not work
> for multi-link operation(MLO) since in case of MLO only deflink
> (or one of the links) is considered and not all links.
> 
> Hence, add the link_sinfo structure to provide infrastructure
> for link-level station statistics for multi-link operation(MLO).
> 
> Additionally, accumulated stats for MLO are included in a concise
> manner to provide a comprehensive overview of the ML stations.
> 

Uh. So I really wanted to apply this now, and also started making fixes
to it, but no, it's not really working for me at this stage.

I've pushed what I had to wireless-next link-sta-stats branch so you can
see what fixes I already made (and pick them up), but I have further
comments.

Jeff, I think you should probably get stuff that doesn't depend on this
out soon, from your pending branch, we'd have to have really quick
turnaround on all of this to get it all in place for 6.16 still.

johannes

Re: [PATCH wireless-next v8 02/10] wifi: cfg80211: add link_station_info structure to support MLO statistics

[Johannes Berg]

On Thu, 2025-05-15 at 11:18 +0530, Sarika Sharma wrote:
> 
> + * @addr: For MLO STA connection, filled with address of the link of station.
> + **/
> +
> +struct link_station_info {

nit: neither ** nor the blank line should be there

johannes

Re: [PATCH wireless-next v8 03/10] wifi: cfg80211: extend to embed link level statistics in NL message

[Johannes Berg]

On Thu, 2025-05-15 at 11:18 +0530, Sarika Sharma wrote:
> 
> +		for_each_valid_link(sinfo, link_id) {
> +			link_sinfo = sinfo->links[link_id];
> +			link = nla_nest_start(msg, link_id + 1);
> +			if (!link)
> +				goto nla_put_failure;
> +
> +			if (nla_put_u8(msg, NL80211_ATTR_MLO_LINK_ID,
> +				       link_id))
> +				goto nla_put_failure;
> +
> +			if (link_sinfo &&
> +			    !is_zero_ether_addr(link_sinfo->addr) &&
> +			    nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN,
> +				    link_sinfo->addr))
> +				goto nla_put_failure;
> +
> +			if (link_sinfo && nl80211_fill_link_station(msg, rdev,
> +								    link_sinfo))
> +				goto nla_put_failure;
> +
> +			nla_nest_end(msg, link);

This seems wrong, should be is_valid_ether_addr(), and
WARN_ON(!link_sinfo), and that really shouldn't even be possible after
iterating the bitmap. In all of those cases it also really shouldn't
build the nested attribute at all. (I had made those changes.)

johannes

Re: [PATCH wireless-next v8 02/10] wifi: cfg80211: add link_station_info structure to support MLO statistics

[Johannes Berg]

On Thu, 2025-05-15 at 11:18 +0530, Sarika Sharma wrote:
> 
> +/**
> + * struct link_station_info - link station information
> + *
> + * Link station information filled by driver for get_station() and
> + *	dump_station().
> + * @link_id: Link ID uniquely identifying the link STA. This is -1 for non-ML

Oh also, the link_id shouldn't be in here, the documentation is wrong (-
1 is never used that way), and it's already indexed by link ID so it's
just a source of errors to have two values that should be the same...

And yes this implies changes to patch 9, but those are _good_, having
the link ID implicit in the output structure seems awkward.

johannes

Re: [PATCH wireless-next v8 05/10] wifi: cfg80211: allocate memory for link_station info structure

[Johannes Berg]

On Thu, 2025-05-15 at 11:18 +0530, Sarika Sharma wrote:
> 
> +++ b/include/net/cfg80211.h
> @@ -8579,7 +8579,16 @@ int cfg80211_sinfo_alloc_tid_stats(struct station_info *sinfo, gfp_t gfp);
>   */
>  static inline void cfg80211_sinfo_release_content(struct station_info *sinfo)
>  {
> +	int link_id;
> +
>  	kfree(sinfo->pertid);
> +
> +	for_each_valid_link(sinfo, link_id) {
> +		if (sinfo->links[link_id]) {

Took me some time, but obviously this leaks memory all the time. The
hwsim tests blew up for me _immediately_ on this, FWIW, so maybe
consider changing your config to have more kernel debug stuff when you
run those. You do, right? :)


Ultimately, also I know I said to do it this way, but now that I really
look and see how the tidstats are allocated on demand, we could do the
same here?

And then you don't even (later) need
cfg80211_link_sinfo_alloc_tid_stats(), allocating tid stats for the link
or not could be an argument to allocating the link?

Anyway maybe we don't need to change that right now, but I think longer
term that'd be the better internal API.

johannes

Re: [PATCH wireless-next v8 06/10] wifi: mac80211: add support to accumulate removed link statistics

[Johannes Berg]

On Thu, 2025-05-15 at 11:19 +0530, Sarika Sharma wrote:
> 
> +void sta_set_accumulated_removed_links_sinfo(struct sta_info *sta,
> +					     struct station_info *sinfo)
> +{
> +	/* Resetting the MLO statistics for accumulated fields, to
> +	 * avoid duplication.
> +	 */
> +	sinfo->tx_packets = 0;
> +	sinfo->rx_packets = 0;
> +	sinfo->tx_bytes = 0;
> +	sinfo->rx_bytes = 0;
> +	sinfo->tx_retries = 0;
> +	sinfo->tx_failed = 0;
> +	sinfo->rx_dropped_misc = 0;
> +	sinfo->beacon_loss_count = 0;
> +	sinfo->expected_throughput = 0;
> +	sinfo->rx_mpdu_count = 0;
> +	sinfo->fcs_err_count = 0;
> +	sinfo->rx_beacon = 0;
> +	sinfo->rx_duration = 0;
> +	sinfo->tx_duration = 0;
> +
> +	/* Accumulating the removed link statistics. */
> +	sinfo->tx_packets += sta->rem_link_stats.tx_packets;
> +	sinfo->rx_packets += sta->rem_link_stats.rx_packets;
> +	sinfo->tx_bytes += sta->rem_link_stats.tx_bytes;
> +	sinfo->rx_bytes += sta->rem_link_stats.rx_bytes;
> +	sinfo->tx_retries += sta->rem_link_stats.tx_retries;
> +	sinfo->tx_failed += sta->rem_link_stats.tx_failed;
> +	sinfo->rx_dropped_misc += sta->rem_link_stats.rx_dropped_misc;

Setting something to 0 just to += it seems silly?

However I think it also needs a bit more explanation - it's sinfo, so
it's zeroed at allocation, where would non-zero numbers come from?

johannes

Re: [PATCH wireless-next v8 03/10] wifi: cfg80211: extend to embed link level statistics in NL message

[Johannes Berg]

Oh ... missed this one when I sent the other mail. And really this is
the one that I saw and decided not to continue fixing things up myself.

On Thu, 2025-05-15 at 11:18 +0530, Sarika Sharma wrote:
> 
> @@ -6880,7 +7062,8 @@ static int nl80211_send_station(struct sk_buff *msg, u32 cmd, u32 portid,
>  #undef PUT_SINFO
>  #undef PUT_SINFO_U64
>  
> -	if (sinfo->pertid) {
> +	if ((sinfo->filled & BIT_ULL(NL80211_STA_INFO_TID_STATS)) &&
> +	    sinfo->pertid) {

This breaks mac80211 at this point. You also never even mention it in
the commit log.

Ultimately, I see why you did it, but that's really not how we do
things. If we cannot get away without this, then it really needs to be
split out into separate patch that doesn't break mac80211. The change to
fix mac80211 is also randomly interleaved into a commit that doesn't
even mention it.

I would prefer to not do this, but if we really need it then you need to
split out this API change and do that properly while fixing all the
users.

johannes

Re: [PATCH wireless-next v8 04/10] wifi: cfg80211: add statistics for providing overview for MLO station

[Johannes Berg]

On Thu, 2025-05-15 at 11:18 +0530, Sarika Sharma wrote:
> 
> +static void cfg80211_sta_set_mld_rate_info(struct rate_info *sinfo_rate,
> +					   struct rate_info *link_sinfo_rate)
> +{
> +	sinfo_rate->flags = link_sinfo_rate->flags;
> +	sinfo_rate->legacy = link_sinfo_rate->legacy;
> +	sinfo_rate->mcs = link_sinfo_rate->mcs;
> +	sinfo_rate->nss = link_sinfo_rate->nss;
> +	sinfo_rate->bw = link_sinfo_rate->bw;
> +	sinfo_rate->he_gi = link_sinfo_rate->he_gi;
> +	sinfo_rate->he_dcm = link_sinfo_rate->he_dcm;
> +	sinfo_rate->he_ru_alloc = link_sinfo_rate->he_ru_alloc;
> +	sinfo_rate->n_bonded_ch = link_sinfo_rate->n_bonded_ch;
> +	sinfo_rate->eht_gi = link_sinfo_rate->eht_gi;
> +	sinfo_rate->eht_ru_alloc = link_sinfo_rate->eht_ru_alloc;
> +}

What, no. Remove this and just do

 *sinfo_rate = *link_sinfo_rate;

johannes

Re: [PATCH wireless-next v8 07/10] wifi: cfg80211: reset sinfo->filled for MLO station statistics

[Johannes Berg]

On Thu, 2025-05-15 at 11:19 +0530, Sarika Sharma wrote:
> Currently, sinfo->filled is for set in sta_set_sinfo() after filling
> the corresponding fields in station_info structure for station statistics.
> 
> For non-ML stations, the fields are correctly filled from sta->deflink
> and corresponding sinfo->filled bit are set, but for MLO any one of
> link's data is filled and corresponding sinfo->filled bit is set.
> 
> For MLO before embed NL message, fields of sinfo structure like
> bytes, packets, signal are updated with accumulated, best, least of all
> links data. But some of fields like rssi, pertid don't make much sense
> at MLO level.
> 
> Hence, to prevent misinterpretation, reset sinfo->filled for fields

                                       ^^^^^ clear?

> which don't make much sense at MLO level. This will prevent filling the
> unnecessary values in NL message.

Not sure I'd say "unnecessary" but perhaps misleading? I'm also not sure
this shouldn't be WARN_ON, we're throwing away data that was provided.
In mac80211 it even allocates tidstats memory for nothing, in this case,
that's super weird?

johannes

Re: [PATCH wireless-next v8 09/10] wifi: mac80211: add link_sta_statistics ops to fill link station statistics

[Johannes Berg]

On Thu, 2025-05-15 at 11:19 +0530, Sarika Sharma wrote:
> 
> +	void (*link_sta_statistics)(struct ieee80211_hw *hw,
> +				    struct ieee80211_vif *vif,
> +				    struct ieee80211_sta *sta,
> +				    struct link_station_info *link_sinfo);

That should pass the link STA, not the STA, not just because I removed
the link_id from link_sinfo, but also that just generally makes way more
sense? Looking at this prototype you have no idea how to even do
anything link related.

johannes

Re: [PATCH wireless-next v8 06/10] wifi: mac80211: add support to accumulate removed link statistics

[Johannes Berg]

Also,

On Thu, 2025-05-15 at 11:19 +0530, Sarika Sharma wrote:
> 
> +static void sta_accumulate_removed_link_stats(struct sta_info *sta, int link_id)
> +{
> +	struct link_sta_info *link_sta = wiphy_dereference(sta->local->hw.wiphy,
> +							   sta->link[link_id]);
> +	int ac;
> +
> +	for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
> +		sta->rem_link_stats.tx_packets +=
> +			link_sta->tx_stats.packets[ac];
> +		sta->rem_link_stats.tx_bytes += link_sta->tx_stats.bytes[ac];

It seems odd to take per-AC values and flatten them in this case? How do
they even get reported, as overall values only? Then you get the same
inconsistency again on per-AC values since those are (or at least
could/should be) summed up over all links, but aren't kept post removal?

johannes

Re: [PATCH wireless-next v8 08/10] wifi: mac80211: extend support to fill link level sinfo structure

[Johannes Berg]

On Thu, 2025-05-15 at 11:19 +0530, Sarika Sharma wrote:
> 
> +	if (sinfo->valid_links) {
> +		struct ieee80211_link_data *link_sdata;
> 

That's always just called "link" elsewhere, maybe stick to that
convention? There's no mention of link_sdata elsewhere anyway.

johannes

Re: [PATCH wireless-next v8 00/10] wifi: cfg80211/mac80211: add support to handle per link statistics of multi-link station

[Sarika Sharma]

On 5/15/2025 4:49 PM, Johannes Berg wrote:
> On Thu, 2025-05-15 at 11:18 +0530, Sarika Sharma wrote:
>> Current implementation of NL80211_CMD_GET_STATION does not work
>> for multi-link operation(MLO) since in case of MLO only deflink
>> (or one of the links) is considered and not all links.
>>
>> Hence, add the link_sinfo structure to provide infrastructure
>> for link-level station statistics for multi-link operation(MLO).
>>
>> Additionally, accumulated stats for MLO are included in a concise
>> manner to provide a comprehensive overview of the ML stations.
>>
> 
> Uh. So I really wanted to apply this now, and also started making fixes
> to it, but no, it's not really working for me at this stage.
> 
> I've pushed what I had to wireless-next link-sta-stats branch so you can
> see what fixes I already made (and pick them up), but I have further
> comments.

Sure, let me go through them and fix others.
Thankyou!

> 
> Jeff, I think you should probably get stuff that doesn't depend on this
> out soon, from your pending branch, we'd have to have really quick
> turnaround on all of this to get it all in place for 6.16 still.
> 
> johannes

Re: [PATCH wireless-next v8 00/10] wifi: cfg80211/mac80211: add support to handle per link statistics of multi-link station

[Jeff Johnson]

On 5/15/2025 4:19 AM, Johannes Berg wrote:
> On Thu, 2025-05-15 at 11:18 +0530, Sarika Sharma wrote:
>> Current implementation of NL80211_CMD_GET_STATION does not work
>> for multi-link operation(MLO) since in case of MLO only deflink
>> (or one of the links) is considered and not all links.
>>
>> Hence, add the link_sinfo structure to provide infrastructure
>> for link-level station statistics for multi-link operation(MLO).
>>
>> Additionally, accumulated stats for MLO are included in a concise
>> manner to provide a comprehensive overview of the ML stations.
>>
> 
> Uh. So I really wanted to apply this now, and also started making fixes
> to it, but no, it's not really working for me at this stage.
> 
> I've pushed what I had to wireless-next link-sta-stats branch so you can
> see what fixes I already made (and pick them up), but I have further
> comments.
> 
> Jeff, I think you should probably get stuff that doesn't depend on this
> out soon, from your pending branch, we'd have to have really quick
> turnaround on all of this to get it all in place for 6.16 still.

ACK.
I'm also dealing with patches made for AP features breaking non-AP, so I need
to working through the 'pending' patches to see which I can promote and which
I need to defer.

/jeff

Re: [PATCH wireless-next v8 02/10] wifi: cfg80211: add link_station_info structure to support MLO statistics

[Sarika Sharma]

On 5/15/2025 4:51 PM, Johannes Berg wrote:
> On Thu, 2025-05-15 at 11:18 +0530, Sarika Sharma wrote:
>>
>> + * @addr: For MLO STA connection, filled with address of the link of station.
>> + **/
>> +
>> +struct link_station_info {
> 
> nit: neither ** nor the blank line should be there
> 

Sure, I see you already corrected it.
Thanks!

> johannes

Re: [PATCH wireless-next v8 03/10] wifi: cfg80211: extend to embed link level statistics in NL message

[Sarika Sharma]

On 5/15/2025 4:55 PM, Johannes Berg wrote:
> On Thu, 2025-05-15 at 11:18 +0530, Sarika Sharma wrote:
>>
>> +		for_each_valid_link(sinfo, link_id) {
>> +			link_sinfo = sinfo->links[link_id];
>> +			link = nla_nest_start(msg, link_id + 1);
>> +			if (!link)
>> +				goto nla_put_failure;
>> +
>> +			if (nla_put_u8(msg, NL80211_ATTR_MLO_LINK_ID,
>> +				       link_id))
>> +				goto nla_put_failure;
>> +
>> +			if (link_sinfo &&
>> +			    !is_zero_ether_addr(link_sinfo->addr) &&
>> +			    nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN,
>> +				    link_sinfo->addr))
>> +				goto nla_put_failure;
>> +
>> +			if (link_sinfo && nl80211_fill_link_station(msg, rdev,
>> +								    link_sinfo))
>> +				goto nla_put_failure;
>> +
>> +			nla_nest_end(msg, link);
> 
> This seems wrong, should be is_valid_ether_addr(), and
> WARN_ON(!link_sinfo), and that really shouldn't even be possible after
> iterating the bitmap. In all of those cases it also really shouldn't
> build the nested attribute at all. (I had made those changes.)

Sure, let me pull those patches.
Thankyou!

> 
> johannes

Re: [PATCH wireless-next v8 02/10] wifi: cfg80211: add link_station_info structure to support MLO statistics

[Sarika Sharma]

On 5/15/2025 4:56 PM, Johannes Berg wrote:
> On Thu, 2025-05-15 at 11:18 +0530, Sarika Sharma wrote:
>>
>> +/**
>> + * struct link_station_info - link station information
>> + *
>> + * Link station information filled by driver for get_station() and
>> + *	dump_station().
>> + * @link_id: Link ID uniquely identifying the link STA. This is -1 for non-ML
> 
> Oh also, the link_id shouldn't be in here, the documentation is wrong (-
> 1 is never used that way), and it's already indexed by link ID so it's
> just a source of errors to have two values that should be the same...
> 

Sure, was using in previous design to check for deflink, here we can 
directly use from link_sta.
Let me remove it and use existing link_id.

> And yes this implies changes to patch 9, but those are _good_, having
> the link ID implicit in the output structure seems awkward.

Okay, sure.

> 
> johannes

Re: [PATCH wireless-next v8 03/10] wifi: cfg80211: extend to embed link level statistics in NL message

[Sarika Sharma]

On 5/15/2025 5:04 PM, Johannes Berg wrote:
> Oh ... missed this one when I sent the other mail. And really this is
> the one that I saw and decided not to continue fixing things up myself.
> 
> On Thu, 2025-05-15 at 11:18 +0530, Sarika Sharma wrote:
>>
>> @@ -6880,7 +7062,8 @@ static int nl80211_send_station(struct sk_buff *msg, u32 cmd, u32 portid,
>>   #undef PUT_SINFO
>>   #undef PUT_SINFO_U64
>>   
>> -	if (sinfo->pertid) {
>> +	if ((sinfo->filled & BIT_ULL(NL80211_STA_INFO_TID_STATS)) &&
>> +	    sinfo->pertid) {
> 
> This breaks mac80211 at this point. You also never even mention it in
> the commit log.

oops! Yes correct with this the pertid will not be reported to user-space.

> 
> Ultimately, I see why you did it, but that's really not how we do
> things. If we cannot get away without this, then it really needs to be
> split out into separate patch that doesn't break mac80211. The change to
> fix mac80211 is also randomly interleaved into a commit that doesn't
> even mention it.

Sure, let me introduce this as a separate patch.

Yes, this is needed when we don't really want to report the pertid at 
MLO level( as currently pertid is one of links data(deflink) for MLO).

or may be we can directly free the pertid memory in case of MLO in 
cfg80211 instead of relying on sinfo->filled bit for tid?

> 
> I would prefer to not do this, but if we really need it then you need to
> split out this API change and do that properly while fixing all the
> users.
> 
> johannes

Re: [PATCH wireless-next v8 04/10] wifi: cfg80211: add statistics for providing overview for MLO station

[Sarika Sharma]

On 5/15/2025 5:05 PM, Johannes Berg wrote:
> On Thu, 2025-05-15 at 11:18 +0530, Sarika Sharma wrote:
>>
>> +static void cfg80211_sta_set_mld_rate_info(struct rate_info *sinfo_rate,
>> +					   struct rate_info *link_sinfo_rate)
>> +{
>> +	sinfo_rate->flags = link_sinfo_rate->flags;
>> +	sinfo_rate->legacy = link_sinfo_rate->legacy;
>> +	sinfo_rate->mcs = link_sinfo_rate->mcs;
>> +	sinfo_rate->nss = link_sinfo_rate->nss;
>> +	sinfo_rate->bw = link_sinfo_rate->bw;
>> +	sinfo_rate->he_gi = link_sinfo_rate->he_gi;
>> +	sinfo_rate->he_dcm = link_sinfo_rate->he_dcm;
>> +	sinfo_rate->he_ru_alloc = link_sinfo_rate->he_ru_alloc;
>> +	sinfo_rate->n_bonded_ch = link_sinfo_rate->n_bonded_ch;
>> +	sinfo_rate->eht_gi = link_sinfo_rate->eht_gi;
>> +	sinfo_rate->eht_ru_alloc = link_sinfo_rate->eht_ru_alloc;
>> +}
> 
> What, no. Remove this and just do
> 
>   *sinfo_rate = *link_sinfo_rate;
> 
> johannes
> 
Oops! yes true, can directly do this.
Thanks, will correct it.

Re: [PATCH wireless-next v8 05/10] wifi: cfg80211: allocate memory for link_station info structure

[Sarika Sharma]

On 5/15/2025 4:58 PM, Johannes Berg wrote:
> On Thu, 2025-05-15 at 11:18 +0530, Sarika Sharma wrote:
>>
>> +++ b/include/net/cfg80211.h
>> @@ -8579,7 +8579,16 @@ int cfg80211_sinfo_alloc_tid_stats(struct station_info *sinfo, gfp_t gfp);
>>    */
>>   static inline void cfg80211_sinfo_release_content(struct station_info *sinfo)
>>   {
>> +	int link_id;
>> +
>>   	kfree(sinfo->pertid);
>> +
>> +	for_each_valid_link(sinfo, link_id) {
>> +		if (sinfo->links[link_id]) {
> 
> Took me some time, but obviously this leaks memory all the time. The
> hwsim tests blew up for me _immediately_ on this, FWIW, so maybe
> consider changing your config to have more kernel debug stuff when you
> run those. You do, right? :)
> 

Sure, let me change the configs to avoid this in future.

> 
> Ultimately, also I know I said to do it this way, but now that I really
> look and see how the tidstats are allocated on demand, we could do the
> same here?
> 
> And then you don't even (later) need
> cfg80211_link_sinfo_alloc_tid_stats(), allocating tid stats for the link
> or not could be an argument to allocating the link?
> 
> Anyway maybe we don't need to change that right now, but I think longer
> term that'd be the better internal API.

Sure, we can take this at later part.

> 
> johannes
> 

Re: [PATCH wireless-next v8 06/10] wifi: mac80211: add support to accumulate removed link statistics

[Sarika Sharma]

On 5/15/2025 5:00 PM, Johannes Berg wrote:
> On Thu, 2025-05-15 at 11:19 +0530, Sarika Sharma wrote:
>>
>> +void sta_set_accumulated_removed_links_sinfo(struct sta_info *sta,
>> +					     struct station_info *sinfo)
>> +{
>> +	/* Resetting the MLO statistics for accumulated fields, to
>> +	 * avoid duplication.
>> +	 */
>> +	sinfo->tx_packets = 0;
>> +	sinfo->rx_packets = 0;
>> +	sinfo->tx_bytes = 0;
>> +	sinfo->rx_bytes = 0;
>> +	sinfo->tx_retries = 0;
>> +	sinfo->tx_failed = 0;
>> +	sinfo->rx_dropped_misc = 0;
>> +	sinfo->beacon_loss_count = 0;
>> +	sinfo->expected_throughput = 0;
>> +	sinfo->rx_mpdu_count = 0;
>> +	sinfo->fcs_err_count = 0;
>> +	sinfo->rx_beacon = 0;
>> +	sinfo->rx_duration = 0;
>> +	sinfo->tx_duration = 0;
>> +
>> +	/* Accumulating the removed link statistics. */
>> +	sinfo->tx_packets += sta->rem_link_stats.tx_packets;
>> +	sinfo->rx_packets += sta->rem_link_stats.rx_packets;
>> +	sinfo->tx_bytes += sta->rem_link_stats.tx_bytes;
>> +	sinfo->rx_bytes += sta->rem_link_stats.rx_bytes;
>> +	sinfo->tx_retries += sta->rem_link_stats.tx_retries;
>> +	sinfo->tx_failed += sta->rem_link_stats.tx_failed;
>> +	sinfo->rx_dropped_misc += sta->rem_link_stats.rx_dropped_misc;
> 
> Setting something to 0 just to += it seems silly?
>
> However I think it also needs a bit more explanation - it's sinfo, so
> it's zeroed at allocation, where would non-zero numbers come from?

Currently, the station information for MLO is populated with some values 
from sta->deflink, as the sta_set_sinfo() call is common for both 
non-MLO and MLO.

When updating the station_info structure in 
cfg80211_sta_set_mld_sinfo(), the accumulated fields (such as packets, 
bytes, etc.) will already contain values set by mac80211 (from the 
deflink fields).

Therefore, directly adding to these fields would be incorrect, so they 
should be reset to zero.

May be this, resetting can be done directly in cfg80211 during 
cfg80211_sta_set_mld_sinfo(), will correct this.

> 
> johannes
> 

Re: [PATCH wireless-next v8 06/10] wifi: mac80211: add support to accumulate removed link statistics

[Ben Greear]

On 5/15/25 10:35, Sarika Sharma wrote:
> On 5/15/2025 5:00 PM, Johannes Berg wrote:
>> On Thu, 2025-05-15 at 11:19 +0530, Sarika Sharma wrote:
>>>
>>> +void sta_set_accumulated_removed_links_sinfo(struct sta_info *sta,
>>> +                         struct station_info *sinfo)
>>> +{
>>> +    /* Resetting the MLO statistics for accumulated fields, to
>>> +     * avoid duplication.
>>> +     */
>>> +    sinfo->tx_packets = 0;
>>> +    sinfo->rx_packets = 0;
>>> +    sinfo->tx_bytes = 0;
>>> +    sinfo->rx_bytes = 0;
>>> +    sinfo->tx_retries = 0;
>>> +    sinfo->tx_failed = 0;
>>> +    sinfo->rx_dropped_misc = 0;
>>> +    sinfo->beacon_loss_count = 0;
>>> +    sinfo->expected_throughput = 0;
>>> +    sinfo->rx_mpdu_count = 0;
>>> +    sinfo->fcs_err_count = 0;
>>> +    sinfo->rx_beacon = 0;
>>> +    sinfo->rx_duration = 0;
>>> +    sinfo->tx_duration = 0;
>>> +
>>> +    /* Accumulating the removed link statistics. */
>>> +    sinfo->tx_packets += sta->rem_link_stats.tx_packets;
>>> +    sinfo->rx_packets += sta->rem_link_stats.rx_packets;
>>> +    sinfo->tx_bytes += sta->rem_link_stats.tx_bytes;
>>> +    sinfo->rx_bytes += sta->rem_link_stats.rx_bytes;
>>> +    sinfo->tx_retries += sta->rem_link_stats.tx_retries;
>>> +    sinfo->tx_failed += sta->rem_link_stats.tx_failed;
>>> +    sinfo->rx_dropped_misc += sta->rem_link_stats.rx_dropped_misc;
>>
>> Setting something to 0 just to += it seems silly?
>>
>> However I think it also needs a bit more explanation - it's sinfo, so
>> it's zeroed at allocation, where would non-zero numbers come from?
> 
> Currently, the station information for MLO is populated with some values from sta->deflink, as the sta_set_sinfo() call is common for both non-MLO and MLO.
> 
> When updating the station_info structure in cfg80211_sta_set_mld_sinfo(), the accumulated fields (such as packets, bytes, etc.) will already contain values set 
> by mac80211 (from the deflink fields).
> 
> Therefore, directly adding to these fields would be incorrect, so they should be reset to zero.
> 
> May be this, resetting can be done directly in cfg80211 during cfg80211_sta_set_mld_sinfo(), will correct this.

If nothing else, you could just do assignment instead of setting to zero and then
incrementing?

I did not actually review the over-all logic, so perhaps there are bigger
issues I'm not aware of.

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH wireless-next v8 07/10] wifi: cfg80211: reset sinfo->filled for MLO station statistics

[Sarika Sharma]

On 5/15/2025 5:06 PM, Johannes Berg wrote:
> On Thu, 2025-05-15 at 11:19 +0530, Sarika Sharma wrote:
>> Currently, sinfo->filled is for set in sta_set_sinfo() after filling
>> the corresponding fields in station_info structure for station statistics.
>>
>> For non-ML stations, the fields are correctly filled from sta->deflink
>> and corresponding sinfo->filled bit are set, but for MLO any one of
>> link's data is filled and corresponding sinfo->filled bit is set.
>>
>> For MLO before embed NL message, fields of sinfo structure like
>> bytes, packets, signal are updated with accumulated, best, least of all
>> links data. But some of fields like rssi, pertid don't make much sense
>> at MLO level.
>>
>> Hence, to prevent misinterpretation, reset sinfo->filled for fields
> 
>                                         ^^^^^ clear?

sure.

> 
>> which don't make much sense at MLO level. This will prevent filling the
>> unnecessary values in NL message.
> 
> Not sure I'd say "unnecessary" but perhaps misleading? I'm also not sure
> this shouldn't be WARN_ON, we're throwing away data that was provided.
> In mac80211 it even allocates tidstats memory for nothing, in this case,
> that's super weird?
> 

Yes, that why I initially came up with the other design.
Some of fields, that are getting filled in mac80211 for sinfo are from 
sta->deflink, i.e one of the links for MLO.

Now either we have to split the sta_set_sinfo(), such a way that some of 
fields applicable at station level is only filled by mac80211 in 
station_info structure for both non-ML and MLO and others which are not 
applicable or filled from sta->deflink, need not be filled during MLO 
case or if filled it need to be free.

> johannes

Re: [PATCH wireless-next v8 08/10] wifi: mac80211: extend support to fill link level sinfo structure

[Sarika Sharma]

On 5/15/2025 5:11 PM, Johannes Berg wrote:
> On Thu, 2025-05-15 at 11:19 +0530, Sarika Sharma wrote:
>>
>> +	if (sinfo->valid_links) {
>> +		struct ieee80211_link_data *link_sdata;
>>
> 
> That's always just called "link" elsewhere, maybe stick to that
> convention? There's no mention of link_sdata elsewhere anyway.
> 
> johannes

Sure.

Re: [PATCH wireless-next v8 09/10] wifi: mac80211: add link_sta_statistics ops to fill link station statistics

[Sarika Sharma]

On 5/15/2025 5:08 PM, Johannes Berg wrote:
> On Thu, 2025-05-15 at 11:19 +0530, Sarika Sharma wrote:
>>
>> +	void (*link_sta_statistics)(struct ieee80211_hw *hw,
>> +				    struct ieee80211_vif *vif,
>> +				    struct ieee80211_sta *sta,
>> +				    struct link_station_info *link_sinfo);
> 
> That should pass the link STA, not the STA, not just because I removed
> the link_id from link_sinfo, but also that just generally makes way more
> sense? Looking at this prototype you have no idea how to even do
> anything link related.

Oops! my bad :-(, just took this change from previous design.
missed this to change.

> 
> johannes

Re: [PATCH wireless-next v8 06/10] wifi: mac80211: add support to accumulate removed link statistics

[Sarika Sharma]

On 5/15/2025 5:11 PM, Johannes Berg wrote:
> Also,
> 
> On Thu, 2025-05-15 at 11:19 +0530, Sarika Sharma wrote:
>>
>> +static void sta_accumulate_removed_link_stats(struct sta_info *sta, int link_id)
>> +{
>> +	struct link_sta_info *link_sta = wiphy_dereference(sta->local->hw.wiphy,
>> +							   sta->link[link_id]);
>> +	int ac;
>> +
>> +	for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
>> +		sta->rem_link_stats.tx_packets +=
>> +			link_sta->tx_stats.packets[ac];
>> +		sta->rem_link_stats.tx_bytes += link_sta->tx_stats.bytes[ac];
> 
> It seems odd to take per-AC values and flatten them in this case? How do
> they even get reported, as overall values only? Then you get the same
> inconsistency again on per-AC values since those are (or at least
> could/should be) summed up over all links, but aren't kept post removal?

Looks like we can flatten them. In existing code as well we have 
flattened and use, during sta_set_sinfo()

         "if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_PACKETS))) {
                 sinfo->tx_packets = 0;
                 for (ac = 0; ac < IEEE80211_NUM_ACS; ac++)
                         sinfo->tx_packets += 
sta->deflink.tx_stats.packets[ac];
                 sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_PACKETS);
         } "

> 
> johannes

Re: [PATCH wireless-next v8 06/10] wifi: mac80211: add support to accumulate removed link statistics

[Sarika Sharma]

On 5/15/2025 11:08 PM, Ben Greear wrote:
> On 5/15/25 10:35, Sarika Sharma wrote:
>> On 5/15/2025 5:00 PM, Johannes Berg wrote:
>>> On Thu, 2025-05-15 at 11:19 +0530, Sarika Sharma wrote:
>>>>
>>>> +void sta_set_accumulated_removed_links_sinfo(struct sta_info *sta,
>>>> +                         struct station_info *sinfo)
>>>> +{
>>>> +    /* Resetting the MLO statistics for accumulated fields, to
>>>> +     * avoid duplication.
>>>> +     */
>>>> +    sinfo->tx_packets = 0;
>>>> +    sinfo->rx_packets = 0;
>>>> +    sinfo->tx_bytes = 0;
>>>> +    sinfo->rx_bytes = 0;
>>>> +    sinfo->tx_retries = 0;
>>>> +    sinfo->tx_failed = 0;
>>>> +    sinfo->rx_dropped_misc = 0;
>>>> +    sinfo->beacon_loss_count = 0;
>>>> +    sinfo->expected_throughput = 0;
>>>> +    sinfo->rx_mpdu_count = 0;
>>>> +    sinfo->fcs_err_count = 0;
>>>> +    sinfo->rx_beacon = 0;
>>>> +    sinfo->rx_duration = 0;
>>>> +    sinfo->tx_duration = 0;
>>>> +
>>>> +    /* Accumulating the removed link statistics. */
>>>> +    sinfo->tx_packets += sta->rem_link_stats.tx_packets;
>>>> +    sinfo->rx_packets += sta->rem_link_stats.rx_packets;
>>>> +    sinfo->tx_bytes += sta->rem_link_stats.tx_bytes;
>>>> +    sinfo->rx_bytes += sta->rem_link_stats.rx_bytes;
>>>> +    sinfo->tx_retries += sta->rem_link_stats.tx_retries;
>>>> +    sinfo->tx_failed += sta->rem_link_stats.tx_failed;
>>>> +    sinfo->rx_dropped_misc += sta->rem_link_stats.rx_dropped_misc;
>>>
>>> Setting something to 0 just to += it seems silly?
>>>
>>> However I think it also needs a bit more explanation - it's sinfo, so
>>> it's zeroed at allocation, where would non-zero numbers come from?
>>
>> Currently, the station information for MLO is populated with some 
>> values from sta->deflink, as the sta_set_sinfo() call is common for 
>> both non-MLO and MLO.
>>
>> When updating the station_info structure in 
>> cfg80211_sta_set_mld_sinfo(), the accumulated fields (such as packets, 
>> bytes, etc.) will already contain values set by mac80211 (from the 
>> deflink fields).
>>
>> Therefore, directly adding to these fields would be incorrect, so they 
>> should be reset to zero.
>>
>> May be this, resetting can be done directly in cfg80211 during 
>> cfg80211_sta_set_mld_sinfo(), will correct this.
> 
> If nothing else, you could just do assignment instead of setting to zero 
> and then
> incrementing?
> 
> I did not actually review the over-all logic, so perhaps there are bigger
> issues I'm not aware of.

Yes, here I can directly use assignment.
But some of fields I need to reset to 0 in cfg80211.
Will check and update this in next version.

> 
> Thanks,
> Ben
> 
> 

Re: [PATCH wireless-next v8 07/10] wifi: cfg80211: reset sinfo->filled for MLO station statistics

[Sarika Sharma]

On 5/15/2025 5:06 PM, Johannes Berg wrote:
> On Thu, 2025-05-15 at 11:19 +0530, Sarika Sharma wrote:
>> Currently, sinfo->filled is for set in sta_set_sinfo() after filling
>> the corresponding fields in station_info structure for station statistics.
>>
>> For non-ML stations, the fields are correctly filled from sta->deflink
>> and corresponding sinfo->filled bit are set, but for MLO any one of
>> link's data is filled and corresponding sinfo->filled bit is set.
>>
>> For MLO before embed NL message, fields of sinfo structure like
>> bytes, packets, signal are updated with accumulated, best, least of all
>> links data. But some of fields like rssi, pertid don't make much sense
>> at MLO level.
>>
>> Hence, to prevent misinterpretation, reset sinfo->filled for fields
> 
>                                         ^^^^^ clear?
> 
>> which don't make much sense at MLO level. This will prevent filling the
>> unnecessary values in NL message.
> 
> Not sure I'd say "unnecessary" but perhaps misleading? I'm also not sure
> this shouldn't be WARN_ON, we're throwing away data that was provided.
> In mac80211 it even allocates tidstats memory for nothing, in this case,
> that's super weird?
> 

I reviewed the code again to check for pertid and realized that it is 
applicable at the MLO level as well. The rx, tx fields are reported per 
link (which can be accumulated for the MLO level), while the TXQ_STATS 
related fields are applicable for station level meaning MLO level.

Therefore, pertid is applicable at MLO level and also at link level for 
rx/tx fields.

Let me add code for it and send the version 9?

> johannes

Re: [PATCH wireless-next v8 03/10] wifi: cfg80211: extend to embed link level statistics in NL message

[Johannes Berg]

On Thu, 2025-05-15 at 22:28 +0530, Sarika Sharma wrote:
> 
> Yes, this is needed when we don't really want to report the pertid at 
> MLO level( as currently pertid is one of links data(deflink) for MLO).

I'm not sure it makes sense at all. Why would we have mac80211 provide
some data that we then discard immediately? Maybe we can

 if (WARN_ON(pertid_data)) {
    kfree(pertid_data);
    pertid_data = NULL;
 }

for userspace consistency, but I think we should really avoid doing work
to create data that doesn't make sense and isn't needed.

johannes

Re: [PATCH wireless-next v8 06/10] wifi: mac80211: add support to accumulate removed link statistics

[Johannes Berg]

On Thu, 2025-05-15 at 23:05 +0530, Sarika Sharma wrote:
> On 5/15/2025 5:00 PM, Johannes Berg wrote:
> > On Thu, 2025-05-15 at 11:19 +0530, Sarika Sharma wrote:
> > > 
> > > +void sta_set_accumulated_removed_links_sinfo(struct sta_info *sta,
> > > +					     struct station_info *sinfo)
> > > +{
> > > +	/* Resetting the MLO statistics for accumulated fields, to
> > > +	 * avoid duplication.
> > > +	 */
> > > +	sinfo->tx_packets = 0;
> > > +	sinfo->rx_packets = 0;
> > > +	sinfo->tx_bytes = 0;
> > > +	sinfo->rx_bytes = 0;
> > > +	sinfo->tx_retries = 0;
> > > +	sinfo->tx_failed = 0;
> > > +	sinfo->rx_dropped_misc = 0;
> > > +	sinfo->beacon_loss_count = 0;
> > > +	sinfo->expected_throughput = 0;
> > > +	sinfo->rx_mpdu_count = 0;
> > > +	sinfo->fcs_err_count = 0;
> > > +	sinfo->rx_beacon = 0;
> > > +	sinfo->rx_duration = 0;
> > > +	sinfo->tx_duration = 0;
> > > +
> > > +	/* Accumulating the removed link statistics. */
> > > +	sinfo->tx_packets += sta->rem_link_stats.tx_packets;
> > > +	sinfo->rx_packets += sta->rem_link_stats.rx_packets;
> > > +	sinfo->tx_bytes += sta->rem_link_stats.tx_bytes;
> > > +	sinfo->rx_bytes += sta->rem_link_stats.rx_bytes;
> > > +	sinfo->tx_retries += sta->rem_link_stats.tx_retries;
> > > +	sinfo->tx_failed += sta->rem_link_stats.tx_failed;
> > > +	sinfo->rx_dropped_misc += sta->rem_link_stats.rx_dropped_misc;
> > 
> > Setting something to 0 just to += it seems silly?
> > 
> > However I think it also needs a bit more explanation - it's sinfo, so
> > it's zeroed at allocation, where would non-zero numbers come from?
> 
> Currently, the station information for MLO is populated with some values 
> from sta->deflink, as the sta_set_sinfo() call is common for both 
> non-MLO and MLO.

OK but deflink will not actually _have_ any values. And again, mac80211
shouldn't be doing work the results of which are then only discarded...

> When updating the station_info structure in 
> cfg80211_sta_set_mld_sinfo(), the accumulated fields (such as packets, 
> bytes, etc.) will already contain values set by mac80211 (from the 
> deflink fields).

Which arguably is the problem? But also not because those are zero
anyway? I still don't think this makes any sense. Either it's not filled
and remains zero, or it should be filled only with some sensible values
like the accumulated stats from removed links.

johannes

Re: [PATCH wireless-next v8 06/10] wifi: mac80211: add support to accumulate removed link statistics

[Johannes Berg]

On Thu, 2025-05-15 at 23:17 +0530, Sarika Sharma wrote:
> 
> Looks like we can flatten them. In existing code as well we have 
> flattened and use, during sta_set_sinfo()
> 
>          "if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_PACKETS))) {
>                  sinfo->tx_packets = 0;
>                  for (ac = 0; ac < IEEE80211_NUM_ACS; ac++)
>                          sinfo->tx_packets += 
> sta->deflink.tx_stats.packets[ac];
>                  sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_PACKETS);
>          } "
> 

Indeed, weird, why do we even bother counting them per AC?

johannes

Re: [PATCH wireless-next v8 07/10] wifi: cfg80211: reset sinfo->filled for MLO station statistics

[Johannes Berg]

On Sun, 2025-05-18 at 22:04 +0530, Sarika Sharma wrote:
> 
> I reviewed the code again to check for pertid and realized that it is 
> applicable at the MLO level as well. The rx, tx fields are reported per 
> link (which can be accumulated for the MLO level), while the TXQ_STATS 
> related fields are applicable for station level meaning MLO level.
> 
> Therefore, pertid is applicable at MLO level and also at link level for 
> rx/tx fields.
> 

Hm OK? Not sure I completely follow. Still seems mac80211 should only
fill that up with accumulated-from-removed-links stats?

And now it gets pretty tricky since we have some stats that are
accumulated from removed links, and some that aren't?

johannes

Re: [PATCH wireless-next v8 06/10] wifi: mac80211: add support to accumulate removed link statistics

[Sarika Sharma]

On 5/20/2025 2:08 PM, Johannes Berg wrote:
> On Thu, 2025-05-15 at 23:05 +0530, Sarika Sharma wrote:
>> On 5/15/2025 5:00 PM, Johannes Berg wrote:
>>> On Thu, 2025-05-15 at 11:19 +0530, Sarika Sharma wrote:
>>>>
>>>> +void sta_set_accumulated_removed_links_sinfo(struct sta_info *sta,
>>>> +					     struct station_info *sinfo)
>>>> +{
>>>> +	/* Resetting the MLO statistics for accumulated fields, to
>>>> +	 * avoid duplication.
>>>> +	 */
>>>> +	sinfo->tx_packets = 0;
>>>> +	sinfo->rx_packets = 0;
>>>> +	sinfo->tx_bytes = 0;
>>>> +	sinfo->rx_bytes = 0;
>>>> +	sinfo->tx_retries = 0;
>>>> +	sinfo->tx_failed = 0;
>>>> +	sinfo->rx_dropped_misc = 0;
>>>> +	sinfo->beacon_loss_count = 0;
>>>> +	sinfo->expected_throughput = 0;
>>>> +	sinfo->rx_mpdu_count = 0;
>>>> +	sinfo->fcs_err_count = 0;
>>>> +	sinfo->rx_beacon = 0;
>>>> +	sinfo->rx_duration = 0;
>>>> +	sinfo->tx_duration = 0;
>>>> +
>>>> +	/* Accumulating the removed link statistics. */
>>>> +	sinfo->tx_packets += sta->rem_link_stats.tx_packets;
>>>> +	sinfo->rx_packets += sta->rem_link_stats.rx_packets;
>>>> +	sinfo->tx_bytes += sta->rem_link_stats.tx_bytes;
>>>> +	sinfo->rx_bytes += sta->rem_link_stats.rx_bytes;
>>>> +	sinfo->tx_retries += sta->rem_link_stats.tx_retries;
>>>> +	sinfo->tx_failed += sta->rem_link_stats.tx_failed;
>>>> +	sinfo->rx_dropped_misc += sta->rem_link_stats.rx_dropped_misc;
>>>
>>> Setting something to 0 just to += it seems silly?
>>>
>>> However I think it also needs a bit more explanation - it's sinfo, so
>>> it's zeroed at allocation, where would non-zero numbers come from?
>>
>> Currently, the station information for MLO is populated with some values
>> from sta->deflink, as the sta_set_sinfo() call is common for both
>> non-MLO and MLO.
> 
> OK but deflink will not actually _have_ any values. And again, mac80211
> shouldn't be doing work the results of which are then only discarded...
> 
>> When updating the station_info structure in
>> cfg80211_sta_set_mld_sinfo(), the accumulated fields (such as packets,
>> bytes, etc.) will already contain values set by mac80211 (from the
>> deflink fields).
> 
> Which arguably is the problem? But also not because those are zero
> anyway? I still don't think this makes any sense. Either it's not filled
> and remains zero, or it should be filled only with some sensible values
> like the accumulated stats from removed links.

okay, then either changes needed to be done so, some fields way of 
filling data is different for non-ML and MLO station or need to maintain 
all accumulated removed links data for addable fields.

Let me check and add this.

> 
> johannes

Re: [PATCH wireless-next v8 07/10] wifi: cfg80211: reset sinfo->filled for MLO station statistics

[Sarika Sharma]

On 5/20/2025 2:12 PM, Johannes Berg wrote:
> On Sun, 2025-05-18 at 22:04 +0530, Sarika Sharma wrote:
>>
>> I reviewed the code again to check for pertid and realized that it is
>> applicable at the MLO level as well. The rx, tx fields are reported per
>> link (which can be accumulated for the MLO level), while the TXQ_STATS
>> related fields are applicable for station level meaning MLO level.
>>
>> Therefore, pertid is applicable at MLO level and also at link level for
>> rx/tx fields.
>>
> 
> Hm OK? Not sure I completely follow. Still seems mac80211 should only
> fill that up with accumulated-from-removed-links stats?
> 
> And now it gets pretty tricky since we have some stats that are
> accumulated from removed links, and some that aren't?

Then it seems I need to maintain all accumulated fields data to be 
filled from accumulated-from-removed-links stats first, that will 
automatically reset if some deflink values are there and fill with 
removed links stats.

> 
> johannes

Re: [PATCH wireless-next v8 06/10] wifi: mac80211: add support to accumulate removed link statistics

[Sarika Sharma]

On 5/20/2025 2:10 PM, Johannes Berg wrote:
> On Thu, 2025-05-15 at 23:17 +0530, Sarika Sharma wrote:
>>
>> Looks like we can flatten them. In existing code as well we have
>> flattened and use, during sta_set_sinfo()
>>
>>           "if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_PACKETS))) {
>>                   sinfo->tx_packets = 0;
>>                   for (ac = 0; ac < IEEE80211_NUM_ACS; ac++)
>>                           sinfo->tx_packets +=
>> sta->deflink.tx_stats.packets[ac];
>>                   sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_PACKETS);
>>           } "
>>
> 
> Indeed, weird, why do we even bother counting them per AC?
> 

Currently, tx_packets and tx_bytes are maintained per AC only in mac80211.
I'm not entirely sure why this per-AC tracking was introduced, as I 
haven't come across a specific use case that justifies maintaining these 
metrics separately per AC.

I also checked to see if there are any other parameters tracking 
tx_packets and tx_bytes, but couldn't find any.

Would you like me to consolidate tx_packets and tx_bytes instead of 
maintaining them per AC?
Or should we consider this change separately as different patch?

> johannes

Re: [PATCH wireless-next v8 06/10] wifi: mac80211: add support to accumulate removed link statistics

[Johannes Berg]

On Thu, 2025-05-22 at 10:13 +0530, Sarika Sharma wrote:
> 
> > Indeed, weird, why do we even bother counting them per AC?
> > 
> 
> Currently, tx_packets and tx_bytes are maintained per AC only in mac80211.
> I'm not entirely sure why this per-AC tracking was introduced, as I 
> haven't come across a specific use case that justifies maintaining these 
> metrics separately per AC.
> 
> I also checked to see if there are any other parameters tracking 
> tx_packets and tx_bytes, but couldn't find any.
> 
> Would you like me to consolidate tx_packets and tx_bytes instead of 
> maintaining them per AC?
> Or should we consider this change separately as different patch?

Let's say that's different and unrelated, maybe we had something
accessing it in the past.

johannes

Re: [PATCH wireless-next v8 06/10] wifi: mac80211: add support to accumulate removed link statistics

[Sarika Sharma]

On 5/22/2025 10:33 AM, Johannes Berg wrote:
> On Thu, 2025-05-22 at 10:13 +0530, Sarika Sharma wrote:
>>
>>> Indeed, weird, why do we even bother counting them per AC?
>>>
>>
>> Currently, tx_packets and tx_bytes are maintained per AC only in mac80211.
>> I'm not entirely sure why this per-AC tracking was introduced, as I
>> haven't come across a specific use case that justifies maintaining these
>> metrics separately per AC.
>>
>> I also checked to see if there are any other parameters tracking
>> tx_packets and tx_bytes, but couldn't find any.
>>
>> Would you like me to consolidate tx_packets and tx_bytes instead of
>> maintaining them per AC?
>> Or should we consider this change separately as different patch?
> 
> Let's say that's different and unrelated, maybe we had something
> accessing it in the past.
> 

Yes, there might be chances it was used before.

Shall I do this cleanup before this series something like cleanup to 
maintain consolidate data for tx_packets and tx_bytes as nowhere it is 
used per AC?

> johannes

Re: [PATCH wireless-next v8 06/10] wifi: mac80211: add support to accumulate removed link statistics

[Johannes Berg]

On Thu, 2025-05-22 at 11:20 +0530, Sarika Sharma wrote:
> 
> 
> Yes, there might be chances it was used before.
> 
> Shall I do this cleanup before this series something like cleanup to 
> maintain consolidate data for tx_packets and tx_bytes as nowhere it is 
> used per AC?

Let's just leave it for now, this is already enough in a series.

johannes

Re: [PATCH wireless-next v8 06/10] wifi: mac80211: add support to accumulate removed link statistics

[Sarika Sharma]

On 5/22/2025 11:21 AM, Johannes Berg wrote:
> On Thu, 2025-05-22 at 11:20 +0530, Sarika Sharma wrote:
>>
>>
>> Yes, there might be chances it was used before.
>>
>> Shall I do this cleanup before this series something like cleanup to
>> maintain consolidate data for tx_packets and tx_bytes as nowhere it is
>> used per AC?
> 
> Let's just leave it for now, this is already enough in a series.

Sure, will take up at later point.
Thankyou!

> 
> johannes