Re: [PATCH v4] mac80211: add an intermediate software queue implementation

[Felix Fietkau <nbd@openwrt.org>]

On 2015-03-18 21:07, Johannes Berg wrote:
> On Wed, 2015-03-18 at 21:03 +0100, Felix Fietkau wrote:
>> On 2015-03-18 20:41, Johannes Berg wrote:
>> >> + * The driver is expected to release its own buffered frames and also call
>> >> + * ieee80211_tx_dequeue() within that callback.
>> > 
>> > Perhaps that should read
>> > "The driver is expected to release its own buffered frames (if any) and
>> > request the remaining dequeued frames by calling
>> > ieee80211_tx_dequeue()."
>> > 
>> > I'm not really sure it needs to be within that callback? I see no
>> > particular reason for that.
>> Releasing multiple packets works, even if there is only one packet
>> buffered in the driver and the rest in the txq. It also keeps the code
>> more consistent.
> 
> Right. I still phrased that badly. I meant that the "also" should be
> limited by the number of frames really needed, i.e. use driver-buffered
> first and fill up with any mac80211-buffered by dequeuing. That's
> probably obvious enough though.
> 
> Anyway - not sure it needs to be in the callback?
I think it makes things simpler this way. What would you prefer?

>> If I don't lock here, one last dequeue call might still be running on
>> another CPU. This would produce a theoretical race in accessing the
>> sequence number, which the caller of this function reads before setting
>> up the BA request.
>> Dequeueing happens outside of the normal network stack tx context, so
>> synchronize_net is not enough.
> 
> Ah, makes sense, I didn't think of the seqno. Can you please put that in
> a comment somewhere? :)
Sure.

- Felix