| Message ID | 1346784741-7003-1-git-send-email-ordex@autistici.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Tue, 2012-09-04 at 20:52 +0200, Antonio Quartulli wrote: > Each band maps the bitmap of rates to different real bitrates, therefore using > the same bitmask for every band (as it is now) is not correct. > Each band must have its own bitmask where the bits of the rates specified by the > user on IBSS join have to be set > > Signed-off-by: Antonio Quartulli <ordex@autistici.org> > --- > include/net/cfg80211.h | 4 ++-- > net/mac80211/ibss.c | 6 ++++-- > net/mac80211/ieee80211_i.h | 2 +- > net/wireless/nl80211.c | 12 +++++++----- I'm not sure I see the need to change nl80211, and the change seems wrong anyway. As far as I understand it, the basic rates that are passed into the kernel are intended to be used when the kernel creates a new IBSS. This always happens on the channel that is also passed in, so it's always bound to a given channel (band). Now mac80211 internally seems to mess this up a bit, but that doesn't affect nl80211/cfg80211? 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 Tue, Sep 04, 2012 at 09:38:44PM +0200, Johannes Berg wrote: > On Tue, 2012-09-04 at 20:52 +0200, Antonio Quartulli wrote: > > Each band maps the bitmap of rates to different real bitrates, therefore using > > the same bitmask for every band (as it is now) is not correct. > > Each band must have its own bitmask where the bits of the rates specified by the > > user on IBSS join have to be set > > > > Signed-off-by: Antonio Quartulli <ordex@autistici.org> > > --- > > include/net/cfg80211.h | 4 ++-- > > net/mac80211/ibss.c | 6 ++++-- > > net/mac80211/ieee80211_i.h | 2 +- > > net/wireless/nl80211.c | 12 +++++++----- > > I'm not sure I see the need to change nl80211, and the change seems > wrong anyway. > > As far as I understand it, the basic rates that are passed into the > kernel are intended to be used when the kernel creates a new IBSS. This > always happens on the channel that is also passed in, so it's always > bound to a given channel (band). I think basic rates are also used in case of merging, right? In this case we could switch to another channel (and band). > > Now mac80211 internally seems to mess this up a bit, but that doesn't > affect nl80211/cfg80211? I modified nl/cfg as well because it is in that point that the bitrates provided by the user are converted to the band bitmask and so only here I create a bitmask for every band and pass them all to the mac80211. In this way we have a bitmask for each band and we can use the one we need whenever we switch band. is it a wrong approach? Cheers,
On Tue, 2012-09-04 at 21:45 +0200, Antonio Quartulli wrote: > > As far as I understand it, the basic rates that are passed into the > > kernel are intended to be used when the kernel creates a new IBSS. This > > always happens on the channel that is also passed in, so it's always > > bound to a given channel (band). > > I think basic rates are also used in case of merging, right? In this case we > could switch to another channel (and band). No, as I understand it we should in that case use the IBSS's existing basic rates, not our own. 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 Tue, Sep 04, 2012 at 09:47:36PM +0200, Johannes Berg wrote: > On Tue, 2012-09-04 at 21:45 +0200, Antonio Quartulli wrote: > > > > As far as I understand it, the basic rates that are passed into the > > > kernel are intended to be used when the kernel creates a new IBSS. This > > > always happens on the channel that is also passed in, so it's always > > > bound to a given channel (band). > > > > I think basic rates are also used in case of merging, right? In this case we > > could switch to another channel (and band). > > No, as I understand it we should in that case use the IBSS's existing > basic rates, not our own. mh..ok. At this point it doesn't make sense to really store all the bitmasks. Will modify it. Thank you
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 4c518f1..d16f903 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -1261,7 +1261,7 @@ struct cfg80211_disassoc_request { * sets/clears %NL80211_STA_FLAG_AUTHORIZED. If true, the driver is * required to assume that the port is unauthorized until authorized by * user space. Otherwise, port is marked authorized by default. - * @basic_rates: bitmap of basic rates to use when creating the IBSS + * @basic_rates: per-band bitmap of basic rates * @mcast_rate: per-band multicast rate index + 1 (0: disabled) */ struct cfg80211_ibss_params { @@ -1272,7 +1272,7 @@ struct cfg80211_ibss_params { u8 *ie; u8 ssid_len, ie_len; u16 beacon_interval; - u32 basic_rates; + u32 basic_rates[IEEE80211_NUM_BANDS]; bool channel_fixed; bool privacy; bool control_port; diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c index a9d9328..9b9b11f 100644 --- a/net/mac80211/ibss.c +++ b/net/mac80211/ibss.c @@ -686,7 +686,8 @@ static void ieee80211_sta_create_ibss(struct ieee80211_sub_if_data *sdata) sdata->drop_unencrypted = 0; __ieee80211_sta_join_ibss(sdata, bssid, sdata->vif.bss_conf.beacon_int, - ifibss->channel, ifibss->basic_rates, + ifibss->channel, + ifibss->basic_rates[ifibss->channel->band], capability, 0); } @@ -1045,7 +1046,8 @@ int ieee80211_ibss_join(struct ieee80211_sub_if_data *sdata, sdata->u.ibss.privacy = params->privacy; sdata->u.ibss.control_port = params->control_port; - sdata->u.ibss.basic_rates = params->basic_rates; + memcpy(sdata->u.ibss.basic_rates, params->basic_rates, + sizeof(params->basic_rates)); memcpy(sdata->vif.bss_conf.mcast_rate, params->mcast_rate, sizeof(params->mcast_rate)); diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index 204bfed..3465620 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -508,7 +508,7 @@ struct ieee80211_if_ibss { unsigned long last_scan_completed; - u32 basic_rates; + u32 basic_rates[IEEE80211_NUM_BANDS]; bool timer_running; diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 787aeaa..f9ecc17 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -5326,11 +5326,13 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info) nla_len(info->attrs[NL80211_ATTR_BSS_BASIC_RATES]); struct ieee80211_supported_band *sband = wiphy->bands[ibss.channel->band]; - - err = ieee80211_get_ratemask(sband, rates, n_rates, - &ibss.basic_rates); - if (err) - return err; + for (i = 0; i < IEEE80211_NUM_BANDS; i++) { + err = ieee80211_get_ratemask(wiphy->bands[i], rates, + n_rates, + ibss.basic_rates[i]); + if (err) + return err; + } } if (info->attrs[NL80211_ATTR_MCAST_RATE] &&
Each band maps the bitmap of rates to different real bitrates, therefore using the same bitmask for every band (as it is now) is not correct. Each band must have its own bitmask where the bits of the rates specified by the user on IBSS join have to be set Signed-off-by: Antonio Quartulli <ordex@autistici.org> --- include/net/cfg80211.h | 4 ++-- net/mac80211/ibss.c | 6 ++++-- net/mac80211/ieee80211_i.h | 2 +- net/wireless/nl80211.c | 12 +++++++----- 4 files changed, 14 insertions(+), 10 deletions(-)