2012-07-03 08:37:45

by Vladimir Kondratiev

[permalink] [raw]
Subject: [PATCH 0/2] High bitrate support

Proposed patches solve issue with high bitrates not fitting into u16.

I split it into 2 patches: one introduces new u32 bitrate,
and 2-nd adds 60g specific calculations.

Idea is to report (in nl80211) both old and new values,
or only new if it not fits into u16.
User space tools will migrate to the new bitrate, then old one
may be removed from kernel.

There are few places where cfg80211_calculate_bitrate() used
besides nl80211.c; but these places are OK with bitrate changing
from u16 to u32.

Vladimir Kondratiev (2):
{nl,cfg}80211: support high bitrates
cfg80211: bitrate calculation for 60g

include/linux/nl80211.h | 5 +++++
include/net/cfg80211.h | 4 +++-
net/wireless/nl80211.c | 8 ++++++--
net/wireless/util.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 64 insertions(+), 4 deletions(-)

--
1.7.9.5



2012-07-03 11:55:24

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 0/2] High bitrate support

On Tue, 2012-07-03 at 11:37 +0300, Vladimir Kondratiev wrote:
> Proposed patches solve issue with high bitrates not fitting into u16.
>
> I split it into 2 patches: one introduces new u32 bitrate,
> and 2-nd adds 60g specific calculations.
>
> Idea is to report (in nl80211) both old and new values,
> or only new if it not fits into u16.
> User space tools will migrate to the new bitrate, then old one
> may be removed from kernel.
>
> There are few places where cfg80211_calculate_bitrate() used
> besides nl80211.c; but these places are OK with bitrate changing
> from u16 to u32.
>
> Vladimir Kondratiev (2):
> {nl,cfg}80211: support high bitrates
> cfg80211: bitrate calculation for 60g

I've applied this (but not yet pushed out in case somebody objects
still).

I've fixed it for you, but please break your commit logs at (at most) 72
characters.

johannes


2012-07-03 08:37:49

by Vladimir Kondratiev

[permalink] [raw]
Subject: [PATCH 1/2] {nl,cfg}80211: support high bitrates

With recently introduced high bitrates, u16 that used to represent bitrate
becomes too small.

Thus, new 32-bit bitrate introduced. nl80211 reports new bitrate and old one,
in case bitrate fits into u16.

