Re: [PATCH 1/2] mac80211: supp_rates initialization and rate control notification

Johannes Berg
2009-11-24T01:17:18Z

[-- Attachment #1: Type: text/plain, Size: 2613 bytes --]

On Mon, 2009-11-23 at 16:57 -0800, Adam Wozniak wrote:
> Previously, not all code paths set supp_rates, and a rate change did not
> reinitialize the rate control layer.
> This patch fixes those issues.
>
> Signed-off-by: Adam Wozniak <awozniak@irobot.com>
> ---
> diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
> index 10d1385..474f66d 100644
> --- a/net/mac80211/ibss.c
> +++ b/net/mac80211/ibss.c
> @@ -246,9 +246,13 @@ static void ieee80211_rx_bss_info(struct
> ieee80211_sub_if_data *sdata,
> if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
> return;
>
> + supp_rates = ieee80211_sta_get_rates(local, elems, band);
> +
> + /* make sure mandatory rates are always added */
> + supp_rates |= ieee80211_mandatory_rates(local, band);
> +
> if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
> memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
> - supp_rates = ieee80211_sta_get_rates(local, elems, band);

This change is pointless -- if we don't get into this if() we don't care
about the value of supp_rates.

> @@ -257,12 +261,10 @@ static void ieee80211_rx_bss_info(struct
> ieee80211_sub_if_data *sdata,
> u32 prev_rates;
>
> prev_rates = sta->sta.supp_rates[band];
> - /* make sure mandatory rates are always added */
> - sta->sta.supp_rates[band] = supp_rates |
> - ieee80211_mandatory_rates(local, band);
> + sta->sta.supp_rates[band] = supp_rates;

That's just as pointless, it doesn't matter whether you add the
mandatory rates here or earlier.

> + if (sta->sta.supp_rates[band] != prev_rates) {
> #ifdef CONFIG_MAC80211_IBSS_DEBUG
> - if (sta->sta.supp_rates[band] != prev_rates)
> printk(KERN_DEBUG "%s: updated supp_rates set "
> "for %pM based on beacon info (0x%llx | "
> "0x%llx -> 0x%llx)\n",
> @@ -272,6 +274,8 @@ static void ieee80211_rx_bss_info(struct
> ieee80211_sub_if_data *sdata,
> (unsigned long long) supp_rates,
> (unsigned long long) sta->sta.supp_rates[band]);
> #endif
> + rate_control_rate_init(sta);
> + }
> } else
> ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa,
> supp_rates);

And that's not really right -- I don't think we should be calling
_init() over and over again. This could be a call to
rate_control_rate_update() with a certain flag instead.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]