Hi,
our net80211 gapwait accounting implementation seems to have several
problems:
- If we lose packets with serial numbers 0 und 2 but receive the
packet with serial number 1, the first gap wait timeout will
skip serial number 0, flush out serial number 1 and then wait
for serial number 2. However, at this time the timeout is not
re-armed and we have to wait for a window slide.
- The logic that does gap skip if too many packets are in the reorder
buffer does not kick in if we lose two packets (as gapwait cannot
reach windowsize - 1. Additionally, what the logic does is mostly
equivalent to a normal window slide if we receive a packet that is
beyond the window. So remove this logic completely.
To fix this use ba_gapwait to actually count all the packets
currently in the reorder buffer and restart the gap timeout if the
buffer is not empty after we flush out some of the packets.
Found and fix by Christian Erhardt (CC).
ok?
Index: ieee80211_input.c
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v
retrieving revision 1.221
diff -u -p -r1.221 ieee80211_input.c
--- ieee80211_input.c 28 Aug 2020 12:01:48 -0000 1.221
+++ ieee80211_input.c 7 Dec 2020 12:58:31 -0000
@@ -839,30 +839,10 @@ ieee80211_input_ba(struct ieee80211com *
/* store Rx meta-data too */
rxi->rxi_flags |= IEEE80211_RXI_AMPDU_DONE;
ba->ba_buf[idx].rxi = *rxi;
+ ba->ba_gapwait++;
- if (ba->ba_buf[ba->ba_head].m == NULL) {
- if (ba->ba_gapwait < (ba->ba_winsize - 1)) {
- if (ba->ba_gapwait == 0) {
- timeout_add_msec(&ba->ba_gap_to,
- IEEE80211_BA_GAP_TIMEOUT);
- }
- ba->ba_gapwait++;
- } else {
- /*
- * A full BA window worth of frames is now waiting.
- * Skip the missing frame at the head of the window.
- */
- int skipped = ieee80211_input_ba_gap_skip(ba);
- ic->ic_stats.is_ht_rx_ba_frame_lost += skipped;
- ba->ba_gapwait = 0;
- if (timeout_pending(&ba->ba_gap_to))
- timeout_del(&ba->ba_gap_to);
- }
- } else {
- ba->ba_gapwait = 0;
- if (timeout_pending(&ba->ba_gap_to))
- timeout_del(&ba->ba_gap_to);
- }
+ if (ba->ba_buf[ba->ba_head].m == NULL && ba->ba_gapwait == 1)
+ timeout_add_msec(&ba->ba_gap_to, IEEE80211_BA_GAP_TIMEOUT);
ieee80211_input_ba_flush(ic, ni, ba, ml);
}
@@ -894,6 +874,7 @@ ieee80211_input_ba_seq(struct ieee80211c
ieee80211_inputm(ifp, ba->ba_buf[ba->ba_head].m,
ni, &ba->ba_buf[ba->ba_head].rxi, ml);
ba->ba_buf[ba->ba_head].m = NULL;
+ ba->ba_gapwait--;
} else
ic->ic_stats.is_ht_rx_ba_frame_lost++;
ba->ba_head = (ba->ba_head + 1) % IEEE80211_BA_MAX_WINSZ;
@@ -916,12 +897,18 @@ ieee80211_input_ba_flush(struct ieee8021
ieee80211_inputm(ifp, ba->ba_buf[ba->ba_head].m, ni,
&ba->ba_buf[ba->ba_head].rxi, ml);
ba->ba_buf[ba->ba_head].m = NULL;
+ ba->ba_gapwait--;
ba->ba_head = (ba->ba_head + 1) % IEEE80211_BA_MAX_WINSZ;
/* move window forward */
ba->ba_winstart = (ba->ba_winstart + 1) & 0xfff;
}
ba->ba_winend = (ba->ba_winstart + ba->ba_winsize - 1) & 0xfff;
+
+ if (timeout_pending(&ba->ba_gap_to))
+ timeout_del(&ba->ba_gap_to);
+ if (ba->ba_gapwait)
+ timeout_add_msec(&ba->ba_gap_to, IEEE80211_BA_GAP_TIMEOUT);
}
/*
@@ -989,6 +976,7 @@ ieee80211_ba_move_window(struct ieee8021
ieee80211_inputm(ifp, ba->ba_buf[ba->ba_head].m, ni,
&ba->ba_buf[ba->ba_head].rxi, ml);
ba->ba_buf[ba->ba_head].m = NULL;
+ ba->ba_gapwait--;
} else
ic->ic_stats.is_ht_rx_ba_frame_lost++;
ba->ba_head = (ba->ba_head + 1) % IEEE80211_BA_MAX_WINSZ;
