Re: [PATCH] mac80211: Fix WMM ACM parsing and AC downgrade operation

[Johannes Berg <johannes@sipsolutions.net>]

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

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
> > >  	/* in case we are a client verify acm is not set for this ac */
> > >  	while (unlikely(local->wmm_acm & BIT(skb->priority))) {
> > >  		if (wme_downgrade_ac(skb)) {
> > > -			/* The old code would drop the packet in this
> > > -			 * case.
> > > +			/*
> > > +			 * This should not really happen. The AP has marked all
> > > +			 * lower ACs to require admission control which is not
> > > +			 * a reasonable configuration. Allow the frame to be
> > > +			 * transmitted using AC_BK as a workaround.
> > >  			 */
> > > -			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.

> In theory, the correct behavior would be to
> drop the frame if the AP is using mandatory admission control for all
> ACs, but in practice, it is probably better not to break the connection
> completely if the AP is misconfigured. The behavior after this patch is
> to downgrade the AC until one is found without ACM and if no such AC
> exists, use the lowest priority (AC_BK).

True. I'm just questioning that this really is what's happening with
this patch :)

johannes

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