On Wed, 2009-02-18 at 14:37 +0100, Johannes Berg wrote:
> On Wed, 2009-02-18 at 07:18 -0500, Dan Williams wrote:
>
> [snipped explanation]
>
> > Anyone think all this stuff really, really sucks? Yes!!! So lets just
> > have drivers/stack provide a few sane values that userspace really
> > doesn't have to go through this shit to calculate...
> >
> > <rant ends>
> >
> > NM is probably fine here with qual == 0 because I doubt the GIWRANGE
> > handler is returning a valid max_qual.qual > 0 anymore with Johannes'
> > patch. Could be wrong though.
>
> We're actually getting _two_ things wrong now and nobody ever noticed.
> So much for "but someone might care about the values wext returns". But
> there actually is a problem.
>
> First, we're reporting max_qual.qual != 0 still, this should be changed.
> wpa_supplicant is still reporting quality values, so the invalid flags
> aren't ever set. This needs to change in mac80211 (fixing the immediate
> regression) and wpa_supplicant.
>
> HOWEVER. Since all our qual.qual values are 0 though, your code _should_
> cope fine, because of this (from NM):
>
> /* If the quality percent was 0 or doesn't exist, then try to use signal levels instead */
> if ((percent < 1) && (level_percent >= 0))
> percent = level_percent;
>
> Now, why doesn't it?
>
> Because we don't report proper max_qual _level_ values. Something
> clearly nobody cared about because NM was using qual.qual and not
> qual.level. Here's the relevant snippet from mac80211:
>
> /* cfg80211 requires this, and enforces 0..100 */
> if (local->hw.flags & IEEE80211_HW_SIGNAL_UNSPEC)
> range->max_qual.level = 100;
> else if (local->hw.flags & IEEE80211_HW_SIGNAL_DBM)
> range->max_qual.level = -110;
> else
> range->max_qual.level = 0;
>
> Note the dBm branch -- clearly wrong. Did anyone care? Clearly not.
Almost; I was partially wrong as well. The comments from wireless.h
imply that max_qual.level in the dBm case can be < 0. Remember that
these values are *s8*, and thus max_qual.level can be < 0 and greater
than -127 at least.
This of course doesn't allow RSSI-based levels of > 127, but whatever.
To get the definitive determination of RSSI vs. dBm for level, you use
(updated & IW_QUAL_DBM).
The understanding of max_qual.level == 0 means dBm was probably from
conversations with Jean and that's apparently not borne out by the
evidence from wireless.h, unless there's something else I'm not reading.
NM doesn't handle IW_QUAL_DBM. It should.
> Therefore, here's what I'm going to do:
> 1) always set IW_QUAL_QUAL_INVALID, even in max_qual (fixes bug #1)
> 2) set max_qual.level to 0 for dBm instead of -110 (fixes bug #2)
>
> patch below.
I think leaving max_qual.level == -110 is OK in mac80211 actually. It's
NM that's got the problem by not respecting IW_QUAL_DBM.
Dan
> johannes
>
> Subject: mac80211: fix wext max_qual report
>
> mac80211 is reporting a max_qual.level of -110 for dBm while it
> should be reporting 0. It is also reporting a valid max_qual.qual
> although we no longer set qual.qual, which trips up NetworkManager
> due to a wpa_supplicant bug. But we don't need to report that our
> max_qual.qual value is valid.
>
> Fix the max_qual.level to be 0 for dBm and 100 for non-dBm and
> remove max_qual.qual. max_qual.noise = -110 also seems wrong,
> so fix that while at it.
>
> Signed-off-by: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
> ---
> net/mac80211/wext.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> --- wireless-testing.orig/net/mac80211/wext.c 2009-02-18 14:24:56.000000000 +0100
> +++ wireless-testing/net/mac80211/wext.c 2009-02-18 14:29:07.000000000 +0100
> @@ -148,11 +148,19 @@ static u8 ieee80211_get_wstats_flags(str
> {
> u8 wstats_flags = 0;
>
> - wstats_flags |= local->hw.flags & (IEEE80211_HW_SIGNAL_UNSPEC |
> - IEEE80211_HW_SIGNAL_DBM) ?
> - IW_QUAL_QUAL_UPDATED : IW_QUAL_QUAL_INVALID;
> - wstats_flags |= local->hw.flags & IEEE80211_HW_NOISE_DBM ?
> - IW_QUAL_NOISE_UPDATED : IW_QUAL_NOISE_INVALID;
> + wstats_flags |= IW_QUAL_QUAL_INVALID;
> +
> + if (local->hw.flags & (IEEE80211_HW_SIGNAL_UNSPEC |
> + IEEE80211_HW_SIGNAL_DBM))
> + wstats_flags |= IW_QUAL_LEVEL_UPDATED;
> + else
> + wstats_flags |= IW_QUAL_LEVEL_INVALID;
> +
> + if (local->hw.flags & IEEE80211_HW_NOISE_DBM)
> + wstats_flags |= IW_QUAL_NOISE_UPDATED;
> + else
> + wstats_flags |= IW_QUAL_NOISE_INVALID;
> +
> if (local->hw.flags & IEEE80211_HW_SIGNAL_DBM)
> wstats_flags |= IW_QUAL_DBM;
>
> @@ -191,19 +199,11 @@ static int ieee80211_ioctl_giwrange(stru
> if (local->hw.flags & IEEE80211_HW_SIGNAL_UNSPEC)
> range->max_qual.level = 100;
> else if (local->hw.flags & IEEE80211_HW_SIGNAL_DBM)
> - range->max_qual.level = -110;
> - else
> range->max_qual.level = 0;
>
> - if (local->hw.flags & IEEE80211_HW_NOISE_DBM)
> - range->max_qual.noise = -110;
> - else
> - range->max_qual.noise = 0;
> -
> - range->max_qual.qual = 100;
> + range->max_qual.noise = 0;
> range->max_qual.updated = ieee80211_get_wstats_flags(local);
>
> - range->avg_qual.qual = 50;
> /* not always true but better than nothing */
> range->avg_qual.level = range->max_qual.level / 2;
> range->avg_qual.noise = range->max_qual.noise / 2;
>
>
--
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
