public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Remi Pommarel <repk@triplefau.lt>
To: Pablo MARTIN-GOMEZ <pmartin-gomez@freebox.fr>
Cc: Johannes Berg <johannes@sipsolutions.net>,
	linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH wireless] wifi: mac80211: Fix ADDBA update when HW supports reordering
Date: Thu, 26 Feb 2026 16:49:31 +0100	[thread overview]
Message-ID: <aaBo7rnVkRFz3h-U@pilgrim> (raw)
In-Reply-To: <19061b55-a211-4448-8c49-ca8daa6d9d61@freebox.fr>

On Mon, Feb 23, 2026 at 02:25:13PM +0100, Pablo MARTIN-GOMEZ wrote:
> Hello,
> 
> On 23/02/2026 12:50, Johannes Berg wrote:
> > On Sun, 2026-02-22 at 17:06 +0100, Remi Pommarel wrote:
> >>>> That does make sense. However, if I understand correctly, it means that
> >>>> even if we end up storing the timeout for drivers that support
> >>>> reordering, a new IEEE80211_AMPDU_RX_UPDATE should still be introduced,
> >>>> right?
> >>>
> >>> I guess in order to do a no-op update that doesn't change the timeout,
> >>> we could? But not sure it's all worth it?
> >>
> >> I was going to say it would not be a no-op for a buf_size update but,
> >> if I understand correctly, even when SUPPORTS_REORDERING_BUFFER is not
> >> set the buf_size update is ignored completely and the reorder_buf is
> >> not resized yet a successful addba response is sent. Don't we need to
> >> check for buf_size change as well as timeout also?
> > 
> > I was going to say that I thought buf_size is not allowed to change, but
> > (re)reading the spec doesn't seem to bear that out.
> For once, the standard (802.11-2024) is really clear on this (10.25.2):
> 
> A block ack agreement may be modified by the originator by sending an
> ADDBA Request frame (see 11.5.2, except that MLME-ADDBA primitives are
> not used by the originator). All parameters of the agreement may be
> modified except for the TID. If the request is not successful, the
> existing agreement is not modified. If the request is successful, the
> behavior is as if a DELBA frame for the block ack agreement had been
> transmitted by the originator and received by the recipient immediately
> prior to the ADDBA Request frame.
> > 
> > I guess we could just unconditionally reject any updates. I'm not sure
> > now why we ever added the update in the first place. Perhaps some
> > implementation was doing no-op updates and failing on negative
> > responses?
> > 
> If the originator is compliant and trying to update, unconditionally
> rejecing any update shouldn't have any side-effects. But if the
> originator doesn't receive the ADDBA response, it is going to resend an
> ADDBA request; for the originator, it is just a retry, but for the
> recipient it's an update, so the originator is going to receive a non
> success. And unless the originator sends a DELBA "by default" at some
> point, it won't be able to create a new BA (for a given TID) with the
> recipient.
> [...]

I’m a bit concerned that ignoring all ADDBA updates might break STAs
with finicky aggregation implementations. For example, I’ve observed an
ath12k mesh point get stuck in a loop, repeatedly sending the same ADDBA
request (with unchanged timeout or buffer size) only for us to keep
reject it each time.

But I am, of course, unfortunately unable to reproduce that easily.

-- 
Remi

  reply	other threads:[~2026-02-26 16:23 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-17 10:36 [PATCH wireless] wifi: mac80211: Fix ADDBA update when HW supports reordering Remi Pommarel
2026-02-17 11:30 ` Johannes Berg
2026-02-17 13:05   ` Remi Pommarel
2026-02-17 13:59     ` Johannes Berg
2026-02-17 14:38       ` Remi Pommarel
2026-02-17 16:00         ` Johannes Berg
2026-02-22 16:06           ` Remi Pommarel
2026-02-23 11:50             ` Johannes Berg
2026-02-23 13:25               ` Pablo MARTIN-GOMEZ
2026-02-26 15:49                 ` Remi Pommarel [this message]
2026-02-26 16:28                   ` Johannes Berg
2026-02-17 15:30       ` Pablo MARTIN-GOMEZ

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aaBo7rnVkRFz3h-U@pilgrim \
    --to=repk@triplefau.lt \
    --cc=johannes@sipsolutions.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=pmartin-gomez@freebox.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox