-----Message d'origine-----
De : Johannes Berg [off-list ref]
Envoyé : vendredi 26 novembre 2021 13:05
À : Jean-Pierre TOSONI [off-list ref]; 'linux-
wireless@vger.kernel.org' [off-list ref]
Objet : Re: [RFC v4] mac80211: fix rx blockack session race condition

On Thu, 2021-10-28 at 10:25 +0000, Jean-Pierre TOSONI wrote:

quoted

+		spin_lock_bh(&rx->sta->ampdu_mlme.rx_offl_lock);
 		if (!test_bit(tid, rx->sta->ampdu_mlme.agg_session_valid)

&&

quoted

-		    !test_and_set_bit(tid, rx->sta-
ampdu_mlme.unexpected_agg))
+		    /* back_req is allowed if the fw just received addba */

If we're going to add an unconditional lock here, I see little reason to
have all the test_bit()/test_and_set_bit() etc.

The lock specifically protects the successive testing of agg_session_valid
and tid_rx_manage_offl. So I could just isolate the agg_session_valid 
condition as major condition then take the spinlock then test&set
agg_session_valid and tid_rx_manage_offl as a minor condition, so the
spinlock would be conditional. But see below.

I really don't think it's _right_ to add an unconditional lock here
though, if it can at all be avoided.

johannes

After extensive testing, adding this lock was effective but only removed
half of the DELBA frames wrongly sent. I suspect there is another race
condition somewhere else.

Thinking about it, here the DELBA is sent because the ath10k driver
passes a BAREQ before the signaling the establishment of the BA session.
either the firmware sent the two events in wrong order, or there was
actually an inversion in the frames sent by the host (in my testing,
Wireshark shows the first case only).

In the first case, we should not send a DELBA at all, because actually,
the session is correctly set up. The second case should be detected
and acted upon by ath10k itself since it was responsible for acking the
ADDBA; hence mac80211 should not send a -duplicate- DELBA either.

So, I completely removed this call to ieee80211_send_delba() when
working with ath10k, and my RX BA sessions  do not break anymore.
(I added my own hardware flag IEEE80211_HW_RX_AMPDU_IN_HW
similar to TX)

With my hardware it works, but I have no idea how to identify
chipsets/drivers/firmwares that will handle this correctly. So I
cannot submit a generic patch.

Please close this request, I will stay with my proprietary fix. Hopefully
someone with more ath10k/firmware knowledge will find a complete
way to fix.

Thanks for the review,
Jean-Pierre