From: Marcus Glocker Subject: Re: sys/qwz: handle QoS in native WiFi frames To: "Kirill A. Korinsky" Cc: tech@openbsd.org Date: Sat, 23 May 2026 13:29:27 +0200 On Sat, May 23, 2026 at 12:29:27PM +0200, Kirill A. Korinsky wrote: > On Fri, 22 May 2026 21:13:04 +0200, > Marcus Glocker wrote: > > > > > + 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. > > Thanks! It was leftover from very agressive tests, cleaned up. > > I also added missed *mp = NULL, thanks. > > And on my setup it Unifi stop says that that AP/Client signal balance is > poor, and I don't see this errors on boot anymore: > > qwz0: failed to setup rxd tid queue for tid 2: 12 > qwz0: failed to setup dp for peer b6:fb:e4:9b:30:cd on vdev 0 (12) > > Ok? Much better thanks. One last nit, comment in-line. With that included ok mglocker@ > Index: sys/dev/ic/qwz.c > =================================================================== > RCS file: /home/cvs/src/sys/dev/ic/qwz.c,v > diff -u -p -r1.33 qwz.c > --- sys/dev/ic/qwz.c 19 May 2026 21:15:21 -0000 1.33 > +++ sys/dev/ic/qwz.c 23 May 2026 10:27:30 -0000 > @@ -15116,13 +15116,68 @@ 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 (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) || We should use 'le32toh(mpdu->info6)' here. > + 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; > > - if (!msdu->is_first_msdu) > - printf("%s: not implemented\n", __func__); > + 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 = htole16(sc->hal_rx_ops->rx_desc_get_mpdu_tid(msdu->rx_desc) & > + IEEE80211_QOS_TID); > + > + 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 > @@ -23076,6 +23131,50 @@ 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) { > + *mp = NULL; > + return ENOBUFS; > + } > + > + 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) { > + *mp = NULL; > + return ENOBUFS; > + } > + > + 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 > @@ -23272,28 +23371,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 >