Re: [PATCH 14/30] wifi: mac80211: put filtered data frames back on TXQs

["Toke Høiland-Jørgensen" <toke@toke.dk>]

Johannes Berg <johannes@sipsolutions.net> writes:

> From: Johannes Berg <johannes.berg@intel.com>
>
> When a device filters frames because the station went to
> sleep, we put them back on tx_filtered or pending queues.
> This can mix up ordering between TXQs and filtered frames
> when the station wakes up again later.
>
> Put filtered frames onto the frags queue of the TXQ they
> came from, fixing the order and simplifying the queueing
> structure in mac80211.
>
> Assisted-by: LLM
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  net/mac80211/ht.c          |  5 ++++
>  net/mac80211/ieee80211_i.h |  5 +++-
>  net/mac80211/sta_info.c    | 52 +++++++++++++++++++++++++++++++++++-
>  net/mac80211/sta_info.h    |  4 ++-
>  net/mac80211/status.c      | 54 +++++++++++++++++++++++++++++++++++++-
>  net/mac80211/tx.c          | 37 ++++++++++++++++++++++++++
>  6 files changed, 153 insertions(+), 4 deletions(-)
>
> diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
> index c5c2113fa4f7..e26d12659942 100644
> --- a/net/mac80211/ht.c
> +++ b/net/mac80211/ht.c
> @@ -372,6 +372,11 @@ void ieee80211_ba_session_work(struct wiphy *wiphy, struct wiphy_work *work)
>  			/* Allow only frags to be dequeued */
>  			set_bit(IEEE80211_TXQ_STOP, &txqi->flags);
>  
> +			/*
> +			 * Fragments and filtered frames (which already have a
> +			 * sequence number) must be out before aggregation
> +			 * starts, otherwise they'd be sent as part of it.
> +			 */
>  			if (!skb_queue_empty(&txqi->frags)) {
>  				/* Fragmented Tx is ongoing, wait for it to
>  				 * finish. Reschedule worker to retry later.
> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> index cb947341fee6..4a9b68022250 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -987,7 +987,8 @@ enum txq_info_flags {
>   * @tin: contains packets split into multiple flows
>   * @def_cvars: codel vars for the @tin's default_flow
>   * @cstats: code statistics for this queue
> - * @frags: used to keep fragments created after dequeue
> + * @frags: used to keep fragments created after dequeue, and frames that
> + *	were filtered by the device (queued before the fragments)

Hmm, so rather than splicing these two together in one queue and having
all that logic to insert into the middle, and to walk the queue to check
flags, isn't it simpler to just have two distinct queues as struct
members and check them in order on dequeue?

-Toke