User space tools should switch to new bitrate. Old one is deprecated and going to be
removed in the future.

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
include/linux/nl80211.h | 5 +++++
include/net/cfg80211.h | 2 +-
net/wireless/nl80211.c | 8 ++++++--
net/wireless/util.c | 2 +-
4 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 74cc55c..4ba32e3 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -1638,12 +1638,16 @@ struct nl80211_sta_flag_update {
*
* These attribute types are used with %NL80211_STA_INFO_TXRATE
* when getting information about the bitrate of a station.
+ * Legacy bitrate represented with 16-bit value, while new
+ * "high throughput" bitrate uses 32-bit value. User space tools
+ * should use new bitrate.
*
* @__NL80211_RATE_INFO_INVALID: attribute number 0 is reserved
* @NL80211_RATE_INFO_BITRATE: total bitrate (u16, 100kbit/s)
* @NL80211_RATE_INFO_MCS: mcs index for 802.11n (u8)
* @NL80211_RATE_INFO_40_MHZ_WIDTH: 40 Mhz dualchannel bitrate
* @NL80211_RATE_INFO_SHORT_GI: 400ns guard interval
+ * @NL80211_RATE_INFO_BITRATE_HT: total bitrate (u32, 100kbit/s)
* @NL80211_RATE_INFO_MAX: highest rate_info number currently defined
* @__NL80211_RATE_INFO_AFTER_LAST: internal use
*/
@@ -1653,6 +1657,7 @@ enum nl80211_rate_info {
NL80211_RATE_INFO_MCS,
NL80211_RATE_INFO_40_MHZ_WIDTH,
NL80211_RATE_INFO_SHORT_GI,
+ NL80211_RATE_INFO_BITRATE_HT,

/* keep last */
__NL80211_RATE_INFO_AFTER_LAST,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 0b564e8..8837efc 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -3487,7 +3487,7 @@ void cfg80211_ch_switch_notify(struct net_device *dev, int freq,
*
* return 0 if MCS index >= 32
*/
-u16 cfg80211_calculate_bitrate(struct rate_info *rate);
+u32 cfg80211_calculate_bitrate(struct rate_info *rate);

/* Logging, debugging and troubleshooting/diagnostic helpers. */

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 77102e6..3e370d9 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2618,7 +2618,8 @@ static bool nl80211_put_sta_rate(struct sk_buff *msg, struct rate_info *info,
int attr)
{
struct nlattr *rate;
- u16 bitrate;
+ u32 bitrate;
+ u16 bitrate_compat;

rate = nla_nest_start(msg, attr);
if (!rate)
@@ -2626,8 +2627,11 @@ static bool nl80211_put_sta_rate(struct sk_buff *msg, struct rate_info *info,

/* cfg80211_calculate_bitrate will return 0 for mcs >= 32 */
bitrate = cfg80211_calculate_bitrate(info);
+ bitrate_compat = bitrate < (1UL << 16) ? bitrate : 0;
if ((bitrate > 0 &&
- nla_put_u16(msg, NL80211_RATE_INFO_BITRATE, bitrate)) ||
+ nla_put_u32(msg, NL80211_RATE_INFO_BITRATE_HT, bitrate)) ||
+ (bitrate_compat > 0 &&
+ nla_put_u16(msg, NL80211_RATE_INFO_BITRATE, bitrate_compat)) ||
((info->flags & RATE_INFO_FLAGS_MCS) &&
nla_put_u8(msg, NL80211_RATE_INFO_MCS, info->mcs)) ||
((info->flags & RATE_INFO_FLAGS_40_MHZ_WIDTH) &&
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 0228c64..6e52726 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -900,7 +900,7 @@ int cfg80211_change_iface(struct cfg80211_registered_device *rdev,
return err;
}

-u16 cfg80211_calculate_bitrate(struct rate_info *rate)
+u32 cfg80211_calculate_bitrate(struct rate_info *rate)
{
int modulation, streams, bitrate;

--
1.7.9.5


2012-07-04 05:41:15

by Vladimir Kondratiev

[permalink] [raw]
Subject: Re: [PATCH 1/2] {nl,cfg}80211: support high bitrates

On Tuesday, July 03, 2012 09:04:38 AM Luis R. Rodriguez wrote:
> On Tue, Jul 3, 2012 at 1:37 AM, Vladimir Kondratiev
>
> <[email protected]> wrote:
> > User space tools should switch to new bitrate. Old one is deprecated and
> > going to be removed in the future.
>
> Removed?? No way. You can't make old userspace break.
>
> > @@ -1638,12 +1638,16 @@ struct nl80211_sta_flag_update {
> >
> > *
> > * These attribute types are used with %NL80211_STA_INFO_TXRATE
> > * when getting information about the bitrate of a station.
> >
> > + * Legacy bitrate represented with 16-bit value, while new
> > + * "high throughput" bitrate uses 32-bit value. User space tools
> > + * should use new bitrate.
>
> Something a bit more clearer would say that we send both, and that
> userspace should use the new one given that new technologies required
> a higher data type.

Agree - I'll rephrase

> Also explaining what gets sent to userspace for
> those high data bit rates on the old data type would help -- from what
> I read in your patch we set those high bit rates to 0 on the old data
> type.

Not exactly. If value is 0, it is just not send:
(bitrate_compat > 0 &&
nla_put_u16(msg, NL80211_RATE_INFO_BITRATE, bitrate_compat)

>
> > *
> > * @__NL80211_RATE_INFO_INVALID: attribute number 0 is reserved
> > * @NL80211_RATE_INFO_BITRATE: total bitrate (u16, 100kbit/s)
> > * @NL80211_RATE_INFO_MCS: mcs index for 802.11n (u8)
> > * @NL80211_RATE_INFO_40_MHZ_WIDTH: 40 Mhz dualchannel bitrate
> > * @NL80211_RATE_INFO_SHORT_GI: 400ns guard interval
> >
> > + * @NL80211_RATE_INFO_BITRATE_HT: total bitrate (u32, 100kbit/s)
>
> NL80211_RATE_INFO_BITRATE_HT seems misleading as likely we will also
> use this for VHT, and whatever other fun acronym the industry comes up
> with for bitrates for 802.11.

In case "HT" for "High Throuput" is misleading, I am open to any
other proposals. What name can you suggest?

Thanks, Vladimir

2012-07-03 08:37:52

by Vladimir Kondratiev

[permalink] [raw]
Subject: [PATCH 2/2] cfg80211: bitrate calculation for 60g

60g band uses different from .11n MCS scheme, so bitrate should be calculated
differently

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
include/net/cfg80211.h | 2 ++
net/wireless/util.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 51 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 8837efc..51f67a9 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -580,11 +580,13 @@ enum station_info_flags {
* @RATE_INFO_FLAGS_MCS: @tx_bitrate_mcs filled
* @RATE_INFO_FLAGS_40_MHZ_WIDTH: 40 Mhz width transmission
* @RATE_INFO_FLAGS_SHORT_GI: 400ns guard interval
+ * @RATE_INFO_FLAGS_60G: 60gHz MCS
*/
enum rate_info_flags {
RATE_INFO_FLAGS_MCS = 1<<0,
RATE_INFO_FLAGS_40_MHZ_WIDTH = 1<<1,
RATE_INFO_FLAGS_SHORT_GI = 1<<2,
+ RATE_INFO_FLAGS_60G = 1<<3,
};

/**
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 6e52726..e31f1db 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -900,12 +900,61 @@ int cfg80211_change_iface(struct cfg80211_registered_device *rdev,
return err;
}

+static u32 cfg80211_calculate_bitrate_60g(struct rate_info *rate)
+{
+ static const u32 __mcs2bitrate[] = {
+ /* control PHY */
+ [0] = 275,
+ /* SC PHY */
+ [1] = 3850,
+ [2] = 7700,
+ [3] = 9625,
+ [4] = 11550,
+ [5] = 12512, /* 1251.25 mbps */
+ [6] = 15400,
+ [7] = 19250,
+ [8] = 23100,
+ [9] = 25025,
+ [10] = 30800,
+ [11] = 38500,
+ [12] = 46200,
+ /* OFDM PHY */
+ [13] = 6930,
+ [14] = 8662, /* 866.25 mbps */
+ [15] = 13860,
+ [16] = 17325,
+ [17] = 20790,
+ [18] = 27720,
+ [19] = 34650,
+ [20] = 41580,
+ [21] = 45045,
+ [22] = 51975,
+ [23] = 62370,
+ [24] = 67568, /* 6756.75 mbps */
+ /* LP-SC PHY */
+ [25] = 6260,
+ [26] = 8340,
+ [27] = 11120,
+ [28] = 12510,
+ [29] = 16680,
+ [30] = 22240,
+ [31] = 25030,
+ };
+
+ if (WARN_ON_ONCE(rate->mcs >= ARRAY_SIZE(__mcs2bitrate)))
+ return 0;
+
+ return __mcs2bitrate[rate->mcs];
+}
+
u32 cfg80211_calculate_bitrate(struct rate_info *rate)
{
int modulation, streams, bitrate;

if (!(rate->flags & RATE_INFO_FLAGS_MCS))
return rate->legacy;
+ if (rate->flags & RATE_INFO_FLAGS_60G)
+ return cfg80211_calculate_bitrate_60g(rate);

/* the formula below does only work for MCS values smaller than 32 */
if (WARN_ON_ONCE(rate->mcs >= 32))
--
1.7.9.5


2012-07-03 18:02:12

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 0/2] High bitrate support

On Tue, Jul 3, 2012 at 4:55 AM, Johannes Berg <[email protected]> wrote:
> On Tue, 2012-07-03 at 11:37 +0300, Vladimir Kondratiev wrote:
>> Proposed patches solve issue with high bitrates not fitting into u16.
>>
>> I split it into 2 patches: one introduces new u32 bitrate,
>> and 2-nd adds 60g specific calculations.
>>
>> Idea is to report (in nl80211) both old and new values,
>> or only new if it not fits into u16.
>> User space tools will migrate to the new bitrate, then old one
>> may be removed from kernel.
>>
>> There are few places where cfg80211_calculate_bitrate() used
>> besides nl80211.c; but these places are OK with bitrate changing
>> from u16 to u32.
>>
>> Vladimir Kondratiev (2):
>> {nl,cfg}80211: support high bitrates
>> cfg80211: bitrate calculation for 60g
>
> I've applied this (but not yet pushed out in case somebody objects
> still).

In case it is missed, I did point out a few things with the first patch.

Luis

2012-07-03 16:05:00

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 1/2] {nl,cfg}80211: support high bitrates

On Tue, Jul 3, 2012 at 1:37 AM, Vladimir Kondratiev
<[email protected]> wrote:
> User space tools should switch to new bitrate. Old one is deprecated and going to be
> removed in the future.

Removed?? No way. You can't make old userspace break.

> @@ -1638,12 +1638,16 @@ struct nl80211_sta_flag_update {
> *
> * These attribute types are used with %NL80211_STA_INFO_TXRATE
> * when getting information about the bitrate of a station.
> + * Legacy bitrate represented with 16-bit value, while new
> + * "high throughput" bitrate uses 32-bit value. User space tools
> + * should use new bitrate.

Something a bit more clearer would say that we send both, and that
userspace should use the new one given that new technologies required
a higher data type. Also explaining what gets sent to userspace for
those high data bit rates on the old data type would help -- from what
I read in your patch we set those high bit rates to 0 on the old data
type.

> *
> * @__NL80211_RATE_INFO_INVALID: attribute number 0 is reserved
> * @NL80211_RATE_INFO_BITRATE: total bitrate (u16, 100kbit/s)
> * @NL80211_RATE_INFO_MCS: mcs index for 802.11n (u8)
> * @NL80211_RATE_INFO_40_MHZ_WIDTH: 40 Mhz dualchannel bitrate
> * @NL80211_RATE_INFO_SHORT_GI: 400ns guard interval
> + * @NL80211_RATE_INFO_BITRATE_HT: total bitrate (u32, 100kbit/s)

NL80211_RATE_INFO_BITRATE_HT seems misleading as likely we will also
use this for VHT, and whatever other fun acronym the industry comes up
with for bitrates for 802.11.

Luis