[RFCv2,1/6] cfg80211: export expected throughput through get_station()

Message ID 1396211704-4677-2-git-send-email-antonio@meshcoding.com (mailing list archive)
State Not Applicable, archived
Headers

Commit Message

Antonio Quartulli March 30, 2014, 8:34 p.m. UTC
  From: Antonio Quartulli <antonio@open-mesh.com>

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 <antonio@open-mesh.com>
---
 include/net/cfg80211.h | 63 ++++++++++++++++++++++++++++----------------------
 1 file changed, 35 insertions(+), 28 deletions(-)
  

Comments

Andrew Lunn March 31, 2014, 8:22 a.m. UTC | #1
On Sun, Mar 30, 2014 at 10:34:59PM +0200, Antonio Quartulli wrote:
> From: Antonio Quartulli <antonio@open-mesh.com>
> 
> 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 <antonio@open-mesh.com>
> ---
>  include/net/cfg80211.h | 63 ++++++++++++++++++++++++++++----------------------
>  1 file changed, 35 insertions(+), 28 deletions(-)
> 
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index fb8afce..4c8ebe9 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -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,
> +	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_EXPECTED_THROUGHPUT	= 1<<28,
>  };


Hi Antonio

>  /**
> @@ -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;
>  
> +	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.

       Andrew
  
Antonio Quartulli March 31, 2014, 8:46 a.m. UTC | #2
Hi Andrew,

On 31/03/14 10:22, Andrew Lunn wrote:
>>  /**
>> @@ -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;
>>  
>> +	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.
> 

Thanks for raising the point.

Actually this is something we could discuss with Felix: at the moment
this value uses the same unit used by the cur_tp member of the
minstrel_ht_sta structure.

It should be Mbps/100 --> expected_throughput = 1 ==> 0.01Mbps

When exposing this value to the user (i.e. iw output) I'd recommend to
convert it to Mbps, but for internal purposes I don't see a clear
problem in exporting a value in this form.

Thoughts? Suggestions?

Cheers,
  
Johannes Berg April 8, 2014, 10:03 a.m. UTC | #3
On Sun, 2014-03-30 at 22:34 +0200, Antonio Quartulli wrote:

> -	STATION_INFO_INACTIVE_TIME	= 1<<0,

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


> @@ -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 ... :)

johannes
  
Antonio Quartulli April 10, 2014, 4:22 p.m. UTC | #4
On 08/04/14 12:03, Johannes Berg wrote:
> On Sun, 2014-03-30 at 22:34 +0200, Antonio Quartulli wrote:
> 
>> -	STATION_INFO_INACTIVE_TIME	= 1<<0,
> 
> Since you're touching these all anyway, maybe convert to use BIT().

Will do!

> 
> 
>> @@ -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 ... :)

Yep. As suggested by Andrew, I will try to be clear about what unit is
returned.

Thanks a lot for your review!!!!

Cheers,
  

Patch

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index fb8afce..4c8ebe9 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -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,
+	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_EXPECTED_THROUGHPUT	= 1<<28,
 };
 
 /**
@@ -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;
 
+	u32 expected_throughput;
+
 	/*
 	 * Note: Add a new enum station_info_flags value for each new field and
 	 * use it to check which fields are initialized.