Re: [PATCH v3 1/3] wifi: mac80211: add ieee80211_tx_peek API

[Johannes Berg]

On Mon, 2026-07-20 at 09:08 +0200, Jose Ignacio Tornos Martinez wrote:
> 
> +const struct sk_buff *ieee80211_tx_peek(struct ieee80211_hw *hw,
> +					struct ieee80211_txq *txq)

While it makes some sense, I don't think I like the way you've
implemented this.

First, it basically does a lot of the work twice, say if it has to walk
a lot of flows to find the SKB.

Second, the work that it does is almost entirely disjoint from
ieee80211_tx_dequeue(). The latter doesn't even fill the hash (and not
all drivers might care), although I'm not even sure it's _needed_ to do
skb_get_hash() there, seems like this fairly obviously happens *after*
ieee80211_select_queue() or ieee80211_select_queue_80211(). So maybe
this isn't strictly more work than what dequeue() would do, because it's
a nop anyway.
Also though it does a LOT less work, although it pretends that the SKB
is fully formed, at least it makes no representations otherwise, and
that's obviously not true. Most things in the ieee80211_tx_info are not
filled in, kind of randomly by where and when mac80211 decided to do
that (which isn't really random, of course, but might as well be random
to the driver). As such, it seems only useful in a very limited subset
of cases.

Third, it documents that the skb will be returned by a subsequent
ieee80211_tx_dequeue() call, but that's obviously not true - the next
such call might not return anything at all, even for quite a while if
queues are stopped etc. which isn't considered in peek(), and I'm not
even convinced that it must ever be returned, if something else happened
in the meantime while such a situation occurred (queues stopped). Even
ieee80211_tx_dequeue() itself can drop SKBs entirely.

As such I'm not even convinced that there's a guarantee that after the
skb peek you even have done the work you claim to have done, if a
different skb is returned from dequeue() instead of this one.


I think I sort of understand the motivation, but it feels like you need
to go back to the drawing board. Maybe even further back, and perhaps
ath11k/ath12k just shouldn't use the hash of the SKB itself, but
something else that can be derived without the SKB hash?

If that's not feasible then this probably needs to be turned upside
down; perhaps there's some way to dequeue it anyway and hold it
somewhere for later (though that has issues with all the pointers in the
skb->cb), I don't have a good idea though.

johannes