Export the expected throughput

From: Antonio Quartulli <[email protected]>

Users may need information about the expected throughput towards a given peer computed by the RC algorithm. Export such value through the get_station() API.

This information is useful to the batman-adv module which will use it for its new metric computation.

Signed-off-by Antonio Quartulli <[email protected]>

---
 include/net/cfg80211.h | 63 ++++++++++++++++++++++++++++----------------------
 1 file changed, 35 insertions(+), 28 deletions(-)

include/net/cfg80211.h +21 -28

index fb8afce..4c8ebe9 100644
@@ -862,36 +862,38 @@ int cfg80211_check_station_change(struct wiphy *wiphy,
  * @STATION_INFO_NONPEER_PM: @nonpeer_pm filled
  * @STATION_INFO_CHAIN_SIGNAL: @chain_signal filled
  * @STATION_INFO_CHAIN_SIGNAL_AVG: @chain_signal_avg filled
+ * @STATION_INFO_EXPECTED_THROUGHPUT: @expected_throughput filled
  */
 enum station_info_flags {
-	STATION_INFO_INACTIVE_TIME	= 1<<0,
-	STATION_INFO_RX_BYTES		= 1<<1,
-	STATION_INFO_TX_BYTES		= 1<<2,
-	STATION_INFO_LLID		= 1<<3,
-	STATION_INFO_PLID		= 1<<4,
-	STATION_INFO_PLINK_STATE	= 1<<5,
-	STATION_INFO_SIGNAL		= 1<<6,
-	STATION_INFO_TX_BITRATE		= 1<<7,
-	STATION_INFO_RX_PACKETS		= 1<<8,
-	STATION_INFO_TX_PACKETS		= 1<<9,
-	STATION_INFO_TX_RETRIES		= 1<<10,
-	STATION_INFO_TX_FAILED		= 1<<11,
-	STATION_INFO_RX_DROP_MISC	= 1<<12,
-	STATION_INFO_SIGNAL_AVG		= 1<<13,
-	STATION_INFO_RX_BITRATE		= 1<<14,
-	STATION_INFO_BSS_PARAM          = 1<<15,
-	STATION_INFO_CONNECTED_TIME	= 1<<16,
-	STATION_INFO_ASSOC_REQ_IES	= 1<<17,
-	STATION_INFO_STA_FLAGS		= 1<<18,
-	STATION_INFO_BEACON_LOSS_COUNT	= 1<<19,
-	STATION_INFO_T_OFFSET		= 1<<20,
-	STATION_INFO_LOCAL_PM		= 1<<21,
-	STATION_INFO_PEER_PM		= 1<<22,
-	STATION_INFO_NONPEER_PM		= 1<<23,
-	STATION_INFO_RX_BYTES64		= 1<<24,
-	STATION_INFO_TX_BYTES64		= 1<<25,
-	STATION_INFO_CHAIN_SIGNAL	= 1<<26,
-	STATION_INFO_CHAIN_SIGNAL_AVG	= 1<<27,
+	STATION_INFO_INACTIVE_TIME		= 1<<0,
+	STATION_INFO_RX_BYTES			= 1<<1,
+	STATION_INFO_TX_BYTES			= 1<<2,
+	STATION_INFO_LLID			= 1<<3,
+	STATION_INFO_PLID			= 1<<4,
+	STATION_INFO_PLINK_STATE		= 1<<5,
+	STATION_INFO_SIGNAL			= 1<<6,
+	STATION_INFO_TX_BITRATE			= 1<<7,
+	STATION_INFO_RX_PACKETS			= 1<<8,
+	STATION_INFO_TX_PACKETS			= 1<<9,
+	STATION_INFO_TX_RETRIES			= 1<<10,
+	STATION_INFO_TX_FAILED			= 1<<11,
+	STATION_INFO_RX_DROP_MISC		= 1<<12,
+	STATION_INFO_SIGNAL_AVG			= 1<<13,
+	STATION_INFO_RX_BITRATE			= 1<<14,
+	STATION_INFO_BSS_PARAM			= 1<<15,
+	STATION_INFO_CONNECTED_TIME		= 1<<16,
+	STATION_INFO_ASSOC_REQ_IES		= 1<<17,
+	STATION_INFO_STA_FLAGS			= 1<<18,
+	STATION_INFO_BEACON_LOSS_COUNT		= 1<<19,

include/net/cfg80211.h +3 -0

 /**

@@ -1013,6 +1015,9 @@ struct sta_bss_parameters {
  * @local_pm: local mesh STA power save mode
  * @peer_pm: peer mesh STA power save mode
  * @nonpeer_pm: non-peer mesh STA power save mode
+ * @expected_throughput: expected throughput as reported by the RC algorithm
+ *  about the bitrate having the maximum throughput. This field can be filled
+ *  only by drivers using Minstrel
  */
 struct station_info {
 	u32 filled;

Quoted 15 lines

/**

include/net/cfg80211.h +3 -0

@@ -1013,6 +1015,9 @@ struct sta_bss_parameters {
  * @local_pm: local mesh STA power save mode
  * @peer_pm: peer mesh STA power save mode
  * @nonpeer_pm: non-peer mesh STA power save mode
+ * @expected_throughput: expected throughput as reported by the RC algorithm
+ *  about the bitrate having the maximum throughput. This field can be filled
+ *  only by drivers using Minstrel
  */
 struct station_info {
 	u32 filled;

@@ -1051,6 +1056,8 @@ struct station_info {
 	enum nl80211_mesh_power_mode peer_pm;
 	enum nl80211_mesh_power_mode nonpeer_pm;
 

include/net/cfg80211.h struct station_info { · line 1060

Quoted 2 lines

include/net/cfg80211.h +2 -0

+	u32 expected_throughput;
+

It would be nice to comment on what the units are. I known from our BATMAN V discussions, it is something odd.

From: Antonio Quartulli <[email protected]>

In order to make get_station() export the expected throughput, we need a new API which extracts such information from the Rate Control algorithm. Therefore add the get_expected_throughput() member to the rate_control_ops structure.

Quoted 7 lines · Antonio Quartulli

From: Antonio Quartulli <[email protected]>

In order to make get_station() export the expected throughput, we need a new API which extracts such information from the Rate Control algorithm. Therefore add the get_expected_throughput() member to the rate_control_ops structure.

What about drivers with HW rate control?

include/net/mac80211.h struct rate_control_ops { · line 4481

Quoted 2 lines · Antonio Quartulli

include/net/mac80211.h +2 -0

+	u32 (*get_expected_throughput)(void *priv, void *priv_sta,
+				       struct ieee80211_supported_band *sband);

why would that need an sband argument?

net/mac80211/rc80211_minstrel.c minstrel_free(void *priv) · line 671

Quoted 12 lines · Antonio Quartulli

net/mac80211/rc80211_minstrel.c +12 -0

+static u32
+minstrel_get_expected_throughput(void *priv, void *priv_sta,
+				 struct ieee80211_supported_band *sband)
+{
+	struct minstrel_sta_info *mi = priv_sta;
+	int idx = mi->max_tp_rate[0];
+	u32 bitrate, ret;
+
+	bitrate = sband->bitrates[mi->r[idx].rix].bitrate;
+	ret = bitrate * MINSTREL_TRUNC(mi->r[idx].probability * 1000) / 1000;
+
+	return ret;

You don't need a ret variable.

include/net/cfg80211.h struct station_info { · line 1076

Quoted 10 lines · Antonio Quartulli

/**
+ * cfg80211_get_station - retrieve information about a given station
+ * @dev: the device where the station is supposed to be connected to
+ * @mac_addr: the mac address of the station of interest
+ * @sinfo: pointer to the structure to fill with the information
+ *
+ * Returns 0 on success or a negative error code otherwise.
+ */

include/net/cfg80211.h +2 -0

+int cfg80211_get_station(struct net_device *dev, u8 *mac_addr,
+			 struct station_info *sinfo);

mac_addr should be const

Any thoughts about clearing/filling/partially filling *sinfo when returning an error?

include/net/cfg80211.h int cfg80211_check_station_change(struct wiphy *wiphy, · line 867

Quoted 1 line · Antonio Quartulli

include/net/cfg80211.h +0 -1

-	STATION_INFO_INACTIVE_TIME	= 1<<0,

Since you're touching these all anyway, maybe convert to use BIT().

include/net/cfg80211.h struct sta_bss_parameters { · line 1020

Quoted 9 lines

include/net/cfg80211.h +3 -0

@@ -1013,6 +1015,9 @@ struct sta_bss_parameters {
  * @local_pm: local mesh STA power save mode
  * @peer_pm: peer mesh STA power save mode
  * @nonpeer_pm: non-peer mesh STA power save mode
+ * @expected_throughput: expected throughput as reported by the RC algorithm
+ *  about the bitrate having the maximum throughput. This field can be filled
+ *  only by drivers using Minstrel

This value needs a documented unit ... :)

Quoted 2 lines

Any thoughts about clearing/filling/partially filling *sinfo when returning an error?

At the moment this function relies on what rdev_get_station() does and I always assumed that in case of error the content of *sinfo should be considered "undefined".

Quoted 2 lines

What about drivers with HW rate control?

True, they do not implement the rate_control_ops API.

Therefore, as discussed on IRC, I need to move this new API at the mac80211 level, so that each driver can implement it the way it prefer.

Quoted 7 lines · Johannes Berg

include/net/mac80211.h struct rate_control_ops { · line 4481

Quoted 2 lines · Antonio Quartulli

include/net/mac80211.h +2 -0

+	u32 (*get_expected_throughput)(void *priv, void *priv_sta,
+				       struct ieee80211_supported_band *sband);

why would that need an sband argument?

I needed this because I saw it is required by some RC algo (only minstrel) to extract the bitrate starting from the index.

You can check patch 4/6 for this:

(quoted diff) +1 -0

+	bitrate = sband->bitrates[mi->r[idx].rix].bitrate;

maybe I can extract it from another structure? I couldn't find any way..

Quoted 3 lines · Antonio Quartulli

Quoted 2 lines

What about drivers with HW rate control?

Quoted 4 lines

True, they do not implement the rate_control_ops API.

Therefore, as discussed on IRC, I need to move this new API at the mac80211 level, so that each driver can implement it the way it prefer.

Well, not *move*, but rather have both. Or you can just say that the drivers that may need this can worry about it ...

Quoted 6 lines · Antonio Quartulli

Quoted 2 lines

Any thoughts about clearing/filling/partially filling *sinfo when returning an error?

At the moment this function relies on what rdev_get_station() does and I always assumed that in case of error the content of *sinfo should be considered "undefined".

That's fine, maybe add a line of documentation?

Quoted 13 lines · Antonio Quartulli

Quoted 7 lines · Johannes Berg

include/net/mac80211.h struct rate_control_ops { · line 4481

Quoted 2 lines · Antonio Quartulli

include/net/mac80211.h +2 -0

+	u32 (*get_expected_throughput)(void *priv, void *priv_sta,
+				       struct ieee80211_supported_band *sband);

why would that need an sband argument?

I needed this because I saw it is required by some RC algo (only minstrel) to extract the bitrate starting from the index.

Quoted 5 lines

You can check patch 4/6 for this:

(quoted diff) +1 -0

+	bitrate = sband->bitrates[mi->r[idx].rix].bitrate;

maybe I can extract it from another structure? I couldn't find any way..

Bit strange that the algorithm doesn't have the band, if it has an index? but I guess I didn't check all the patches...