Re: [PATCH 4/9] rt2x00: Use rt2x00 queue numbering

[Ivo van Doorn <ivdoorn@gmail.com>]

On Monday 21 April 2008, Ivo van Doorn wrote:
> On Monday 21 April 2008, Johannes Berg wrote:
> > Hi Ivo,
> > 
> > On Mon, 2008-04-21 at 19:00 +0200, Ivo van Doorn wrote:
> > > Use the rt2x00 queue enumeration as much as possible,
> > > removing the usage of the mac80211 queue numbering
> > > wherever it is possible.
> > > 
> > > This makes it easier for mac80211 to change it queue
> > > identification scheme without having to deal with
> > > big changes in the rt2x00 code.
> > 
> > Cool, thanks a lot. One small note:
> > 
> > > +static inline enum data_queue_qid mac80211_queue_to_qid(unsigned int queue)
> > > +{
> > > +	/* Regular TX queues are mapped directly */
> > > +	if (queue < NUM_TX_DATA_QUEUES)
> > > +		return queue;
> > > +	else if (queue == IEEE80211_TX_QUEUE_BEACON)
> > > +		return QID_BEACON;
> > > +	else if (queue == IEEE80211_TX_QUEUE_AFTER_BEACON)
> > > +		return QID_ATIM;
> > > +	return QID_OTHER;
> > 
> > Are you actually using TX_QUEUE_AFTER_BEACON? That queue number is a
> > historical accident (imho) and not used, we actually use the
> > IEEE80211_TXCTL_SEND_AFTER_DTIM tx control flag.
> 
> Actually both the checks for IEEE80211_TX_QUEUE_BEACON and IEEE80211_TX_QUEUE_AFTER_BEACON
> are fake and only present for completeness.
> rt2x00 does check for IEEE80211_TXCTL_SEND_AFTER_DTIM and will resolve it to QID_ATIM
> if that queue is supported by the hardware (rt2400pci and rt2500pci) and resolves it to the control->queue
> in all other cases.
> 
> > Also, with the QoS patch, those queue numbers should finally go away and
> > mac80211 will not pass anything but the data queues, hence, this small
> > inline would probably be reduced to "return queue;" by the patch.
> 
> This function only exists to move the translation in a single location.
> When the translation isn't needed anymore because skb_get_queue_mapping(skb)
> or something else is used, then this function can be removed completely. :)

Ps. Can I assume that skb_get_queue_mapping(skb) or whatever replacement
for control->queue will make sure the given queue will not exceed the max
number of supported queues as configured by the driver (through hw->queues)?

Or should the driver drop such packets or resolve it to the highest queue available?

Ivo