On Sun, Mar 08, 2009 at 03:25:33PM +0100, Johannes Berg wrote:
> On Sun, 2009-03-08 at 16:17 +0200, Jouni Malinen wrote:
> > On Sun, Mar 08, 2009 at 01:28:07PM +0100, Johannes Berg wrote:
> > > On Thu, 2009-03-05 at 17:23 +0200, Jouni Malinen wrote:
> > > > @@ -99,10 +99,13 @@ static u16 classify80211(struct ieee8021
> > > > while (unlikely(local->wmm_acm & BIT(skb->priority))) {
> > > > if (wme_downgrade_ac(skb)) {
> > > > - return 0;
> > > > + break;
> > > It seems to me that return 0 here was incorrect, or wme_downgrade_ac
> > > needs changes?
> > Yes, this return 0 was incorrect and that's why I'm fixing it to not
> > return 0 in this patch.. ;-)
> Sorry, I meant correct. Why is it not correct? wme_downgrade_ac doesn't
> modify the skb when it returns an error.
Ah, okay, that's a more sensible comment and I can even try to provide a
more useful reply after having understood your point ;-). The failed
call to wme_downgrade_ac() does not modify the skb, but the previous
call(s) did (if there was one and there likely was). In other words, the
loop of calling wme_downgrade_ac() will end up downgrading the skb all
the way to using skb->priority = 2 which maps to AC_BK.
classify80211() returns the queue number based on the 802.1d tag (= UP)
and the old code was returning 0 in the error case (could not
downgrade). If you take a look at the ieee802_1d_to_ac[] array in the
beginning of wme.c you can see that the value 0 is used for UP values 6
and 7, i.e., something that maps to AC_VO, while we really should return
ieee802_1d_to_ac[2] = 3 (AC_BK) here. Breaking the loop on error makes
the classify80211() look up the proper queue number from
ieee802_1d_to_ac[] based on the downgraded skb->priority, not by using a
hardcoded value here, and ends up returning the desired value here
(lowest priority queue, not highest as the current code does).
--
Jouni Malinen PGP id EFC895FA
--
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
