Re: [PATCH v2 3/4] mt76: mt76x02: fix tx status reporting issues

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3118 bytes --]

> When the hardware falls back to lower rates for a transmit attempt, only the
> first status report will show the number of retries correctly. The frames
> that follow will report the correct final rate, but number of retries set to 0.
> This can cause the rate control module to vastly underestimate the number of
> retransmissions per rate.
> 
> To fix this, we need to keep track of the initial requested tx rate per packet
> and pass it to the status information.
> For frames with tx status requested, this is simple: use the rate configured
> in info->control.rates[0] as reference.
> For no-skb tx status information, we have to encode the requested tx rate in
> the packet id (and make it possible to distinguish it from real packet ids).
> 
> To do that, reduce the packet id field size by one bit, and use that bit to
> indicate packet id vs rate.
> 
> This change also improves reporting by filling the status rate array with
> rates from first rate to final rate, taking the same steps as the hardware
> fallback table. This matters in corner cases like MCS8 on HT, where the
> fallback target is MCS0, not MCS7.
> 

[...]

> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_txrx.c b/drivers/net/wireless/mediatek/mt76/mt76x02_txrx.c
> index cf7abd9b7d2e..95c73049a68b 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_txrx.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_txrx.c
> @@ -164,9 +164,14 @@ int mt76x02_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
>  	mt76x02_mac_write_txwi(dev, txwi, tx_info->skb, wcid, sta, len);
>  
>  	pid = mt76_tx_status_skb_add(mdev, wcid, tx_info->skb);
> +
> +	/* encode packet rate for no-skb packet id to fix up status reporting */
> +	if (pid == MT_PACKET_ID_NO_SKB)
> +		pid = MT_PACKET_ID_HAS_RATE | (txwi->rate & MT_RXWI_RATE_INDEX);

I guess here we should do:
		pid = MT_PACKET_ID_HAS_RATE | (le16_to_cpu(txwi->rate) & MT_RXWI_RATE_INDEX);


> +
>  	txwi->pktid = pid;
>  
> -	if (pid >= MT_PACKET_ID_FIRST)
> +	if (mt76_is_skb_pktid(pid))
>  		qsel = MT_QSEL_MGMT;
>  
>  	tx_info->info = FIELD_PREP(MT_TXD_INFO_QSEL, qsel) |
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
> index 6b89f7eab26c..2436f14ca24a 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
> @@ -89,9 +89,14 @@ int mt76x02u_tx_prepare_skb(struct mt76_dev *mdev, void *data,
>  	skb_push(tx_info->skb, sizeof(*txwi));
>  
>  	pid = mt76_tx_status_skb_add(mdev, wcid, tx_info->skb);
> +
> +	/* encode packet rate for no-skb packet id to fix up status reporting */
> +	if (pid == MT_PACKET_ID_NO_SKB)
> +		pid = MT_PACKET_ID_HAS_RATE | (txwi->rate & MT_RXWI_RATE_INDEX);

same here

Regards,
Lorenzo

> +
>  	txwi->pktid = pid;
>  
> -	if (pid >= MT_PACKET_ID_FIRST || ep == MT_EP_OUT_HCCA)
> +	if (mt76_is_skb_pktid(pid) || ep == MT_EP_OUT_HCCA)
>  		qsel = MT_QSEL_MGMT;
>  	else
>  		qsel = MT_QSEL_EDCA;
> -- 
> 2.17.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]