From: Marcus Glocker Subject: Re: sys/qwz: handle QoS in native WiFi frames To: "Kirill A. Korinsky" Cc: tech@openbsd.org Date: Fri, 22 May 2026 21:13:04 +0200 On Mon, May 18, 2026 at 07:02:14PM +0200, Kirill A. Korinsky wrote: > tech@, > > I'd like to qebuild the QoS control field for native WiFi RX frames from > descriptor metadata before passing them to net80211. > > On TX, strip the hostside QoS control field when using native WiFi > encapsulation, after deriving the TID for the firmware descriptor. > > Unifi at least stop complaining that AP/Client signal balance is poor. > > Ok? In general the approach looks fine, and no regression found with my qwz. A few comments inline. > Index: sys/dev/ic/qwz.c > =================================================================== > RCS file: /cvs/src/sys/dev/ic/qwz.c,v > diff -u -p -r1.29 qwz.c > --- sys/dev/ic/qwz.c 18 May 2026 13:47:32 -0000 1.29 > +++ sys/dev/ic/qwz.c 18 May 2026 17:01:31 -0000 > @@ -14926,13 +14926,67 @@ void > qwz_dp_rx_h_undecap_nwifi(struct qwz_softc *sc, struct qwz_rx_msdu *msdu, > uint8_t *first_hdr, enum hal_encrypt_type enctype) > { > - /* > - * This function will need to do some work once we are receiving > - * aggregated frames. For now, it needs to do nothing. > - */ > + struct rx_mpdu_start_qcn9274 *mpdu; > + struct ieee80211_frame *wh; > + struct mbuf *m = msdu->m; > + uint8_t decap_hdr[IEEE80211_MAX_FRAME_HDR_LEN]; > + size_t hdrlen; > + uint16_t qos_ctl; > > - if (!msdu->is_first_msdu) > - printf("%s: not implemented\n", __func__); > + if (m == NULL) > + return; > + > + if (m->m_len < sizeof(*wh) && > + (m = m_pullup(m, sizeof(*wh))) == NULL) { > + msdu->m = NULL; > + return; > + } > + msdu->m = m; > + > + mpdu = &msdu->rx_desc->u.wcn7850.mpdu_start; > + wh = mtod(m, struct ieee80211_frame *); > + if ((mpdu->info6 & RX_MPDU_START_INFO6_NON_QOS) || > + ieee80211_has_qos(wh)) > + return; > + > + hdrlen = ieee80211_get_hdrlen(wh); > + if (hdrlen > sizeof(decap_hdr)) > + return; > + > + if (m->m_len < hdrlen && > + (m = m_pullup(m, hdrlen)) == NULL) { > + msdu->m = NULL; > + return; > + } > + msdu->m = m; > + > + wh = mtod(m, struct ieee80211_frame *); > + hdrlen = ieee80211_get_hdrlen(wh); > + if (hdrlen > sizeof(decap_hdr)) > + return; > + > + memcpy(decap_hdr, wh, hdrlen); > + wh = (struct ieee80211_frame *)decap_hdr; > + wh->i_fc[0] |= IEEE80211_FC0_SUBTYPE_QOS; > + wh->i_fc[1] &= ~IEEE80211_FC1_ORDER; > + qos_ctl = mpdu->qos_ctrl; The Linux ath12k driver is only rebuilding the qos_ctl field from the TID, plus applying the optional mesh bit: qos_ctl = rxcb->tid; if (rx_info->mesh_ctrl_present) qos_ctl |= IEEE80211_QOS_CTL_MESH_CONTROL_PRESENT; While you are copying the entire hardware captured QoS field. Is there a specific reason for that? For example, the hardware should already de-aggregate A-MSDU in to separate MSDUs, and a copied over A-MSDU bit could make net80211 mis-parse a plain MSDU as aggregated. > + > + m_adj(m, hdrlen); > + > + M_PREPEND(m, sizeof(qos_ctl), M_DONTWAIT); > + if (m == NULL) { > + msdu->m = NULL; > + return; > + } > + memcpy(mtod(m, void *), &qos_ctl, sizeof(qos_ctl)); > + > + M_PREPEND(m, hdrlen, M_DONTWAIT); > + if (m == NULL) { > + msdu->m = NULL; > + return; > + } > + msdu->m = m; > + memcpy(mtod(m, void *), decap_hdr, hdrlen); > } > > void > @@ -22907,6 +22961,46 @@ qwz_dp_tx_get_tid(struct mbuf *m) > return ieee80211_get_qos(wh) & IEEE80211_QOS_TID; > } > > +int > +qwz_dp_tx_encap_nwifi(struct mbuf **mp) > +{ > + struct mbuf *m = *mp; > + struct ieee80211_frame *wh; > + uint8_t *qos_ctl; > + int hdrlen; > + > + if (m->m_len < sizeof(*wh) && > + (m = m_pullup(m, sizeof(*wh))) == NULL) > + return ENOBUFS; You miss the mp = NULL, otherwise we have a double free on return. > + > + wh = mtod(m, struct ieee80211_frame *); > + if (!ieee80211_has_qos(wh)) { > + *mp = m; > + return 0; > + } > + > + hdrlen = ieee80211_get_hdrlen(wh); > + if (m->m_len < hdrlen && > + (m = m_pullup(m, hdrlen)) == NULL) > + return ENOBUFS; Same. > + > + wh = mtod(m, struct ieee80211_frame *); > + if (ieee80211_has_addr4(wh)) > + qos_ctl = ((struct ieee80211_qosframe_addr4 *)wh)->i_qos; > + else > + qos_ctl = ((struct ieee80211_qosframe *)wh)->i_qos; > + > + memmove(mtod(m, uint8_t *) + sizeof(uint16_t), mtod(m, uint8_t *), > + qos_ctl - mtod(m, uint8_t *)); > + m_adj(m, sizeof(uint16_t)); > + > + wh = mtod(m, struct ieee80211_frame *); > + wh->i_fc[0] &= ~IEEE80211_FC0_SUBTYPE_QOS; > + > + *mp = m; > + return 0; > +} > + > /* > * Build a WCN7850 / ath12k WiFi7 TCL data descriptor. Encap/encrypt/ > * search settings live in the bank addressed by ti->bank_id; the > @@ -23103,28 +23197,14 @@ qwz_dp_tx(struct qwz_softc *sc, struct q > ti.flags1 |= FIELD_PREP(HAL_TCL_DATA_CMD_INFO3_TID_OVERWRITE, 1); > > ti.tid = qwz_dp_tx_get_tid(m); > -#if 0 > - switch (ti.encap_type) { > - case HAL_TCL_ENCAP_TYPE_NATIVE_WIFI: > - ath12k_dp_tx_encap_nwifi(skb); > - break; > - case HAL_TCL_ENCAP_TYPE_RAW: > - if (!test_bit(ATH12K_FLAG_RAW_MODE, &ab->dev_flags)) { > - ret = -EINVAL; > - goto fail_remove_idr; > + if (ti.encap_type == HAL_TCL_ENCAP_TYPE_NATIVE_WIFI) { > + ret = qwz_dp_tx_encap_nwifi(&m); > + if (ret) { > + m_freem(m); > + return ret; > } > - break; > - case HAL_TCL_ENCAP_TYPE_ETHERNET: > - /* no need to encap */ > - break; > - case HAL_TCL_ENCAP_TYPE_802_3: > - default: > - /* TODO: Take care of other encap modes as well */ > - ret = -EINVAL; > - atomic_inc(&ab->soc_stats.tx_err.misc_fail); > - goto fail_remove_idr; > } > -#endif > + > ret = bus_dmamap_load_mbuf(sc->sc_dmat, tx_data->map, > m, BUS_DMA_WRITE | BUS_DMA_NOWAIT); > if (ret && ret != EFBIG) { > > -- > wbr, Kirill >