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

[Johannes Berg <johannes@sipsolutions.net>]

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?

> Now that I'm thinking about this some more, it might even make sense to
> skip the sta PS queue for txq-enabled drivers. That would allow all sta
> data frames to either go through driver scheduling or
> release_buffered_frames.

Well, it's slightly more complicated due to the filtered queue. Not sure
you want to 'pollute' the TXQ abstraction with that?

> 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? :)

johannes