On 2020 Dec 07 (Mon) at 15:02:14 +0100 (+0100), Stefan Sperling wrote:
:On Mon, Dec 07, 2020 at 02:36:05PM +0100, Tobias Heider wrote:
:> 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?
:
:I am happy with this if it works well during testing.
:
:Could someone convince phessler@ to take this for a flight in Minecraft?
:
Works very well in my testing.
With all three of these diffs in my tree, playing in Minecraft over wifi
feels a lot closer to playing over wired. In game taking off to fly is a
lot easier, and I have far fewer rubber-band incidents.
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;
:>
:
--
If the King's English was good enough for Jesus, it's good enough for me!
-- "Ma" Ferguson, Governor of Texas (circa 1920)
