On Wed, Jul 01, 2020 at 06:14:50PM -0500, Tim Chase wrote:
> Just wanted to check back in if there's anything else I can get you
> to help diagnose this.
Please try this patch. It fixes the issue for me.
It looks like my CCMP offload patch for athn(4) broke client mode,
even though it would occasionally work if the stars were aligned right.
It depends on the value the AP chooses for this client's "association ID".
That value factors into the slot we use in the on-device key table.
This is the correct approach when we are acting as AP ourselves and
choose the association IDs for our clients. But in client mode we should
be using the first key table slot only.
I am sorry about this. I should have tested client mode better :-/
Regarding "athn needs cold boot to work":
Code comments I've seen in Linux suggest that the key table is not cleared
automatically on some athn devices. There is code in our driver which is
supposed to clear the on-device crypto key table when the interface comes up.
This code ran very close in time after a full device reset. I suspect it could
have run while the device wasn't fully initialized yet, and hence fail to
reset keys which were installed earlier, or write garbage to the key table.
Combined with the above bug this would lead to hardware using bad key table
entries which results in decryption failures.
This patch should fix that problem as well. It clears the table later in the
startup sequence, and clears on-device keys when the interface is put down.
I have tested this fix on AR9280 (PCI) and AR9271 (USB), both in client and
in hostap mode.
diff refs/heads/master refs/heads/athn-ccmpfix
blob - 3a28d87bc88a0e7b9ed6c873bd7a07682cc91a0b
blob + 1d739529d7d214bea314e50e847594dc01021a41
--- sys/dev/ic/ar5008.c
+++ sys/dev/ic/ar5008.c
@@ -811,12 +811,20 @@ ar5008_ccmp_decap(struct athn_softc *sc, struct mbuf *
/* Sanity checks to ensure this is really a key we installed. */
entry = (uintptr_t)k->k_priv;
if (k->k_flags & IEEE80211_KEY_GROUP) {
- if (k->k_id > IEEE80211_WEP_NKID ||
+ if (k->k_id >= IEEE80211_WEP_NKID ||
entry != k->k_id)
return 1;
- } else if (entry != IEEE80211_WEP_NKID +
- IEEE80211_AID(ni->ni_associd))
- return 1;
+ } else {
+#ifndef IEEE80211_STA_ONLY
+ if (ic->ic_opmode == IEEE80211_M_HOSTAP) {
+ if (entry != IEEE80211_WEP_NKID +
+ IEEE80211_AID(ni->ni_associd))
+ return 1;
+ } else
+#endif
+ if (entry != IEEE80211_WEP_NKID)
+ return 1;
+ }
/* Check that ExtIV bit is set. */
if (!(ivp[3] & IEEE80211_WEP_EXTIV))
blob - 40725b02c43b54e10a87de333acdfd3b8270534d
blob + f7aa77ba15cae787a42fdbffb8a9d9cd2d0226d2
--- sys/dev/ic/athn.c
+++ sys/dev/ic/athn.c
@@ -1037,12 +1037,17 @@ athn_set_key(struct ieee80211com *ic, struct ieee80211
}
if (!(k->k_flags & IEEE80211_KEY_GROUP)) {
- entry = IEEE80211_WEP_NKID + IEEE80211_AID(ni->ni_associd);
+#ifndef IEEE80211_STA_ONLY
+ if (ic->ic_opmode == IEEE80211_M_HOSTAP)
+ entry = IEEE80211_WEP_NKID +
IEEE80211_AID(ni->ni_associd);
+ else
+#endif
+ entry = IEEE80211_WEP_NKID;
if (entry >= sc->kc_entries - IEEE80211_WEP_NKID)
return ENOSPC;
} else {
entry = k->k_id;
- if (entry > IEEE80211_WEP_NKID)
+ if (entry >= IEEE80211_WEP_NKID)
return ENOSPC;
}
k->k_priv = (void *)entry;
@@ -3056,10 +3061,6 @@ athn_init(struct ifnet *ifp)
else
athn_config_pcie(sc);
- /* Reset HW key cache entries. */
- for (i = 0; i < sc->kc_entries; i++)
- athn_reset_key(sc, i);
-
ops->enable_antenna_diversity(sc);
#ifdef ATHN_BT_COEXISTENCE
@@ -3086,6 +3087,10 @@ athn_init(struct ifnet *ifp)
/* Enable Rx. */
athn_rx_start(sc);
+ /* Reset HW key cache entries. */
+ for (i = 0; i < sc->kc_entries; i++)
+ athn_reset_key(sc, i);
+
/* Enable interrupts. */
athn_enable_interrupts(sc);
@@ -3121,7 +3126,7 @@ athn_stop(struct ifnet *ifp, int disable)
{
struct athn_softc *sc = ifp->if_softc;
struct ieee80211com *ic = &sc->sc_ic;
- int qid;
+ int qid, i;
ifp->if_timer = sc->sc_tx_timer = 0;
ifp->if_flags &= ~IFF_RUNNING;
@@ -3158,6 +3163,10 @@ athn_stop(struct ifnet *ifp, int disable)
AR_WRITE_BARRIER(sc);
athn_set_rxfilter(sc, 0);
athn_stop_rx_dma(sc);
+
+ /* Reset HW key cache entries. */
+ for (i = 0; i < sc->kc_entries; i++)
+ athn_reset_key(sc, i);
athn_reset(sc, 0);
athn_init_pll(sc, NULL);
