Maxime Bizon <mbizon@xxxxxxxxxx> writes: > On Tuesday 05 May 2020 à 14:06:18 (+0200), Toke Høiland-Jørgensen wrote: > > Hello Toke, > > thanks for your comments > >> I don't think ieee80211_stop_queue() is meant to be used this way at all >> in the wake_tx_queue case. Rather, when you get a wake_tx_queue() >> callback, you just queue as many frames as you feel like (see '2 >> aggregate' limit above), and then return. Then, on a TX completion you >> just call your internal driver function that tries to pull more frames >> from the mac80211 TXQs. > > alright, indeed .wake_tx_queue() does not have to result in any actual > frame sent. > > but what about .release_buffered_frames then ? > > .release_buffered_frames() and .drv_tx() are both "push" oriented > interface. When they are called, you have to push a frame to the > hardware, so if they are called when hardware FIFO is full, you need > to drop the frame (or add yet another intermediate level of queuing) > > from what I can see, .release_buffered_frames() will happily be called > even if queue is stopped, and you have to at least send one frame. > > I'm just trying to avoid any additionnal intermediate queing in the > driver between what mac80211 gives me and the HW fifo which has a > fixed size (well actually you can choose the size, but only at init > time). > > my current workaround is to pre-size the hw fifo queue to handle what > I think is the worst case Well, I think that should be fine? Having a longer HW queue is fine, as long as you have some other logic to not fill it all the time (like the "max two aggregates" logic I mentioned before). I think this is what ath9k does too. At least it looks like both drv_tx() and release_buffered_frames() will just abort (and drop in the former case) if there is no HW buffer space left. > Also .release_buffered_frames() codepath is difficult to test, how do > you trigger that reliably ? assuming VIF is an AP, then you need the > remote STA to go to sleep even though you have traffic waiting for it > in the txqi. For now I patch the stack to introduce artificial delay. > > Since my hardware has a sticky per-STA PS filter with good status > reporting, I'm considering using ieee80211_sta_block_awake() and only > unblock STA when all its txqi are empty to get rid of > .release_buffered_frames() complexity. I'm probably not the right person to answer that; never did have a good grip on the details of PS support. What hardware is it you're writing a driver for, BTW, and are you planning to upstream it? :) -Toke
