[RFC V3] bcm43xx-mac80211: Provide information to allow transmission rate decreases

[RFC V3] bcm43xx-mac80211: Provide information to allow transmission rate decreases

[Larry Finger]

Michael,

I couldn't find any long/short indication in the header, so I added a bool that
is set when the frame is sent.

Larry
---

In bcm43xx-mac80211, the mechanism for decreasing the transmit rate cannot
be triggered. This may be shown by walking away from the AP with a laptop.
At some distance, communications will be lost and never recovered because
the rate decreasing mechanism of rc80211_simple needs to see excessive_retries
set in the ieee80211_tx_status struct. With this patch, the transmit rate
will decrease until communications restart.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---

Index: wireless-dev/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_dma.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_dma.c
+++ wireless-dev/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_dma.c
@@ -1311,6 +1311,16 @@ void bcm43xx_dma_handle_txstatus(struct 
 			 */
 			if (status->acked)
 				meta->txstat.flags |= IEEE80211_TX_STATUS_ACK;
+			else
+				if (dev->last_frame_long) {
+					if (status->frame_count >=
+					    dev->long_retry_limit)
+						meta->txstat.excessive_retries = 1;
+				} else {
+					if (status->frame_count >=
+					    dev->short_retry_limit)
+						meta->txstat.excessive_retries = 1;
+				}
 			meta->txstat.retry_count = status->frame_count - 1;
 			ieee80211_tx_status_irqsafe(dev->wl->hw, meta->skb, &(meta->txstat));
 			/* skb is freed by ieee80211_tx_status_irqsafe() */
Index: wireless-dev/drivers/net/wireless/bcm43xx-mac80211/bcm43xx.h
===================================================================
--- wireless-dev.orig/drivers/net/wireless/bcm43xx-mac80211/bcm43xx.h
+++ wireless-dev/drivers/net/wireless/bcm43xx-mac80211/bcm43xx.h
@@ -707,6 +707,9 @@ struct bcm43xx_wldev {
 	bool short_preamble;		/* TRUE, if short preamble is enabled. */
 	bool short_slot;		/* TRUE, if short slot timing is enabled. */
 	bool radio_hw_enable;		/* saved state of radio hardware enabled state */
+	bool last_frame_long;		/* true is last frame was long */
+	u8 short_retry_limit;
+	u8 long_retry_limit;
 
 	/* PHY/Radio device. */
 	struct bcm43xx_phy phy;
Index: wireless-dev/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
+++ wireless-dev/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
@@ -3333,10 +3333,12 @@ static int bcm43xx_wireless_core_init(st
 	tmp = limit_value(modparam_short_retry, 0, 0xF);
 	bcm43xx_shm_write16(dev, BCM43xx_SHM_SCRATCH,
 			    BCM43xx_SHM_SC_SRLIMIT, tmp);
+	dev->short_retry_limit = tmp;
 	tmp = limit_value(modparam_long_retry, 0, 0xF);
 	bcm43xx_shm_write16(dev, BCM43xx_SHM_SCRATCH,
 			    BCM43xx_SHM_SC_LRLIMIT, tmp);
 
+	dev->long_retry_limit = tmp;
 	bcm43xx_shm_write16(dev, BCM43xx_SHM_SHARED,
 			    BCM43xx_SHM_SH_SFFBLIM, 3);
 	bcm43xx_shm_write16(dev, BCM43xx_SHM_SHARED,
Index: wireless-dev/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_xmit.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_xmit.c
+++ wireless-dev/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_xmit.c
@@ -264,6 +264,7 @@ static void generate_txhdr_fw4(struct bc
 	 * is a 5Ghz packet.
 	 */
 	txhdr->chan_radio_code = phy->channel;
+	dev->last_frame_long = 0;
 
 	/* PHY TX Control word */
 	if (rate_ofdm)
@@ -336,6 +337,7 @@ static void generate_txhdr_fw4(struct bc
 		if (rts_rate_fb_ofdm)
 			extra_ft |= BCM43xx_TX4_EFT_RTSFBOFDM;
 		mac_ctl |= BCM43xx_TX4_MAC_LONGFRAME;
+		dev->last_frame_long = 1;
 	}
 
 	/* Magic cookie */

Re: [RFC V3] bcm43xx-mac80211: Provide information to allow transmission rate decreases

[Michael Buesch]

On Thursday 02 August 2007, Larry Finger wrote:
> Michael,
> 
> I couldn't find any long/short indication in the header, so I added a bool that
> is set when the frame is sent.

This is not going to work.

But we can do this differently.
You can do something like:

if (!status->acked && !tx_control->noack)
	excessive_retries = 1;

So we don't need to care about the retry count.

Anyway. I don't know why we need excessive_retries _at_ _all_.
The rc algorithm does already know if the frame succeed or failed
anyway.

/me shrugs

> Larry
> ---
> 
> In bcm43xx-mac80211, the mechanism for decreasing the transmit rate cannot
> be triggered. This may be shown by walking away from the AP with a laptop.
> At some distance, communications will be lost and never recovered because
> the rate decreasing mechanism of rc80211_simple needs to see excessive_retries
> set in the ieee80211_tx_status struct. With this patch, the transmit rate
> will decrease until communications restart.
> 
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
> 
> Index: wireless-dev/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_dma.c
> ===================================================================
> --- wireless-dev.orig/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_dma.c
> +++ wireless-dev/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_dma.c
> @@ -1311,6 +1311,16 @@ void bcm43xx_dma_handle_txstatus(struct 
>  			 */
>  			if (status->acked)
>  				meta->txstat.flags |= IEEE80211_TX_STATUS_ACK;
> +			else
> +				if (dev->last_frame_long) {
> +					if (status->frame_count >=
> +					    dev->long_retry_limit)
> +						meta->txstat.excessive_retries = 1;
> +				} else {
> +					if (status->frame_count >=
> +					    dev->short_retry_limit)
> +						meta->txstat.excessive_retries = 1;
> +				}
>  			meta->txstat.retry_count = status->frame_count - 1;
>  			ieee80211_tx_status_irqsafe(dev->wl->hw, meta->skb, &(meta->txstat));
>  			/* skb is freed by ieee80211_tx_status_irqsafe() */
> Index: wireless-dev/drivers/net/wireless/bcm43xx-mac80211/bcm43xx.h
> ===================================================================
> --- wireless-dev.orig/drivers/net/wireless/bcm43xx-mac80211/bcm43xx.h
> +++ wireless-dev/drivers/net/wireless/bcm43xx-mac80211/bcm43xx.h
> @@ -707,6 +707,9 @@ struct bcm43xx_wldev {
>  	bool short_preamble;		/* TRUE, if short preamble is enabled. */
>  	bool short_slot;		/* TRUE, if short slot timing is enabled. */
>  	bool radio_hw_enable;		/* saved state of radio hardware enabled state */
> +	bool last_frame_long;		/* true is last frame was long */
> +	u8 short_retry_limit;
> +	u8 long_retry_limit;
>  
>  	/* PHY/Radio device. */
>  	struct bcm43xx_phy phy;
> Index: wireless-dev/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
> ===================================================================
> --- wireless-dev.orig/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
> +++ wireless-dev/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_main.c
> @@ -3333,10 +3333,12 @@ static int bcm43xx_wireless_core_init(st
>  	tmp = limit_value(modparam_short_retry, 0, 0xF);
>  	bcm43xx_shm_write16(dev, BCM43xx_SHM_SCRATCH,
>  			    BCM43xx_SHM_SC_SRLIMIT, tmp);
> +	dev->short_retry_limit = tmp;
>  	tmp = limit_value(modparam_long_retry, 0, 0xF);
>  	bcm43xx_shm_write16(dev, BCM43xx_SHM_SCRATCH,
>  			    BCM43xx_SHM_SC_LRLIMIT, tmp);
>  
> +	dev->long_retry_limit = tmp;
>  	bcm43xx_shm_write16(dev, BCM43xx_SHM_SHARED,
>  			    BCM43xx_SHM_SH_SFFBLIM, 3);
>  	bcm43xx_shm_write16(dev, BCM43xx_SHM_SHARED,
> Index: wireless-dev/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_xmit.c
> ===================================================================
> --- wireless-dev.orig/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_xmit.c
> +++ wireless-dev/drivers/net/wireless/bcm43xx-mac80211/bcm43xx_xmit.c
> @@ -264,6 +264,7 @@ static void generate_txhdr_fw4(struct bc
>  	 * is a 5Ghz packet.
>  	 */
>  	txhdr->chan_radio_code = phy->channel;
> +	dev->last_frame_long = 0;
>  
>  	/* PHY TX Control word */
>  	if (rate_ofdm)
> @@ -336,6 +337,7 @@ static void generate_txhdr_fw4(struct bc
>  		if (rts_rate_fb_ofdm)
>  			extra_ft |= BCM43xx_TX4_EFT_RTSFBOFDM;
>  		mac_ctl |= BCM43xx_TX4_MAC_LONGFRAME;
> +		dev->last_frame_long = 1;
>  	}
>  
>  	/* Magic cookie */
> 
> 

Re: [RFC V3] bcm43xx-mac80211: Provide information to allow transmission rate decreases

[Larry Finger]

Michael Buesch wrote:
> On Thursday 02 August 2007, Larry Finger wrote:
>> Michael,
>>
>> I couldn't find any long/short indication in the header, so I added a bool that
>> is set when the frame is sent.
> 
> This is not going to work.
> 
> But we can do this differently.
> You can do something like:
> 
> if (!status->acked && !tx_control->noack)
> 	excessive_retries = 1;
> 
> So we don't need to care about the retry count.
> 
> Anyway. I don't know why we need excessive_retries _at_ _all_.
> The rc algorithm does already know if the frame succeed or failed
> anyway.

Yes, it does, but it only looks at the potential for reduction if excessive retries is set. Why? 
Because it was coded that way! As I said earlier, I don't want to get involved there.

I'll try to implement it as above.

Larry

Re: [RFC V3] bcm43xx-mac80211: Provide information to allow transmission rate decreases

[Larry Finger]

Michael Buesch wrote:
> On Thursday 02 August 2007, Larry Finger wrote:
>> Michael,
>>
>> I couldn't find any long/short indication in the header, so I added a bool that
>> is set when the frame is sent.
> 
> This is not going to work.
> 
> But we can do this differently.
> You can do something like:
> 
> if (!status->acked && !tx_control->noack)
> 	excessive_retries = 1;
> 
> So we don't need to care about the retry count.
> 
> Anyway. I don't know why we need excessive_retries _at_ _all_.
> The rc algorithm does already know if the frame succeed or failed
> anyway.

Is this what you had in mind?

Larry


@@ -1311,6 +1311,9 @@ void bcm43xx_dma_handle_txstatus(struct
                          */
                         if (status->acked)
                                 meta->txstat.flags |= IEEE80211_TX_STATUS_ACK;
+                       else
+                               if (!(meta->txstat.flags & IEEE80211_TXCTL_NO_ACK))
+                                       meta->txstat.excessive_retries = 1;
                         meta->txstat.retry_count = status->frame_count - 1;
                         ieee80211_tx_status_irqsafe(dev->wl->hw, meta->skb, &(meta->txstat));
                         /* skb is freed by ieee80211_tx_status_irqsafe() */

Re: [RFC V3] bcm43xx-mac80211: Provide information to allow transmission rate decreases

[Michael Buesch]

On Thursday 02 August 2007, Larry Finger wrote:
> Michael Buesch wrote:
> > On Thursday 02 August 2007, Larry Finger wrote:
> >> Michael,
> >>
> >> I couldn't find any long/short indication in the header, so I added a bool that
> >> is set when the frame is sent.
> > 
> > This is not going to work.
> > 
> > But we can do this differently.
> > You can do something like:
> > 
> > if (!status->acked && !tx_control->noack)
> > 	excessive_retries = 1;
> > 
> > So we don't need to care about the retry count.
> > 
> > Anyway. I don't know why we need excessive_retries _at_ _all_.
> > The rc algorithm does already know if the frame succeed or failed
> > anyway.
> 
> Is this what you had in mind?
> 
> Larry
> 
> 
> @@ -1311,6 +1311,9 @@ void bcm43xx_dma_handle_txstatus(struct
>                           */
>                          if (status->acked)
>                                  meta->txstat.flags |= IEEE80211_TX_STATUS_ACK;
> +                       else
> +                               if (!(meta->txstat.flags & IEEE80211_TXCTL_NO_ACK))
> +                                       meta->txstat.excessive_retries = 1;
>                          meta->txstat.retry_count = status->frame_count - 1;
>                          ieee80211_tx_status_irqsafe(dev->wl->hw, meta->skb, &(meta->txstat));
>                          /* skb is freed by ieee80211_tx_status_irqsafe() */


Yeah, looks good. If you correctly diff that up, I'll queue it up for
the next submission to John.