| Message ID | 1421404724-32326-6-git-send-email-janusz.dziedzic@tieto.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Johannes Berg |
| Headers | show |
On Fri, 2015-01-16 at 11:38 +0100, Janusz Dziedzic wrote: > + /* we both use VHT */ > + struct ieee80211_vht_cap vhtcap_ie; > + struct ieee80211_sta_vht_cap vht_cap = sta->sta.vht_cap; > + > + ieee80211_vht_oper_to_chandef(channel, > + elems->vht_operation, > + &chandef); Ok maybe I'm missing something - but can't this erroneously configure the local HW to 160 MHz when it doesn't even support it, or so? > + memcpy(&vhtcap_ie, elems->vht_cap_elem, sizeof(vhtcap_ie)); > + ieee80211_vht_cap_ie_to_sta_vht_cap(sdata, sband, &vhtcap_ie, sta); > + if (memcmp(&vht_cap, &sta->sta.vht_cap, sizeof(vht_cap))) > + rates_updated |= true; Indentation is wrong here. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 16 January 2015 at 11:55, Johannes Berg <johannes@sipsolutions.net> wrote: > On Fri, 2015-01-16 at 11:38 +0100, Janusz Dziedzic wrote: > >> + /* we both use VHT */ >> + struct ieee80211_vht_cap vhtcap_ie; >> + struct ieee80211_sta_vht_cap vht_cap = sta->sta.vht_cap; >> + >> + ieee80211_vht_oper_to_chandef(channel, >> + elems->vht_operation, >> + &chandef); > > Ok maybe I'm missing something - but can't this erroneously configure > the local HW to 160 MHz when it doesn't even support it, or so? > I will check this more. But seems chandef (sta chandef) is a local variable here, not used by the way. So, our chandef is form cfg80211 (sdata->u.ibss.chandef) and we don't change this. Orginaly this sta chandef was used to compare with ibss->chandef. - if (chandef.center_freq1 != - sdata->u.ibss.chandef.center_freq1) - htcap_ie.cap_info &= - cpu_to_le16(~IEEE80211_HT_CAP_SUP_WIDTH_20_40); But for me this check seems as not needed, eg. We support VHT80 and other ibss have only HT40 support - so we will have different center_freq1 - but still could operate, while sta_add and correct rates for sta configured. One I think we could check here is, if our chandef and sta chandef overlap. Anyway, I am not sure I understand your question correctly, you mean eg. we work in VHT80 mode and other ibss join in VHT160 mode? Does it really matter while we will use sta_add for this new V160 "station" and configure supported rates for this station? BR Janusz >> + memcpy(&vhtcap_ie, elems->vht_cap_elem, sizeof(vhtcap_ie)); >> + ieee80211_vht_cap_ie_to_sta_vht_cap(sdata, sband, &vhtcap_ie, sta); >> + if (memcmp(&vht_cap, &sta->sta.vht_cap, sizeof(vht_cap))) >> + rates_updated |= true; > > Indentation is wrong here. > > johannes > -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2015-01-16 at 13:08 +0100, Janusz Dziedzic wrote: > On 16 January 2015 at 11:55, Johannes Berg <johannes@sipsolutions.net> wrote: > > On Fri, 2015-01-16 at 11:38 +0100, Janusz Dziedzic wrote: > > > >> + /* we both use VHT */ > >> + struct ieee80211_vht_cap vhtcap_ie; > >> + struct ieee80211_sta_vht_cap vht_cap = sta->sta.vht_cap; > >> + > >> + ieee80211_vht_oper_to_chandef(channel, > >> + elems->vht_operation, > >> + &chandef); > > > > Ok maybe I'm missing something - but can't this erroneously configure > > the local HW to 160 MHz when it doesn't even support it, or so? > > > I will check this more. But seems chandef (sta chandef) is a local > variable here, not used by the way. > So, our chandef is form cfg80211 (sdata->u.ibss.chandef) and we don't > change this. > Orginaly this sta chandef was used to compare with ibss->chandef. > > - if (chandef.center_freq1 != > - sdata->u.ibss.chandef.center_freq1) > - htcap_ie.cap_info &= > - > cpu_to_le16(~IEEE80211_HT_CAP_SUP_WIDTH_20_40); > > But for me this check seems as not needed, eg. > We support VHT80 and other ibss have only HT40 support - so we will > have different center_freq1 - but still could operate, while sta_add > and correct rates for sta configured. > One I think we could check here is, if our chandef and sta chandef overlap. > > Anyway, I am not sure I understand your question correctly, you mean eg. > we work in VHT80 mode and other ibss join in VHT160 mode? Does it > really matter while we will use sta_add for this new V160 "station" > and configure supported rates for this station? Well like I said - I might not understand this correctly. But the ieee80211_vht_oper_to_chandef() function doesn't - iirc - take into account local capabilities. As a consequence, if a VHT160 station joins the chandef might be 160 while we're only supporting 80? But anyway - I see that at least what I originally thought was wrong - this code isn't concerned with picking up the channel from the peer to join the networks together, I guess. That's what I was worried about. That code I haven't seen and checked though - so perhaps you can look there if it correctly handles trying to form a network when the peer has higher capabilities than the local hw. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 16 January 2015 at 13:18, Johannes Berg <johannes@sipsolutions.net> wrote: > On Fri, 2015-01-16 at 13:08 +0100, Janusz Dziedzic wrote: >> On 16 January 2015 at 11:55, Johannes Berg <johannes@sipsolutions.net> wrote: >> > On Fri, 2015-01-16 at 11:38 +0100, Janusz Dziedzic wrote: >> > >> >> + /* we both use VHT */ >> >> + struct ieee80211_vht_cap vhtcap_ie; >> >> + struct ieee80211_sta_vht_cap vht_cap = sta->sta.vht_cap; >> >> + >> >> + ieee80211_vht_oper_to_chandef(channel, >> >> + elems->vht_operation, >> >> + &chandef); >> > >> > Ok maybe I'm missing something - but can't this erroneously configure >> > the local HW to 160 MHz when it doesn't even support it, or so? >> > >> I will check this more. But seems chandef (sta chandef) is a local >> variable here, not used by the way. >> So, our chandef is form cfg80211 (sdata->u.ibss.chandef) and we don't >> change this. >> Orginaly this sta chandef was used to compare with ibss->chandef. >> >> - if (chandef.center_freq1 != >> - sdata->u.ibss.chandef.center_freq1) >> - htcap_ie.cap_info &= >> - >> cpu_to_le16(~IEEE80211_HT_CAP_SUP_WIDTH_20_40); >> >> But for me this check seems as not needed, eg. >> We support VHT80 and other ibss have only HT40 support - so we will >> have different center_freq1 - but still could operate, while sta_add >> and correct rates for sta configured. >> One I think we could check here is, if our chandef and sta chandef overlap. >> >> Anyway, I am not sure I understand your question correctly, you mean eg. >> we work in VHT80 mode and other ibss join in VHT160 mode? Does it >> really matter while we will use sta_add for this new V160 "station" >> and configure supported rates for this station? > > Well like I said - I might not understand this correctly. But the > ieee80211_vht_oper_to_chandef() function doesn't - iirc - take into > account local capabilities. As a consequence, if a VHT160 station joins > the chandef might be 160 while we're only supporting 80? > > But anyway - I see that at least what I originally thought was wrong - > this code isn't concerned with picking up the channel from the peer to > join the networks together, I guess. That's what I was worried about. > That code I haven't seen and checked though - so perhaps you can look > there if it correctly handles trying to form a network when the peer has > higher capabilities than the local hw. > I am testing different combinations and TP HT20 <-> HT40 HT20 <-> VHT80 HT40 <-> VHT80 HT40 <-> VHT80 VHT80 <-> HT20 VHT80 <-> HT40 using ath9k and ath10k. What I see using iw wlan0 info, we always using chandef we pass from cfg80211. We also update nss/rates/bw correctly in sta_rc_update(). But anyway will do more tests. BR Janusz -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c index 3c6df2b..b4263b5 100644 --- a/net/mac80211/ibss.c +++ b/net/mac80211/ibss.c @@ -1072,18 +1072,25 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata, memcpy(&htcap_ie, elems->ht_cap_elem, sizeof(htcap_ie)); - /* - * fall back to HT20 if we don't use or use - * the other extension channel - */ - if (chandef.center_freq1 != - sdata->u.ibss.chandef.center_freq1) - htcap_ie.cap_info &= - cpu_to_le16(~IEEE80211_HT_CAP_SUP_WIDTH_20_40); - rates_updated |= ieee80211_ht_cap_ie_to_sta_ht_cap( sdata, sband, &htcap_ie, sta); + if (elems->vht_operation && elems->vht_cap_elem && + sdata->u.ibss.chandef.width != NL80211_CHAN_WIDTH_20 && + sdata->u.ibss.chandef.width != NL80211_CHAN_WIDTH_40) { + /* we both use VHT */ + struct ieee80211_vht_cap vhtcap_ie; + struct ieee80211_sta_vht_cap vht_cap = sta->sta.vht_cap; + + ieee80211_vht_oper_to_chandef(channel, + elems->vht_operation, + &chandef); + memcpy(&vhtcap_ie, elems->vht_cap_elem, sizeof(vhtcap_ie)); + ieee80211_vht_cap_ie_to_sta_vht_cap(sdata, sband, &vhtcap_ie, sta); + if (memcmp(&vht_cap, &sta->sta.vht_cap, sizeof(vht_cap))) + rates_updated |= true; + } + if (bw != sta->sta.bandwidth) rates_updated |= true; } diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index 05d3af5..a3e93ad 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -1953,6 +1953,9 @@ u8 *ieee80211_add_wmm_info_ie(u8 *buf, u8 qosinfo); void ieee80211_ht_oper_to_chandef(struct ieee80211_channel *control_chan, const struct ieee80211_ht_operation *ht_oper, struct cfg80211_chan_def *chandef); +void ieee80211_vht_oper_to_chandef(struct ieee80211_channel *control_chan, + const struct ieee80211_vht_operation *oper, + struct cfg80211_chan_def *chandef); u32 ieee80211_chandef_downgrade(struct cfg80211_chan_def *c); int __must_check diff --git a/net/mac80211/util.c b/net/mac80211/util.c index ee6a601..f5599ac 100644 --- a/net/mac80211/util.c +++ b/net/mac80211/util.c @@ -2393,6 +2393,37 @@ void ieee80211_ht_oper_to_chandef(struct ieee80211_channel *control_chan, cfg80211_chandef_create(chandef, control_chan, channel_type); } +void ieee80211_vht_oper_to_chandef(struct ieee80211_channel *control_chan, + const struct ieee80211_vht_operation *oper, + struct cfg80211_chan_def *chandef) +{ + if (!oper) + return; + + chandef->chan = control_chan; + + switch (oper->chan_width) { + case IEEE80211_VHT_CHANWIDTH_USE_HT: + break; + case IEEE80211_VHT_CHANWIDTH_80MHZ: + chandef->width = NL80211_CHAN_WIDTH_80; + break; + case IEEE80211_VHT_CHANWIDTH_160MHZ: + chandef->width = NL80211_CHAN_WIDTH_160; + break; + case IEEE80211_VHT_CHANWIDTH_80P80MHZ: + chandef->width = NL80211_CHAN_WIDTH_80P80; + break; + } + + chandef->center_freq1 = ieee80211_channel_to_frequency( + oper->center_freq_seg1_idx, + control_chan->band); + chandef->center_freq2 = ieee80211_channel_to_frequency( + oper->center_freq_seg2_idx, + control_chan->band); +} + int ieee80211_parse_bitrates(struct cfg80211_chan_def *chandef, const struct ieee80211_supported_band *sband, const u8 *srates, int srates_len, u32 *rates)
Setup correctly BW in case of STA using VHT. Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com> --- net/mac80211/ibss.c | 25 ++++++++++++++++--------- net/mac80211/ieee80211_i.h | 3 +++ net/mac80211/util.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 9 deletions(-)