On Mon, Dec 07, 2020 at 03:49:20PM +0100, Tobias Heider wrote:
> On Mon, Dec 07, 2020 at 02:33:10PM +0100, Stefan Sperling wrote:
> > On Mon, Dec 07, 2020 at 01:31:09PM +0100, Tobias Heider wrote:
> > > Some APs request a BA agreement and continue to send QOS packets
> > > for the same tid (with normal ack policy). Currently, these packets
> > > make it to the higher layers without going through BA reordering or the
> > > BA buffer. This results in reduced performance later on as the sequence
> > > numbers are expected by BA reordering.
> > >
> > > To fix this, we should use BA agreement immediately after it is
> > > requested by the AP. This causes the sequence number counter in
> > > the BA agreement to advance for the normal qos packets and the gap
> > > wait later on is avoided.
> > >
> > > ok?
> >
> > Not yet, see below:
> >
>
> Update with comments addressed.
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 14:38:24 -0000
> @@ -358,6 +358,17 @@ ieee80211_inputm(struct ifnet *ifp, stru
> /* go through A-MPDU reordering */
> ieee80211_input_ba(ic, m, ni, tid, rxi, ml);
> return; /* don't free m! */
> + } else if (ba_state == IEEE80211_BA_REQUESTED &&
> + (qos & IEEE80211_QOS_ACK_POLICY_MASK) ==
> + IEEE80211_QOS_ACK_POLICY_NORMAL) {
> + /*
> + * Apparently, qos frames for a tid where a
> + * block ack agreement was requested but not
> + * yet confirmed by us should still contribute
> + * to the sequence number for this tid.
> + */
> + ieee80211_input_ba(ic, m, ni, tid, rxi, ml);
> + return; /* don't free m! */
> }
> }
>
> @@ -2698,6 +2709,9 @@ ieee80211_recv_addba_req(struct ieee8021
> ssn = LE_READ_2(&frm[7]) >> 4;
>
> ba = &ni->ni_rx_ba[tid];
> + /* The driver is still processing an ADDBA request for this tid. */
> + if (ba->ba_state == IEEE80211_BA_REQUESTED)
> + return;
> /* check if we already have a Block Ack agreement for this RA/TID */
> if (ba->ba_state == IEEE80211_BA_AGREED) {
> /* XXX should we update the timeout value? */
> @@ -2737,7 +2751,7 @@ ieee80211_recv_addba_req(struct ieee8021
> goto refuse;
>
> /* setup Block Ack agreement */
> - ba->ba_state = IEEE80211_BA_INIT;
> + ba->ba_state = IEEE80211_BA_REQUESTED;
> ba->ba_timeout_val = timeout * IEEE80211_DUR_TU;
> ba->ba_ni = ni;
> ba->ba_token = token;
> @@ -2816,6 +2830,7 @@ ieee80211_addba_req_refuse(struct ieee80
> free(ba->ba_buf, M_DEVBUF,
> IEEE80211_BA_MAX_WINSZ * sizeof(*ba->ba_buf));
> ba->ba_buf = NULL;
> + ba->ba_state = IEEE80211_BA_INIT;
>
> /* MLME-ADDBA.response */
> IEEE80211_SEND_ACTION(ic, ni, IEEE80211_CATEG_BA,
>
