John W. Linville | 7 Oct 21:14
Favicon

[RFC PATCH] rtl8187: do not report ACKs if USB Tx status is non-zero

The vendor-supplied driver treats a USB Tx failure as an un-ACKed frame.
I don't see why we shouldn't do the same thing -- hopefully this makes
rate-scaling algorithms behave sanely with the rtl8187 driver.

Thanks to Felix Fietkau <nbd@...> for suggesting this as an
option.

Signed-off-by: John W. Linville <linville@...>
---
This is currently untested -- anyone with rtl8187 bored enough to try
it? :-)

 drivers/net/wireless/rtl8187_dev.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/rtl8187_dev.c b/drivers/net/wireless/rtl8187_dev.c
index ca5deb6..ae21191 100644
--- a/drivers/net/wireless/rtl8187_dev.c
+++ b/drivers/net/wireless/rtl8187_dev.c
@@ -164,7 +164,12 @@ static void rtl8187_tx_cb(struct urb *urb)
 	skb_pull(skb, priv->is_rtl8187b ? sizeof(struct rtl8187b_tx_hdr) :
 					  sizeof(struct rtl8187_tx_hdr));
 	memset(&info->status, 0, sizeof(info->status));
-	info->flags |= IEEE80211_TX_STAT_ACK;
+	if (!(info->flags & IEEE80211_TX_CTL_NO_ACK)) {
+		if (!urb->status)
+			info->flags |= IEEE80211_TX_STAT_ACK;
+		else /* assume ACK not received */
+			info->status.excessive_retries = 1;
+	}
(Continue reading)

John W. Linville | 8 Oct 20:42
Favicon

Re: [RFC PATCH] rtl8187: do not report ACKs if USB Tx status is non-zero

On Tue, Oct 07, 2008 at 03:18:18PM -0400, John W. Linville wrote:
> The vendor-supplied driver treats a USB Tx failure as an un-ACKed frame.
> I don't see why we shouldn't do the same thing -- hopefully this makes
> rate-scaling algorithms behave sanely with the rtl8187 driver.
> 
> Thanks to Felix Fietkau <nbd@...> for suggesting this as an
> option.
> 
> Signed-off-by: John W. Linville <linville@...>
> ---
> This is currently untested -- anyone with rtl8187 bored enough to try
> it? :-)

AFAICT, this doesn't actually trigger -- at least, the device can
go into its typical failure mode (no frames ACKed until you force a
lower rate) without ever hitting the "assume ACK not received" clause.

I'll keep looking at it -- there are a couple of 'secrets' still
buried in the vendor driver (if I can stomach to keep looking at it)...

John
--

-- 
John W. Linville		Linux should be at the core
linville@...			of your literate lifestyle.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html

(Continue reading)

"Stefanik =?iso | 8 Oct 21:38

Re: [RFC PATCH] rtl8187: do not report ACKs if USB Tx status is non-zero

On Wed, Oct 8, 2008 at 8:42 PM, John W. Linville <linville@...> wrote:
> On Tue, Oct 07, 2008 at 03:18:18PM -0400, John W. Linville wrote:
>> The vendor-supplied driver treats a USB Tx failure as an un-ACKed frame.
>> I don't see why we shouldn't do the same thing -- hopefully this makes
>> rate-scaling algorithms behave sanely with the rtl8187 driver.
>>
>> Thanks to Felix Fietkau <nbd@...> for suggesting this as an
>> option.
>>
>> Signed-off-by: John W. Linville <linville@...>
>> ---
>> This is currently untested -- anyone with rtl8187 bored enough to try
>> it? :-)
>
> AFAICT, this doesn't actually trigger -- at least, the device can
> go into its typical failure mode (no frames ACKed until you force a
> lower rate) without ever hitting the "assume ACK not received" clause.
>
> I'll keep looking at it -- there are a couple of 'secrets' still
> buried in the vendor driver (if I can stomach to keep looking at it)...
>
> John
> --
> John W. Linville                Linux should be at the core
> linville@...                  of your literate lifestyle.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@...
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
(Continue reading)

John W. Linville | 8 Oct 22:02
Favicon

Re: [RFC PATCH] rtl8187: do not report ACKs if USB Tx status is non-zero

On Wed, Oct 08, 2008 at 09:38:49PM +0200, Stefanik Gábor wrote:

> Another weird thing: the code above essentially attempts to do this:
> Wait for an ACK
> If not TX_CTL_NO_ACK:
>    If acked:
>       Report the packet as acked.
>    Endif
>    If no ACK until timeout:
>       Report packet as unacked
>    Endif
> Endif
> 
> This would be better, as it doesn't waste time waiting for an ACK for
> unacked frames:
> 
> If not TX_CTL_NO_ACK:
> Wait for an ACK
>    If acked:
>       Report the packet as acked.
>    Endif
>    If no ACK until timeout:
>       Report packet as unacked
>    Endif
> Endif
> 
> Of course, this only works if "Wait for an ACK" actually works.

Well as I understand it, the only waiting is related to the URB
submission (which is asynchronous anyway).  I don't really see how
(Continue reading)

Favicon

Re: [RFC PATCH] rtl8187: do not report ACKs if USB Tx status is non-zero

On Wednesday 08 October 2008 15:42:05 John W. Linville wrote:
> On Tue, Oct 07, 2008 at 03:18:18PM -0400, John W. Linville wrote:
> > The vendor-supplied driver treats a USB Tx failure as an un-ACKed frame.
> > I don't see why we shouldn't do the same thing -- hopefully this makes
> > rate-scaling algorithms behave sanely with the rtl8187 driver.
> >
> > Thanks to Felix Fietkau <nbd@...> for suggesting this as an
> > option.
> >
> > Signed-off-by: John W. Linville <linville@...>
> > ---
> > This is currently untested -- anyone with rtl8187 bored enough to try
> > it? :-)
>
> AFAICT, this doesn't actually trigger -- at least, the device can
> go into its typical failure mode (no frames ACKed until you force a
> lower rate) without ever hitting the "assume ACK not received" clause.

I tested here and it didn't trigger indeed, here at least urb->status == 0 
always no matter what I tried, which seems normal, only would trigger at some 
not typical usb failure? But there is one thing that I like about the patch, 
seems correct that we shouldn't set IEEE80211_TX_STAT_ACK if 
IEEE80211_TX_CTL_NO_ACK, that is, looks like a bug always setting ack in the 
flags when no_ack is present, not sure if it has some effect in practice 
though (have to look more at the code that cares about info->flags). I would 
say that the patch 'as is' is correct and will not hurt.

>
> I'll keep looking at it -- there are a couple of 'secrets' still
> buried in the vendor driver (if I can stomach to keep looking at it)...
(Continue reading)


Gmane