diff options
| author |   stsp <stsp@openbsd.org> | 2020-07-20 08:09:30 +0000 |
|---|---|---|
| committer | stsp <stsp@openbsd.org> | 2020-07-20 08:09:30 +0000 |
| commit | 5c60d790042c0bf731339a08dc5704bb6c57e1fb (patch) | |
| tree | 9523f4e29e72171f2e74f0a2f235a31577ad274b /sys | |
| parent | update a comment in iwn_ampdu_tx_done following previous SSN -> SEQ change (diff) | |
| download | wireguard-openbsd-5c60d790042c0bf731339a08dc5704bb6c57e1fb.tar.xz wireguard-openbsd-5c60d790042c0bf731339a08dc5704bb6c57e1fb.zip | |
Fix a logic bug in iwn(4) which meant that automatic rate control for
A-MPDU ran while a fixed Tx MCS was configured. The intention was of
course the inverse: Use automatic rate control if Tx MCS is not fixed.
There is a second bug which went unnoticed because of the above bug.
The value of 'seq' provided by firmware may be smaller than ba_winstart.
When this happened it triggered a KASSERT in ieee80211_output_ba_record_ack.
iwn_ampdu_rate_control() implicitly assumes that its 'seq' parameter equals
ba_winstart, so just pass that value directly.
tested by bket@, jmc@, and myself
ok mpi@
Diffstat (limited to 'sys')
| -rw-r--r-- | sys/dev/pci/if_iwn.c | 11 |
1 files changed, 7 insertions, 4 deletions
diff --git a/sys/dev/pci/if_iwn.c b/sys/dev/pci/if_iwn.c index 87036afc6b0..25180341892 100644 --- a/sys/dev/pci/if_iwn.c +++ b/sys/dev/pci/if_iwn.c @@ -1,4 +1,4 @@ -/* $OpenBSD: if_iwn.c,v 1.236 2020/07/20 08:04:41 stsp Exp $ */ +/* $OpenBSD: if_iwn.c,v 1.237 2020/07/20 08:09:30 stsp Exp $ */ /*- * Copyright (c) 2007-2010 Damien Bergamini <damien.bergamini@free.fr> @@ -2379,19 +2379,22 @@ iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_rx_desc *desc, * Multiple BA notifications in a row may be using this number, with * additional bits being set in cba->bitmap. It is unclear how the * firmware decides to shift this window forward. + * We rely on ba->ba_winstart instead. */ seq = le16toh(cba->seq) >> IEEE80211_SEQ_SEQ_SHIFT; /* * The firmware's new BA window starting sequence number * corresponds to the first hole in cba->bitmap, implying - * that all frames between 'seq' and 'ssn' have been acked. + * that all frames between 'seq' and 'ssn' (non-inclusive) + * have been acked. */ ssn = le16toh(cba->ssn); /* Skip rate control if our Tx rate is fixed. */ - if (ic->ic_fixed_mcs != -1) - iwn_ampdu_rate_control(sc, ni, txq, cba->tid, seq, ssn); + if (ic->ic_fixed_mcs == -1) + iwn_ampdu_rate_control(sc, ni, txq, cba->tid, ba->ba_winstart, + ssn); /* * SSN corresponds to the first (perhaps not yet transmitted) frame |