On Wed, Dec 9, 2015 at 12:52 AM, Grumbach, Emmanuel <emmanuel.grumbach@xxxxxxxxx> wrote: > > > On 12/08/2015 09:11 PM, Krishna Chaitanya wrote: >> >> >> On Dec 9, 2015 12:37 AM, "Grumbach, Emmanuel" >> <emmanuel.grumbach@xxxxxxxxx <mailto:emmanuel.grumbach@xxxxxxxxx>> wrote: >> > >> > >> > >> > On 12/08/2015 07:49 PM, Krishna Chaitanya wrote: >> > > >> > > >> > > On Dec 8, 2015 10:13 PM, "Grumbach, Emmanuel" >> > > <emmanuel.grumbach@xxxxxxxxx <mailto:emmanuel.grumbach@xxxxxxxxx> >> <mailto:emmanuel.grumbach@xxxxxxxxx >> <mailto:emmanuel.grumbach@xxxxxxxxx>>> wrote: >> > > > >> > > > >> > > > >> > > > On 12/08/2015 06:35 PM, Krishna Chaitanya wrote: >> > > > > >> > > > > >> > > > > On Dec 8, 2015 10:01 PM, "Grumbach, Emmanuel" >> > > > > <emmanuel.grumbach@xxxxxxxxx >> <mailto:emmanuel.grumbach@xxxxxxxxx> >> <mailto:emmanuel.grumbach@xxxxxxxxx <mailto:emmanuel.grumbach@xxxxxxxxx>> >> > > <mailto:emmanuel.grumbach@xxxxxxxxx >> <mailto:emmanuel.grumbach@xxxxxxxxx> >> > > <mailto:emmanuel.grumbach@xxxxxxxxx >> <mailto:emmanuel.grumbach@xxxxxxxxx>>>> wrote: >> > > > > > >> > > > > > >> > > > > > >> > > > > > On 12/08/2015 06:03 PM, Krishna Chaitanya wrote: >> > > > > > > On Tue, Dec 8, 2015 at 7:34 PM, Emmanuel Grumbach >> > > > > > > <emmanuel.grumbach@xxxxxxxxx >> <mailto:emmanuel.grumbach@xxxxxxxxx> >> > > <mailto:emmanuel.grumbach@xxxxxxxxx >> <mailto:emmanuel.grumbach@xxxxxxxxx>> >> > > <mailto:emmanuel.grumbach@xxxxxxxxx >> <mailto:emmanuel.grumbach@xxxxxxxxx> >> <mailto:emmanuel.grumbach@xxxxxxxxx >> <mailto:emmanuel.grumbach@xxxxxxxxx>>>> >> > > > > wrote: >> > > > > > >> index 7a76ce6..f4a5287 100644 >> > > > > > >> --- a/net/mac80211/ht.c >> > > > > > >> +++ b/net/mac80211/ht.c >> > > > > > >> @@ -230,6 +230,11 @@ bool >> > > > > ieee80211_ht_cap_ie_to_sta_ht_cap(struct ieee80211_sub_if_data >> *sdata, >> > > > > > >> /* set Rx highest rate */ >> > > > > > >> ht_cap.mcs.rx_highest = ht_cap_ie->mcs.rx_highest; >> > > > > > >> >> > > > > > >> + if (ht_cap.cap & IEEE80211_HT_CAP_MAX_AMSDU) >> > > > > > >> + sta->sta.max_amsdu_len = >> > > > > IEEE80211_MAX_MPDU_LEN_HT_7935; >> > > > > > >> + else >> > > > > > >> + sta->sta.max_amsdu_len = >> > > > > IEEE80211_MAX_MPDU_LEN_HT_3839; >> > > > > > >> + >> > > > > > >> apply: >> > > > > > >> changed = memcmp(&sta->sta.ht_cap, &ht_cap, >> > > sizeof(ht_cap)); >> > > > > > >> >> > > > > > >> diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c >> > > > > > >> index 92c9843..d2f2ff6 100644 >> > > > > > >> --- a/net/mac80211/vht.c >> > > > > > >> +++ b/net/mac80211/vht.c >> > > > > > >> @@ -281,6 +281,24 @@ >> ieee80211_vht_cap_ie_to_sta_vht_cap(struct >> > > > > ieee80211_sub_if_data *sdata, >> > > > > > >> } >> > > > > > >> >> > > > > > >> sta->sta.bandwidth = ieee80211_sta_cur_vht_bw(sta); >> > > > > > >> + >> > > > > > >> + /* If HT IE reported 3839 bytes only, stay with that >> > > size. */ >> > > > > > >> + if (sta->sta.max_amsdu_len == >> > > IEEE80211_MAX_MPDU_LEN_HT_3839) >> > > > > > >> + return; >> > > > > > >> + >> > > > > > >> + switch (vht_cap->cap & >> IEEE80211_VHT_CAP_MAX_MPDU_MASK) { >> > > > > > >> + case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_11454: >> > > > > > >> + sta->sta.max_amsdu_len = >> > > > > IEEE80211_MAX_MPDU_LEN_VHT_11454; >> > > > > > >> + break; >> > > > > > >> + case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_7991: >> > > > > > >> + sta->sta.max_amsdu_len = >> > > > > IEEE80211_MAX_MPDU_LEN_VHT_7991; >> > > > > > >> + break; >> > > > > > >> + case IEEE80211_VHT_CAP_MAX_MPDU_LENGTH_3895: >> > > > > > >> + default: >> > > > > > >> + sta->sta.max_amsdu_len = >> > > > > IEEE80211_MAX_MPDU_LEN_VHT_3895; >> > > > > > >> + break; >> > > > > > >> + } >> > > > > > > Ideally speaking shouldn't we use different variables for HT >> > > and VHT >> > > > > > > and depending on >> > > > > > > rate HT/VHT we should aggregate accordingly? of course that >> > > will be >> > > > > > > tricky as we dont >> > > > > > > have the rate control info at the time of aggregation. >> Standard is >> > > > > > > clear about usage of >> > > > > > > independent lengths depending on whether its an VHT/HT PPDU. >> > > > > > >> > > > > > Yes - but since it is tricky, I preferred to be on the safe >> side and >> > > > > > limit VHT amsdus for the very peculiar AP that would decide >> to have >> > > > > > large A-MSDUs in VHT and small ones in HT (?!). >> > > > > Yes but wouldn't it be safer to just use min(ht , vht)? For a >> good AP >> > > > > it shouldn't matter and bad AP it will still work. >> > > > > >> > > > This is the de-facto what this code does I think. >> > > > If the HT limit is 4K, then don't even take the VHT limit into >> account >> > > > and the VHT limit can't be smaller than the HT one in that case. >> > > > If the HT limit is 8K, then we can limit it further to 4K in >> case VHT >> > > > has a limit of 4K (which is another weird case), but we can also >> make it >> > > > larger for VHT frames? >> > > > >> > > > So I don't really see the bug here, am I missing something? >> > > If we take a maximum case HT=8K and VHT=12K then we endup sending 12K >> > > frames using HT rates... Which is not expected... >> > > >> > Right, but it is explicitly mentioned in the API that if you use an HT >> > preamble you should limit the A-MSDU to 8K. >> > If the HT limit was to be 4K, then the VHT would be 4K as well (even if >> > a broken peer would advertise 8K in HT and 4K in VHT). >> OK I did not read the documentation ;-) yes its clear now but I still >> think min() should make things easier rather than driver taking care >> of this. >> > Not really... You'd need another variable since min() would limit the > AMSDU length to the smallest (HT realistically) limit. Yes. > And as you pointed out, typically, the Tx MPDU and its rate are decided > / built in two different layers. In iwlwifi we plan to be using VHT > preamble only when we have a VHT association (and forbid AMSDU when the > rates drop below a TBD limit). Ok, that makes sense, no point in using AMSDU if the rates are dropping. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
