2016-06-27 09:53:39

by Krishna Chaitanya

[permalink] [raw]
Subject: [PATCH] mac80211: minstrel: Enable STBC and LDPC for VHT Rates

From: Chaitanya T K <[email protected]>

If peer support reception of STBC and LDPC, enable them for better
performance.

Signed-off-by: Chaitanya TK <[email protected]>
---
include/linux/ieee80211.h | 1 +
net/mac80211/rc80211_minstrel_ht.c | 25 +++++++++++++++++--------
2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index b118744..4d01130 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1550,6 +1550,7 @@ struct ieee80211_vht_operation {
#define IEEE80211_VHT_CAP_RXSTBC_3 0x00000300
#define IEEE80211_VHT_CAP_RXSTBC_4 0x00000400
#define IEEE80211_VHT_CAP_RXSTBC_MASK 0x00000700
+#define IEEE80211_VHT_CAP_RXSTBC_SHIFT 8
#define IEEE80211_VHT_CAP_SU_BEAMFORMER_CAPABLE 0x00000800
#define IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE 0x00001000
#define IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT 13
diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index 30fbabf..e2fcdea 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -1166,13 +1166,14 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
struct minstrel_ht_sta_priv *msp = priv_sta;
struct minstrel_ht_sta *mi = &msp->ht;
struct ieee80211_mcs_info *mcs = &sta->ht_cap.mcs;
- u16 sta_cap = sta->ht_cap.cap;
+ u16 ht_cap = sta->ht_cap.cap;
struct ieee80211_sta_vht_cap *vht_cap = &sta->vht_cap;
int use_vht;
int n_supported = 0;
int ack_dur;
int stbc;
int i;
+ bool ldpc;

/* fall back to the old minstrel for legacy stations */
if (!sta->ht_cap.ht_supported)
@@ -1210,16 +1211,24 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
}
mi->sample_tries = 4;

- /* TODO tx_flags for vht - ATM the RC API is not fine-grained enough */
if (!use_vht) {
- stbc = (sta_cap & IEEE80211_HT_CAP_RX_STBC) >>
+ stbc = (ht_cap & IEEE80211_HT_CAP_RX_STBC) >>
IEEE80211_HT_CAP_RX_STBC_SHIFT;
- mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;

- if (sta_cap & IEEE80211_HT_CAP_LDPC_CODING)
- mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
+ if (ht_cap & IEEE80211_HT_CAP_LDPC_CODING)
+ ldpc = true;
+ } else {
+ stbc = (vht_cap->cap & IEEE80211_VHT_CAP_RXSTBC_MASK) >>
+ IEEE80211_VHT_CAP_RXSTBC_SHIFT;
+
+ if (vht_cap->cap & IEEE80211_VHT_CAP_RXLDPC)
+ ldpc = true;
}

+ mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;
+ if (ldpc)
+ mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
+
for (i = 0; i < ARRAY_SIZE(mi->groups); i++) {
u32 gflags = minstrel_mcs_groups[i].flags;
int bw, nss;
@@ -1232,10 +1241,10 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,

if (gflags & IEEE80211_TX_RC_SHORT_GI) {
if (gflags & IEEE80211_TX_RC_40_MHZ_WIDTH) {
- if (!(sta_cap & IEEE80211_HT_CAP_SGI_40))
+ if (!(ht_cap & IEEE80211_HT_CAP_SGI_40))
continue;
} else {
- if (!(sta_cap & IEEE80211_HT_CAP_SGI_20))
+ if (!(ht_cap & IEEE80211_HT_CAP_SGI_20))
continue;
}
}
--
1.7.9.5



2016-06-29 15:56:54

by Krishna Chaitanya

[permalink] [raw]
Subject: Re: [PATCH] mac80211: minstrel: Enable STBC and LDPC for VHT Rates

On Wed, Jun 29, 2016 at 1:50 PM, Krishna Chaitanya
<[email protected]> wrote:
> On Wed, Jun 29, 2016 at 1:41 PM, Karl Beldan <[email protected]> wrote:
>> On Mon, Jun 27, 2016 at 9:53 AM, Chaitanya TK <[email protected]>
>> wrote:
>>>
>>> From: Chaitanya T K <[email protected]>
>>>
>>> If peer support reception of STBC and LDPC, enable them for better
>>> performance.
>>>
>>> Signed-off-by: Chaitanya TK <[email protected]>
>>> ---
>>> include/linux/ieee80211.h | 1 +
>>> net/mac80211/rc80211_minstrel_ht.c | 25 +++++++++++++++++--------
>>> 2 files changed, 18 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
>>> index b118744..4d01130 100644
>>> --- a/include/linux/ieee80211.h
>>> +++ b/include/linux/ieee80211.h
>>> @@ -1550,6 +1550,7 @@ struct ieee80211_vht_operation {
>>> #define IEEE80211_VHT_CAP_RXSTBC_3 0x00000300
>>> #define IEEE80211_VHT_CAP_RXSTBC_4 0x00000400
>>> #define IEEE80211_VHT_CAP_RXSTBC_MASK 0x00000700
>>> +#define IEEE80211_VHT_CAP_RXSTBC_SHIFT 8
>>> #define IEEE80211_VHT_CAP_SU_BEAMFORMER_CAPABLE
>>> 0x00000800
>>> #define IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE
>>> 0x00001000
>>> #define IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT 13
>>> diff --git a/net/mac80211/rc80211_minstrel_ht.c
>>> b/net/mac80211/rc80211_minstrel_ht.c
>>> index 30fbabf..e2fcdea 100644
>>> --- a/net/mac80211/rc80211_minstrel_ht.c
>>> +++ b/net/mac80211/rc80211_minstrel_ht.c
>>> @@ -1166,13 +1166,14 @@ minstrel_ht_update_caps(void *priv, struct
>>> ieee80211_supported_band *sband,
>>> struct minstrel_ht_sta_priv *msp = priv_sta;
>>> struct minstrel_ht_sta *mi = &msp->ht;
>>> struct ieee80211_mcs_info *mcs = &sta->ht_cap.mcs;
>>> - u16 sta_cap = sta->ht_cap.cap;
>>> + u16 ht_cap = sta->ht_cap.cap;
>>> struct ieee80211_sta_vht_cap *vht_cap = &sta->vht_cap;
>>> int use_vht;
>>> int n_supported = 0;
>>> int ack_dur;
>>> int stbc;
>>> int i;
>>> + bool ldpc;
>>>
>>> /* fall back to the old minstrel for legacy stations */
>>> if (!sta->ht_cap.ht_supported)
>>> @@ -1210,16 +1211,24 @@ minstrel_ht_update_caps(void *priv, struct
>>> ieee80211_supported_band *sband,
>>> }
>>> mi->sample_tries = 4;
>>>
>>> - /* TODO tx_flags for vht - ATM the RC API is not fine-grained
>>> enough */
>>> if (!use_vht) {
>>> - stbc = (sta_cap & IEEE80211_HT_CAP_RX_STBC) >>
>>> + stbc = (ht_cap & IEEE80211_HT_CAP_RX_STBC) >>
>>> IEEE80211_HT_CAP_RX_STBC_SHIFT;
>>> - mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;
>>>
>>> - if (sta_cap & IEEE80211_HT_CAP_LDPC_CODING)
>>> - mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
>>> + if (ht_cap & IEEE80211_HT_CAP_LDPC_CODING)
>>> + ldpc = true;
>>> + } else {
>>> + stbc = (vht_cap->cap & IEEE80211_VHT_CAP_RXSTBC_MASK) >>
>>> + IEEE80211_VHT_CAP_RXSTBC_SHIFT;
>>> +
>>> + if (vht_cap->cap & IEEE80211_VHT_CAP_RXLDPC)
>>> + ldpc = true;
>>> }
>>>
>>> + mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;
>>> + if (ldpc)
>>> + mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
>>> +
>>
>>
>> Here you are using ldpc uninitialized.
>>
>>
>> As for the idea, ldpc and stbc support/enable in HT and VHT being
>> independant and tx_flags being shared by all the info.rates, you cannot use
>> it as you are trying to (e.g. you might end up asking lower layers to send
>> stbc/vht with a hw only supporting stbc/ht or/and trying to send stbc/vht
>> with inappropriate advertised vht caps).
> The design in minstrel is it checks only for peers capabilities and it doesn't
> check local hw capabilities (except for those in MCS_GROUPS), so minstrel
> just informs the we can use STBC/LDPC, its HW's decision whether to
> actually use or not.
>> This stems from the STBC/LDPC CTL flags being a mac80211_tx_info_flags, this
>> is what should be changed.
> I understand the problem here, if all rates are same then its no problem.
> But if some rates are HT and some are VHT, then we might end up in problem.
>
> To solve this, we need to move this to mac80211_rate_control_flags so that
> we can convey per rate info.
Karl/Johannes,

Any thoughts on this? This impacts all drivers, so it would be good
to finalize on design before submitting patches.

2016-06-29 08:56:36

by Krishna Chaitanya

[permalink] [raw]
Subject: Re: [PATCH] mac80211: minstrel: Enable STBC and LDPC for VHT Rates

On Wed, Jun 29, 2016 at 1:52 PM, Krishna Chaitanya
<[email protected]> wrote:
> On Wed, Jun 29, 2016 at 1:50 PM, Krishna Chaitanya
> <[email protected]> wrote:
>> On Wed, Jun 29, 2016 at 1:41 PM, Karl Beldan <[email protected]> wrote:
>>> On Mon, Jun 27, 2016 at 9:53 AM, Chaitanya TK <[email protected]>
>>> wrote:
>>>>
>>>> From: Chaitanya T K <[email protected]>
>>>>
>>>> If peer support reception of STBC and LDPC, enable them for better
>>>> performance.
>>>>
>>>> Signed-off-by: Chaitanya TK <[email protected]>
>>>> ---
>>>> include/linux/ieee80211.h | 1 +
>>>> net/mac80211/rc80211_minstrel_ht.c | 25 +++++++++++++++++--------
>>>> 2 files changed, 18 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
>>>> index b118744..4d01130 100644
>>>> --- a/include/linux/ieee80211.h
>>>> +++ b/include/linux/ieee80211.h
>>>> @@ -1550,6 +1550,7 @@ struct ieee80211_vht_operation {
>>>> #define IEEE80211_VHT_CAP_RXSTBC_3 0x00000300
>>>> #define IEEE80211_VHT_CAP_RXSTBC_4 0x00000400
>>>> #define IEEE80211_VHT_CAP_RXSTBC_MASK 0x00000700
>>>> +#define IEEE80211_VHT_CAP_RXSTBC_SHIFT 8
>>>> #define IEEE80211_VHT_CAP_SU_BEAMFORMER_CAPABLE
>>>> 0x00000800
>>>> #define IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE
>>>> 0x00001000
>>>> #define IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT 13
>>>> diff --git a/net/mac80211/rc80211_minstrel_ht.c
>>>> b/net/mac80211/rc80211_minstrel_ht.c
>>>> index 30fbabf..e2fcdea 100644
>>>> --- a/net/mac80211/rc80211_minstrel_ht.c
>>>> +++ b/net/mac80211/rc80211_minstrel_ht.c
>>>> @@ -1166,13 +1166,14 @@ minstrel_ht_update_caps(void *priv, struct
>>>> ieee80211_supported_band *sband,
>>>> struct minstrel_ht_sta_priv *msp = priv_sta;
>>>> struct minstrel_ht_sta *mi = &msp->ht;
>>>> struct ieee80211_mcs_info *mcs = &sta->ht_cap.mcs;
>>>> - u16 sta_cap = sta->ht_cap.cap;
>>>> + u16 ht_cap = sta->ht_cap.cap;
>>>> struct ieee80211_sta_vht_cap *vht_cap = &sta->vht_cap;
>>>> int use_vht;
>>>> int n_supported = 0;
>>>> int ack_dur;
>>>> int stbc;
>>>> int i;
>>>> + bool ldpc;
>>>>
>>>> /* fall back to the old minstrel for legacy stations */
>>>> if (!sta->ht_cap.ht_supported)
>>>> @@ -1210,16 +1211,24 @@ minstrel_ht_update_caps(void *priv, struct
>>>> ieee80211_supported_band *sband,
>>>> }
>>>> mi->sample_tries = 4;
>>>>
>>>> - /* TODO tx_flags for vht - ATM the RC API is not fine-grained
>>>> enough */
>>>> if (!use_vht) {
>>>> - stbc = (sta_cap & IEEE80211_HT_CAP_RX_STBC) >>
>>>> + stbc = (ht_cap & IEEE80211_HT_CAP_RX_STBC) >>
>>>> IEEE80211_HT_CAP_RX_STBC_SHIFT;
>>>> - mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;
>>>>
>>>> - if (sta_cap & IEEE80211_HT_CAP_LDPC_CODING)
>>>> - mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
>>>> + if (ht_cap & IEEE80211_HT_CAP_LDPC_CODING)
>>>> + ldpc = true;
>>>> + } else {
>>>> + stbc = (vht_cap->cap & IEEE80211_VHT_CAP_RXSTBC_MASK) >>
>>>> + IEEE80211_VHT_CAP_RXSTBC_SHIFT;
>>>> +
>>>> + if (vht_cap->cap & IEEE80211_VHT_CAP_RXLDPC)
>>>> + ldpc = true;
>>>> }
>>>>
>>>> + mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;
>>>> + if (ldpc)
>>>> + mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
>>>> +
>>>
>>>
>>> Here you are using ldpc uninitialized.
> yeah, will fix it.
Johannes,

How do you want to proceed? Do you want to
revert this patch and shall i send another patch?

2016-06-29 08:17:59

by Karl Beldan

[permalink] [raw]
Subject: Re: [PATCH] mac80211: minstrel: Enable STBC and LDPC for VHT Rates

On Mon, Jun 27, 2016 at 9:53 AM, Chaitanya TK <[email protected]> wrote:
> From: Chaitanya T K <[email protected]>
>
> If peer support reception of STBC and LDPC, enable them for better
> performance.
>
> Signed-off-by: Chaitanya TK <[email protected]>
> ---
> include/linux/ieee80211.h | 1 +
> net/mac80211/rc80211_minstrel_ht.c | 25 +++++++++++++++++--------
> 2 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
> index b118744..4d01130 100644
> --- a/include/linux/ieee80211.h
> +++ b/include/linux/ieee80211.h
> @@ -1550,6 +1550,7 @@ struct ieee80211_vht_operation {
> #define IEEE80211_VHT_CAP_RXSTBC_3 0x00000300
> #define IEEE80211_VHT_CAP_RXSTBC_4 0x00000400
> #define IEEE80211_VHT_CAP_RXSTBC_MASK 0x00000700
> +#define IEEE80211_VHT_CAP_RXSTBC_SHIFT 8
> #define IEEE80211_VHT_CAP_SU_BEAMFORMER_CAPABLE 0x00000800
> #define IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE 0x00001000
> #define IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT 13
> diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
> index 30fbabf..e2fcdea 100644
> --- a/net/mac80211/rc80211_minstrel_ht.c
> +++ b/net/mac80211/rc80211_minstrel_ht.c
> @@ -1166,13 +1166,14 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
> struct minstrel_ht_sta_priv *msp = priv_sta;
> struct minstrel_ht_sta *mi = &msp->ht;
> struct ieee80211_mcs_info *mcs = &sta->ht_cap.mcs;
> - u16 sta_cap = sta->ht_cap.cap;
> + u16 ht_cap = sta->ht_cap.cap;
> struct ieee80211_sta_vht_cap *vht_cap = &sta->vht_cap;
> int use_vht;
> int n_supported = 0;
> int ack_dur;
> int stbc;
> int i;
> + bool ldpc;
>
> /* fall back to the old minstrel for legacy stations */
> if (!sta->ht_cap.ht_supported)
> @@ -1210,16 +1211,24 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
> }
> mi->sample_tries = 4;
>
> - /* TODO tx_flags for vht - ATM the RC API is not fine-grained enough */
> if (!use_vht) {
> - stbc = (sta_cap & IEEE80211_HT_CAP_RX_STBC) >>
> + stbc = (ht_cap & IEEE80211_HT_CAP_RX_STBC) >>
> IEEE80211_HT_CAP_RX_STBC_SHIFT;
> - mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;
>
> - if (sta_cap & IEEE80211_HT_CAP_LDPC_CODING)
> - mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
> + if (ht_cap & IEEE80211_HT_CAP_LDPC_CODING)
> + ldpc = true;
> + } else {
> + stbc = (vht_cap->cap & IEEE80211_VHT_CAP_RXSTBC_MASK) >>
> + IEEE80211_VHT_CAP_RXSTBC_SHIFT;
> +
> + if (vht_cap->cap & IEEE80211_VHT_CAP_RXLDPC)
> + ldpc = true;
> }
>
> + mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;
> + if (ldpc)
> + mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
> +

Here you are using ldpc uninitialized.


As for the idea, ldpc and stbc support/enable in HT and VHT being
independant and tx_flags being shared by all the info.rates, you
cannot use it as you are trying to (e.g. you might end up asking lower
layers to send stbc/vht with a hw only supporting stbc/ht or/and
trying to send stbc/vht with inappropriate advertised vht caps).
This stems from the STBC/LDPC CTL flags being a
mac80211_tx_info_flags, this is what should be changed.


Regards,
Karl

2016-06-29 08:22:46

by Krishna Chaitanya

[permalink] [raw]
Subject: Re: [PATCH] mac80211: minstrel: Enable STBC and LDPC for VHT Rates

On Wed, Jun 29, 2016 at 1:50 PM, Krishna Chaitanya
<[email protected]> wrote:
> On Wed, Jun 29, 2016 at 1:41 PM, Karl Beldan <[email protected]> wrote:
>> On Mon, Jun 27, 2016 at 9:53 AM, Chaitanya TK <[email protected]>
>> wrote:
>>>
>>> From: Chaitanya T K <[email protected]>
>>>
>>> If peer support reception of STBC and LDPC, enable them for better
>>> performance.
>>>
>>> Signed-off-by: Chaitanya TK <[email protected]>
>>> ---
>>> include/linux/ieee80211.h | 1 +
>>> net/mac80211/rc80211_minstrel_ht.c | 25 +++++++++++++++++--------
>>> 2 files changed, 18 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
>>> index b118744..4d01130 100644
>>> --- a/include/linux/ieee80211.h
>>> +++ b/include/linux/ieee80211.h
>>> @@ -1550,6 +1550,7 @@ struct ieee80211_vht_operation {
>>> #define IEEE80211_VHT_CAP_RXSTBC_3 0x00000300
>>> #define IEEE80211_VHT_CAP_RXSTBC_4 0x00000400
>>> #define IEEE80211_VHT_CAP_RXSTBC_MASK 0x00000700
>>> +#define IEEE80211_VHT_CAP_RXSTBC_SHIFT 8
>>> #define IEEE80211_VHT_CAP_SU_BEAMFORMER_CAPABLE
>>> 0x00000800
>>> #define IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE
>>> 0x00001000
>>> #define IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT 13
>>> diff --git a/net/mac80211/rc80211_minstrel_ht.c
>>> b/net/mac80211/rc80211_minstrel_ht.c
>>> index 30fbabf..e2fcdea 100644
>>> --- a/net/mac80211/rc80211_minstrel_ht.c
>>> +++ b/net/mac80211/rc80211_minstrel_ht.c
>>> @@ -1166,13 +1166,14 @@ minstrel_ht_update_caps(void *priv, struct
>>> ieee80211_supported_band *sband,
>>> struct minstrel_ht_sta_priv *msp = priv_sta;
>>> struct minstrel_ht_sta *mi = &msp->ht;
>>> struct ieee80211_mcs_info *mcs = &sta->ht_cap.mcs;
>>> - u16 sta_cap = sta->ht_cap.cap;
>>> + u16 ht_cap = sta->ht_cap.cap;
>>> struct ieee80211_sta_vht_cap *vht_cap = &sta->vht_cap;
>>> int use_vht;
>>> int n_supported = 0;
>>> int ack_dur;
>>> int stbc;
>>> int i;
>>> + bool ldpc;
>>>
>>> /* fall back to the old minstrel for legacy stations */
>>> if (!sta->ht_cap.ht_supported)
>>> @@ -1210,16 +1211,24 @@ minstrel_ht_update_caps(void *priv, struct
>>> ieee80211_supported_band *sband,
>>> }
>>> mi->sample_tries = 4;
>>>
>>> - /* TODO tx_flags for vht - ATM the RC API is not fine-grained
>>> enough */
>>> if (!use_vht) {
>>> - stbc = (sta_cap & IEEE80211_HT_CAP_RX_STBC) >>
>>> + stbc = (ht_cap & IEEE80211_HT_CAP_RX_STBC) >>
>>> IEEE80211_HT_CAP_RX_STBC_SHIFT;
>>> - mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;
>>>
>>> - if (sta_cap & IEEE80211_HT_CAP_LDPC_CODING)
>>> - mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
>>> + if (ht_cap & IEEE80211_HT_CAP_LDPC_CODING)
>>> + ldpc = true;
>>> + } else {
>>> + stbc = (vht_cap->cap & IEEE80211_VHT_CAP_RXSTBC_MASK) >>
>>> + IEEE80211_VHT_CAP_RXSTBC_SHIFT;
>>> +
>>> + if (vht_cap->cap & IEEE80211_VHT_CAP_RXLDPC)
>>> + ldpc = true;
>>> }
>>>
>>> + mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;
>>> + if (ldpc)
>>> + mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
>>> +
>>
>>
>> Here you are using ldpc uninitialized.
yeah, will fix it.

2016-06-29 07:25:05

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: minstrel: Enable STBC and LDPC for VHT Rates

On Mon, 2016-06-27 at 15:23 +0530, Chaitanya TK wrote:
> From: Chaitanya T K <[email protected]>
>
> If peer support reception of STBC and LDPC, enable them for better
> performance.
>
Applied.

johannes

2016-06-29 08:20:30

by Krishna Chaitanya

[permalink] [raw]
Subject: Re: [PATCH] mac80211: minstrel: Enable STBC and LDPC for VHT Rates

On Wed, Jun 29, 2016 at 1:41 PM, Karl Beldan <[email protected]> wrote:
> On Mon, Jun 27, 2016 at 9:53 AM, Chaitanya TK <[email protected]>
> wrote:
>>
>> From: Chaitanya T K <[email protected]>
>>
>> If peer support reception of STBC and LDPC, enable them for better
>> performance.
>>
>> Signed-off-by: Chaitanya TK <[email protected]>
>> ---
>> include/linux/ieee80211.h | 1 +
>> net/mac80211/rc80211_minstrel_ht.c | 25 +++++++++++++++++--------
>> 2 files changed, 18 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
>> index b118744..4d01130 100644
>> --- a/include/linux/ieee80211.h
>> +++ b/include/linux/ieee80211.h
>> @@ -1550,6 +1550,7 @@ struct ieee80211_vht_operation {
>> #define IEEE80211_VHT_CAP_RXSTBC_3 0x00000300
>> #define IEEE80211_VHT_CAP_RXSTBC_4 0x00000400
>> #define IEEE80211_VHT_CAP_RXSTBC_MASK 0x00000700
>> +#define IEEE80211_VHT_CAP_RXSTBC_SHIFT 8
>> #define IEEE80211_VHT_CAP_SU_BEAMFORMER_CAPABLE
>> 0x00000800
>> #define IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE
>> 0x00001000
>> #define IEEE80211_VHT_CAP_BEAMFORMEE_STS_SHIFT 13
>> diff --git a/net/mac80211/rc80211_minstrel_ht.c
>> b/net/mac80211/rc80211_minstrel_ht.c
>> index 30fbabf..e2fcdea 100644
>> --- a/net/mac80211/rc80211_minstrel_ht.c
>> +++ b/net/mac80211/rc80211_minstrel_ht.c
>> @@ -1166,13 +1166,14 @@ minstrel_ht_update_caps(void *priv, struct
>> ieee80211_supported_band *sband,
>> struct minstrel_ht_sta_priv *msp = priv_sta;
>> struct minstrel_ht_sta *mi = &msp->ht;
>> struct ieee80211_mcs_info *mcs = &sta->ht_cap.mcs;
>> - u16 sta_cap = sta->ht_cap.cap;
>> + u16 ht_cap = sta->ht_cap.cap;
>> struct ieee80211_sta_vht_cap *vht_cap = &sta->vht_cap;
>> int use_vht;
>> int n_supported = 0;
>> int ack_dur;
>> int stbc;
>> int i;
>> + bool ldpc;
>>
>> /* fall back to the old minstrel for legacy stations */
>> if (!sta->ht_cap.ht_supported)
>> @@ -1210,16 +1211,24 @@ minstrel_ht_update_caps(void *priv, struct
>> ieee80211_supported_band *sband,
>> }
>> mi->sample_tries = 4;
>>
>> - /* TODO tx_flags for vht - ATM the RC API is not fine-grained
>> enough */
>> if (!use_vht) {
>> - stbc = (sta_cap & IEEE80211_HT_CAP_RX_STBC) >>
>> + stbc = (ht_cap & IEEE80211_HT_CAP_RX_STBC) >>
>> IEEE80211_HT_CAP_RX_STBC_SHIFT;
>> - mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;
>>
>> - if (sta_cap & IEEE80211_HT_CAP_LDPC_CODING)
>> - mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
>> + if (ht_cap & IEEE80211_HT_CAP_LDPC_CODING)
>> + ldpc = true;
>> + } else {
>> + stbc = (vht_cap->cap & IEEE80211_VHT_CAP_RXSTBC_MASK) >>
>> + IEEE80211_VHT_CAP_RXSTBC_SHIFT;
>> +
>> + if (vht_cap->cap & IEEE80211_VHT_CAP_RXLDPC)
>> + ldpc = true;
>> }
>>
>> + mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;
>> + if (ldpc)
>> + mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
>> +
>
>
> Here you are using ldpc uninitialized.
>
>
> As for the idea, ldpc and stbc support/enable in HT and VHT being
> independant and tx_flags being shared by all the info.rates, you cannot use
> it as you are trying to (e.g. you might end up asking lower layers to send
> stbc/vht with a hw only supporting stbc/ht or/and trying to send stbc/vht
> with inappropriate advertised vht caps).
The design in minstrel is it checks only for peers capabilities and it doesn't
check local hw capabilities (except for those in MCS_GROUPS), so minstrel
just informs the we can use STBC/LDPC, its HW's decision whether to
actually use or not.
> This stems from the STBC/LDPC CTL flags being a mac80211_tx_info_flags, this
> is what should be changed.
I understand the problem here, if all rates are same then its no problem.
But if some rates are HT and some are VHT, then we might end up in problem.

To solve this, we need to move this to mac80211_rate_control_flags so that
we can convey per rate info.

2016-06-29 09:44:51

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: minstrel: Enable STBC and LDPC for VHT Rates


> How do you want to proceed? Do you want to
> revert this patch and shall i send another patch?
>

I'll rebase the patch out.

johannes