Search Linux Wireless

Re: zd-mac80211: Fix TX status reports.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Michael Buesch wrote:

> Automatic rate scaling does not work on the zd-mac80211 driver.
> The driver does not properly report succeed and failed frames
> to mac80211.
> 
> We need to indicate failed frames with the excessive_retries field
> (Should we also fake report some high retry count here?)
> Otherwise the rc algo is not able to scale down.
> 
> Remove the conditional in REQ_TX_STATUS, as mac80211 handles that internally.
> 
> Signed-off-by: Michael Buesch <mb@xxxxxxxxx>

Just a general remark: The ZD1211 device doesn't support TX status
reporting directly. The driver emulates this by getting ACK
packets and transmission failures reported. The driver simply
reports them in sequence to transmitted packets without having a
reliable way to verify that the ACK packet or the failure report
actually belongs to the packet for which the status is reported.
This is error-prone. A typical problem are several correct ACKs
received for the same packet. In such a situation the driver loses
the synchronization of status and transmitted packets. If the
mac80211 stack uses the returned status for more than statistics,
this may cause bugs.

The mac80211 stack currently requires the driver to support
semantics it has no way to support correctly without harming
performance. (There is the option to transmit one packet and wait
a fixed time to report the success of the packet.)

It should also be noted that requiring the device to report ACK
packets to the host takes USB bandwidth away and increases also
the interrupt load on the host. There is a measurable impact
depending on the type of USB interface and the speed of the host
processor.

An alternative option could be that the mac80211 stack would call
a non-atomic function of the driver to report the total count of
transmitted packets and the number of successful transmissions.
The driver could even use counters maintained by the device to
support this. In specific cases where a status is required and
performance is not an issue (association) a reliable status report
mode could be supported, but this would add complexity to the
driver. I understand that this would require invasive changes to
the mac80211 stack, but I believe this change would be the right
thing.

> Johannes, to me it seems that there's also a bug in mac80211.
> I never get a frame with the REQ_TX_STATUS bit set, so frames
> will always end up on the "unreliable" tx status queue.

It appears that the patch tries to fix this by ignoring
REQ_TX_STATUS. See below.

> @@ -356,19 +357,18 @@ static int init_tx_skb_control_block(str
>   * If no status information has been requested, the skb is freed.
>   */
>  static void tx_status(struct ieee80211_hw *hw, struct sk_buff *skb,
> -	              struct ieee80211_tx_status *status)
> +	              struct ieee80211_tx_status *status,
> +		      bool success)
>  {
>  	struct zd_tx_skb_control_block *cb = (struct zd_tx_skb_control_block *)
>  		skb->cb;
>  
>  	ZD_ASSERT(cb->control != NULL);
> -	if ((cb->control->flags & IEEE80211_TXCTL_REQ_TX_STATUS)) {
> -		memcpy(&status->control, cb->control, sizeof(status->control));
> -		clear_tx_skb_control_block(skb);
> -		ieee80211_tx_status_irqsafe(hw, skb, status);
> -	} else {
> -		kfree_tx_skb(skb);
> -	}
> +	memcpy(&status->control, cb->control, sizeof(status->control));
> +	if (!success)
> +		status->excessive_retries = 1;
> +	clear_tx_skb_control_block(skb);
> +	ieee80211_tx_status_irqsafe(hw, skb, status);
>  }


-- 
Uli Kunitz
-
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux