From: Kevin Lo Subject: Re: qwx(4) crypto offloading To: tech@openbsd.org Date: Wed, 29 May 2024 11:26:49 +0800 On Wed, May 29, 2024 at 12:10:13AM +0200, Stefan Sperling wrote: > > On Mon, May 27, 2024 at 05:28:27PM +0200, Stefan Sperling wrote: > > WEP can be fixed with the change below. However, it breaks WPA2 for > > some reason. I don't know if WPA2 not working in RAW mode is a driver > > or a firmware issue. But I will give up now. I plan to write a new diff > > which offloads all of the ciphers because that seems easier on this crazy > > hardware. > > This diff makes WEP and TKIP work, however both types of networks > still suffer from the broadcast Rx issue. I haven't found a way to > avoid that. I am running out of time, so this is the best I can do. Confirmed that WEP works now. qwx0: flags=808843 mtu 1500 lladdr 00:0e:8e:xx:xx:xx index 2 priority 4 llprio 3 groups: wlan egress media: IEEE802.11 autoselect (OFDM48 mode 11a) status: active ieee80211: nwid puffy chan 36 bssid 00:0a:52:xx:xx:xx 0dBm nwkey inet 10.0.0.232 netmask 0xff000000 broadcast 10.255.255.255 Thank you for your hard work!!! > ok? ok kevlo@ > ----------------------------------------------- > repair qwx(4) WEP and TKIP via software crypto > > It is difficult to make WEP and WPA1/TKIP work with hardware crypto. > Add a comment which explains why. > > Ensure that setkey task state is properly cleared when the interface > goes down. This issue was found while trying to add WEP keys for hw > crypto, but is still worth fixing in general. > > Also, use m_makespace to append trailing padding for the MIC when > hardware crypto is used in combination with "raw" frame mode (not > the default), instead of blindly adjusting m_len. > > diff f475ad27b09c6d0a71ec6c04c2d3bf6f36e667ce 6741a8ff1ba30932c37c7b6a84839f402ed1e969 > commit - f475ad27b09c6d0a71ec6c04c2d3bf6f36e667ce > commit + 6741a8ff1ba30932c37c7b6a84839f402ed1e969 > blob - 91500360ed82a3c741f15f029e37b72d879210d4 > blob + 4d3ed3ff5d82894fe280c95f447dff6e25c199bb > --- sys/dev/ic/qwx.c > +++ sys/dev/ic/qwx.c > @@ -157,6 +157,7 @@ int qwx_wmi_vdev_install_key(struct qwx_softc *, > struct wmi_vdev_install_key_arg *, uint8_t); > int qwx_dp_peer_rx_pn_replay_config(struct qwx_softc *, struct qwx_vif *, > struct ieee80211_node *, struct ieee80211_key *, int); > +void qwx_setkey_clear(struct qwx_softc *); > > int qwx_scan(struct qwx_softc *); > void qwx_scan_abort(struct qwx_softc *); > @@ -183,7 +184,45 @@ qwx_init(struct ifnet *ifp) > struct ieee80211com *ic = &sc->sc_ic; > > sc->fw_mode = ATH11K_FIRMWARE_MODE_NORMAL; > - sc->crypto_mode = ATH11K_CRYPT_MODE_HW; > + /* > + * There are several known hardware/software crypto issues > + * on wcn6855 devices, firmware 0x1106196e. It is unclear > + * if these are driver or firmware bugs. > + * > + * 1) Broadcast/Multicast frames will only be received on > + * encrypted networks if hardware crypto is used and a > + * CCMP group key is used. Otherwise such frames never > + * even trigger an interrupt. This breaks ARP and IPv6. > + * This issue is known to affect the Linux ath11k vendor > + * driver when software crypto mode is selected. > + * Workaround: Use hardware crypto on WPA2 networks. > + * However, even with hardware crypto broadcast frames > + * are never received if TKIP is used as the WPA2 group > + * cipher and we have no workaround for this. > + * > + * 2) Adding WEP keys for hardware crypto crashes the firmware. > + * Presumably, lack of WEP support is deliberate because the > + * Linux ath11k vendor driver rejects attempts to install > + * WEP keys to hardware. > + * Workaround: Use software crypto if WEP is enabled. > + * This suffers from the broadcast issues mentioned above. > + * > + * 3) A WPA1 group key handshake message from the AP is never > + * received if hardware crypto is used. > + * Workaround: Use software crypto if WPA1 is enabled. > + * This suffers from the broadcast issues mentioned above, > + * even on WPA2 networks when WPA1 and WPA2 are both enabled. > + * On OpenBSD, WPA1 is disabled by default. > + * > + * The only known fully working configurations are unencrypted > + * networks, and WPA2/CCMP-only networks provided WPA1 remains > + * disabled. > + */ > + if ((ic->ic_flags & IEEE80211_F_WEPON) || > + (ic->ic_rsnprotos & IEEE80211_PROTO_WPA)) > + sc->crypto_mode = ATH11K_CRYPT_MODE_SW; > + else > + sc->crypto_mode = ATH11K_CRYPT_MODE_HW; > sc->frame_mode = ATH11K_HW_TXRX_NATIVE_WIFI; > ic->ic_state = IEEE80211_S_INIT; > sc->ns_nstate = IEEE80211_S_INIT; > @@ -291,6 +330,8 @@ qwx_stop(struct ifnet *ifp) > qwx_del_task(sc, systq, &sc->setkey_task); > refcnt_finalize(&sc->task_refs, "qwxstop"); > > + qwx_setkey_clear(sc); > + > clear_bit(ATH11K_FLAG_CRASH_FLUSH, sc->sc_flags); > > ifp->if_timer = sc->sc_tx_timer = 0; > @@ -529,8 +570,8 @@ qwx_set_key(struct ieee80211com *ic, struct ieee80211_ > struct qwx_softc *sc = ic->ic_softc; > > if (test_bit(ATH11K_FLAG_HW_CRYPTO_DISABLED, sc->sc_flags) || > - (k->k_cipher != IEEE80211_CIPHER_CCMP && > - k->k_cipher != IEEE80211_CIPHER_TKIP)) > + k->k_cipher == IEEE80211_CIPHER_WEP40 || > + k->k_cipher == IEEE80211_CIPHER_WEP104) > return ieee80211_set_key(ic, ni, k); > > return qwx_queue_setkey_cmd(ic, ni, k, QWX_ADD_KEY); > @@ -543,8 +584,8 @@ qwx_delete_key(struct ieee80211com *ic, struct ieee802 > struct qwx_softc *sc = ic->ic_softc; > > if (test_bit(ATH11K_FLAG_HW_CRYPTO_DISABLED, sc->sc_flags) || > - (k->k_cipher != IEEE80211_CIPHER_CCMP && > - k->k_cipher != IEEE80211_CIPHER_TKIP)) { > + k->k_cipher == IEEE80211_CIPHER_WEP40 || > + k->k_cipher == IEEE80211_CIPHER_WEP104) { > ieee80211_delete_key(ic, ni, k); > return; > } > @@ -757,6 +798,24 @@ qwx_setkey_task(void *arg) > splx(s); > } > > +void > +qwx_setkey_clear(struct qwx_softc *sc) > +{ > + struct ieee80211com *ic = &sc->sc_ic; > + struct qwx_setkey_task_arg *a; > + > + while (sc->setkey_nkeys > 0) { > + a = &sc->setkey_arg[sc->setkey_tail]; > + ieee80211_release_node(ic, a->ni); > + a->ni = NULL; > + sc->setkey_tail = (sc->setkey_tail + 1) % > + nitems(sc->setkey_arg); > + sc->setkey_nkeys--; > + } > + memset(sc->setkey_arg, 0, sizeof(sc->setkey_arg)); > + sc->setkey_cur = sc->setkey_tail = sc->setkey_nkeys = 0; > +} > + > int > qwx_newstate(struct ieee80211com *ic, enum ieee80211_state nstate, int arg) > { > @@ -773,21 +832,11 @@ qwx_newstate(struct ieee80211com *ic, enum ieee80211_s > nstate != IEEE80211_S_AUTH) > return 0; > if (ic->ic_state == IEEE80211_S_RUN) { > - struct qwx_setkey_task_arg *a; > #if 0 > qwx_del_task(sc, systq, &sc->ba_task); > #endif > qwx_del_task(sc, systq, &sc->setkey_task); > - while (sc->setkey_nkeys > 0) { > - a = &sc->setkey_arg[sc->setkey_tail]; > - ieee80211_release_node(ic, a->ni); > - a->ni = NULL; > - sc->setkey_tail = (sc->setkey_tail + 1) % > - nitems(sc->setkey_arg); > - sc->setkey_nkeys--; > - } > - memset(sc->setkey_arg, 0, sizeof(sc->setkey_arg)); > - sc->setkey_cur = sc->setkey_tail = sc->setkey_nkeys = 0; > + qwx_setkey_clear(sc); > #if 0 > qwx_del_task(sc, systq, &sc->bgscan_done_task); > #endif > @@ -16021,13 +16070,15 @@ qwx_dp_rx_h_reo_err(struct qwx_softc *sc, struct qwx_r > int > qwx_dp_rx_h_rxdma_err(struct qwx_softc *sc, struct qwx_rx_msdu *msdu) > { > + struct ieee80211com *ic = &sc->sc_ic; > int drop = 0; > #if 0 > ar->ab->soc_stats.rxdma_error[rxcb->err_code]++; > #endif > switch (msdu->err_code) { > case HAL_REO_ENTR_RING_RXDMA_ECODE_TKIP_MIC_ERR: > - drop = 1; /* OpenBSD uses TKIP in software crypto mode only */ > + ic->ic_stats.is_rx_locmicfail++; > + drop = 1; > break; > default: > /* TODO: Review other rxdma error code to check if anything is > @@ -24193,7 +24244,7 @@ qwx_dp_tx(struct qwx_softc *sc, struct qwx_vif *arvif, > void *hal_tcl_desc; > uint8_t pool_id; > uint8_t hal_ring_id; > - int ret, msdu_id; > + int ret, msdu_id, off; > uint32_t ring_selector = 0; > uint8_t ring_map = 0; > > @@ -24238,22 +24289,34 @@ qwx_dp_tx(struct qwx_softc *sc, struct qwx_vif *arvif, > if ((wh->i_fc[1] & IEEE80211_FC1_PROTECTED) && > ti.encap_type == HAL_TCL_ENCAP_TYPE_RAW) { > k = ieee80211_get_txkey(ic, wh, ni); > - switch (k->k_cipher) { > - case IEEE80211_CIPHER_CCMP: > - ti.encrypt_type = HAL_ENCRYPT_TYPE_CCMP_128; > - m->m_pkthdr.len += IEEE80211_CCMP_MICLEN; > - break; > - case IEEE80211_CIPHER_TKIP: > - ti.encrypt_type = HAL_ENCRYPT_TYPE_TKIP_MIC; > - m->m_pkthdr.len += IEEE80211_TKIP_MICLEN; > - break; > - default: > - /* Fallback to software crypto for other ciphers. */ > + if (test_bit(ATH11K_FLAG_HW_CRYPTO_DISABLED, sc->sc_flags)) { > ti.encrypt_type = HAL_ENCRYPT_TYPE_OPEN; > - break; > + } else { > + switch (k->k_cipher) { > + case IEEE80211_CIPHER_CCMP: > + ti.encrypt_type = HAL_ENCRYPT_TYPE_CCMP_128; > + if (m_makespace(m, m->m_pkthdr.len, > + IEEE80211_CCMP_MICLEN, &off) == NULL) { > + m_freem(m); > + return ENOSPC; > + } > + break; > + case IEEE80211_CIPHER_TKIP: > + ti.encrypt_type = HAL_ENCRYPT_TYPE_TKIP_MIC; > + if (m_makespace(m, m->m_pkthdr.len, > + IEEE80211_TKIP_MICLEN, &off) == NULL) { > + m_freem(m); > + return ENOSPC; > + } > + break; > + default: > + ti.encrypt_type = HAL_ENCRYPT_TYPE_OPEN; > + break; > + } > } > > if (ti.encrypt_type == HAL_ENCRYPT_TYPE_OPEN) { > + /* Using software crypto. */ > if ((m = ieee80211_encrypt(ic, m, k)) == NULL) > return ENOBUFS; > /* 802.11 header may have moved. */ >