Index | Thread | Search

From:
Marcus Glocker <marcus@nazgul.ch>
Subject:
Re: sys/qwz: handle QoS in native WiFi frames
To:
"Kirill A. Korinsky" <kirill@korins.ky>
Cc:
tech@openbsd.org
Date:
Sat, 23 May 2026 13:29:27 +0200

Download raw body.

Thread
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 <marcus@nazgul.ch> 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
>