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

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

[John W. Linville]

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@openwrt.org> for suggesting this as an
option.

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
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;
+	}
 	ieee80211_tx_status_irqsafe(hw, skb);
 }
 
-- 
1.5.4.3

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

[John W. Linville]

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@openwrt.org> for suggesting this as an
> option.
> 
> Signed-off-by: John W. Linville <linville@tuxdriver.com>
> ---
> 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@tuxdriver.com			of your literate lifestyle.

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

[Stefanik Gábor]

On Wed, Oct 8, 2008 at 8:42 PM, John W. Linville <linville@tuxdriver.com> 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@openwrt.org> for suggesting this as an
>> option.
>>
>> Signed-off-by: John W. Linville <linville@tuxdriver.com>
>> ---
>> 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@tuxdriver.com                  of your literate lifestyle.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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.

-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

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

[John W. Linville]

On Wed, Oct 08, 2008 at 09:38:49PM +0200, Stefanik G=E1bor 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
>=20
> This would be better, as it doesn't waste time waiting for an ACK for
> unacked frames:
>=20
> 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
>=20
> 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
we could avoid it.

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

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

[Herton Ronaldo Krzesinski]

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@openwrt.org> for suggesting this as an
> > option.
> >
> > Signed-off-by: John W. Linville <linville@tuxdriver.com>
> > ---
> > 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)...

yeah, not very straightforward to grasp the vendor driver unfortunately... 
good would be to have some times docs/explanation about some of the values 
used, when you're luck there is some comments and references at some places.

>
> John

--
[]'s
Herton