Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 5 May 2020 15:15:31 +0200
From:   Maxime Bizon <mbizon@freebox.fr>
To:     Toke =?iso-8859-1?Q?H=F8iland-J=F8rgensen?= <toke@redhat.com>
Cc:     linux-wireless@vger.kernel.org
Subject: Re: Regarding .wake_tx_queue() model
Message-ID: <20200505131531.GA32619@sakura>
References: <20200504193959.GC26805@sakura>
 <878si6oabp.fsf@toke.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <878si6oabp.fsf@toke.dk>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk


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

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.

-- 
Maxime