2010-11-13 18:14:09

by Ivo Van Doorn

[permalink] [raw]
Subject: [PATCH 1/9] rt2x00: Increase REGISTER_BUSY_COUNT

For some hardware the REGISTER_BUSY_COUNT isn't sufficient,
increase the REGISTER_BUSY_COUNT to 100 to catch most
devices which have more problems with accessing the registers.

For normal operating devices nothing would change as they will
exit the loop early anyway.

Signed-off-by: Ivo van Doorn <[email protected]>
Acked-by: Helmut Schaa <[email protected]>
---
drivers/net/wireless/rt2x00/rt2x00.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
index 42bd3a9..0a55eef 100644
--- a/drivers/net/wireless/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/rt2x00/rt2x00.h
@@ -915,7 +915,7 @@ struct rt2x00_dev {
* in those cases REGISTER_BUSY_COUNT attempts should be
* taken with a REGISTER_BUSY_DELAY interval.
*/
-#define REGISTER_BUSY_COUNT 5
+#define REGISTER_BUSY_COUNT 100
#define REGISTER_BUSY_DELAY 100

/*
--
1.7.2.3



2010-11-22 07:00:58

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH 9/9] rt2x00: Modify rt2x00queue_remove_l2pad to make skb->data two-byte alignment

On 11/17/10 17:41, Helmut Schaa wrote:
> Am Mittwoch 17 November 2010 schrieb Gertjan van Wingerde:
>> On Wed, Nov 17, 2010 at 4:07 PM, John W. Linville
>> <[email protected]> wrote:
>>> On Wed, Nov 17, 2010 at 11:48:04AM +0100, Helmut Schaa wrote:
>>>
>>>> John, should I send a follow-up (with a nice description why it this is needed)
>>>> or are you simply reverting this one?
>>>
>>> Is there no chance for a fix in short order?
>
> I don't have a clever idea on how to fix that without reverting this commit.
> IMO this patch is just not correct as it passes the skb back to mac80211 with
> reduced headroom (due to the header & payload alignment) which causes trouble
> when mac80211 requeues the frame. Of course just requesting 4 additional bytes
> headroom would "fix" the symptoms but sounds like a hack to me.
>
>> I may have an idea on how we can fix this, without incurring the
>> performance penalty.
>
> I'm still not convinced that this is really the root cause for the
> performance issues Jay noticed. AFAIK mac80211 doesn't access the payload
> anymore when reporting the frame back (with some exceptions like monitor
> interfaces).
>
> Jay, could you please run a few more performance tests with and without this
> patch to track down if this issue is really the cause for the performance
> degradation?
>
>> Basic idea is to no longer work on the original skb that mac80211
>> supplied us, but to
>> use a copy of that skb. This would prevent us from having to undo any
>> changes we did,
>> as we can simply return the original skb to mac80211 (which wasn't
>> modified in the first
>> place).
>> I'm not sure how this would impact performance, but it would allow us
>> a lot less copying
>> around to undo the changes done before uploading to the HW.
>
> But cloning the skb would double the amount of memory needed to transmit each
> frame. Not sure though if that behaves better or not. Might be worth a try.
>
>> However, I won't be able to look into that opportunity before the weekend.
>>
>> Helmut, can you wait that long and hold off reverting until then?
>

OK. Find attached the patch I cooked up. AFAICS the driver still works correctly,
but unfortunately I am unable to test performance and throughput of the driver
with this patch.

Jay and Helmut, can you test this patch before I submit it?

---
Gertjan.


Attachments:
0001-rt2x00-Ensure-TX-ed-skbs-are-returned-to-mac80211-in.patch (4.97 kB)

2010-11-16 17:01:41

by Helmut Schaa

[permalink] [raw]
Subject: Re: [PATCH 7/9] rt2x00: Fix rt2800 USB TX Path DMA issue

Am Dienstag 16 November 2010 schrieb Johannes Stezenbach:
> On Tue, Nov 16, 2010 at 05:42:08PM +0100, Helmut Schaa wrote:
> > Am Dienstag 16 November 2010 schrieb Johannes Stezenbach:
> > >
> > > Hm, yes, rt2x00mac_conf_tx() has the same issue that it calls
> > > rt2x00queue_get_queue() with an argument which is a mac80211 AC,
> > > but expects an enum data_queue_qid. So one could says one
> > > bug cancels the other out.
> >
> > Right. We could of course add a mac80211-queue <-> rt2x00-queue mapping in
> > the appropriate places or rename the QID_* identifiers accordingly. Not sure
> > if it's worth it ...
>
> Hm, reducing confusion enhances maintainability.

I fully agree. I just meant that there are more severe issues in rt2x00 I plan
to work on. But feel free to send a patch :)

Helmut

2010-11-16 16:12:52

by Helmut Schaa

[permalink] [raw]
Subject: Re: [PATCH 7/9] rt2x00: Fix rt2800 USB TX Path DMA issue

Hi Johannes,

Am Dienstag 16 November 2010 schrieb Johannes Stezenbach:
> On Mon, Nov 15, 2010 at 01:45:24AM -0800, Walter Goldens wrote:
> >
> > These patches appear to have finally brought some life in to the rt307x chips
> > in terms of stability and usability. There are no more duplicate (very few)
> > or lost packets and overall, the driver is improved immensely, however the
> > latency is increased compared to the STA driver. Pinging my gateway with STA
> > yields about 2-3ms average, whereas the rt2800usb is about 8-9ms. The
> > throughput is also about 20% less.
>
> I'm runnung rt2800usb (RT3070) in AP mode, and when pinging
> from the client I get:
>
> [12631.727724] phy0 -> rt2x00usb_watchdog_tx_status: Warning - TX queue 2 status timed out, invoke forced tx handler
> [12632.727451] phy0 -> rt2x00usb_watchdog_tx_status: Warning - TX queue 2 status timed out, invoke forced tx handler
> [12638.728027] phy0 -> rt2x00usb_watchdog_tx_status: Warning - TX queue 2 status timed out, invoke forced tx handler
> [12674.732633] phy0 -> rt2x00usb_watchdog_tx_status: Warning - TX queue 0 status timed out, invoke forced tx handler
> [12676.732855] phy0 -> rt2x00usb_watchdog_tx_status: Warning - TX queue 0 status timed out, invoke forced tx handler
> [12679.733581] phy0 -> rt2x00usb_watchdog_tx_status: Warning - TX queue 0 status timed out, invoke forced tx handler
> [12779.745991] phy0 -> rt2x00usb_watchdog_tx_status: Warning - TX queue 0 status timed out, invoke forced tx handler
> etc.
>
> I wonder why it is using TX queue 2? Isn't this QID_AC_VI, i.e.
> it should only be used when some programs uses setsockopt SO_PRIORITY
> to request QoS for video data?
>
>
> Lokking through the code, in rt2x00mac_tx():
> enum data_queue_qid qid = skb_get_queue_mapping(skb);
> is passed to rt2x00queue_get_queue() which uses
> enum data_queue_qid has 2 == QID_AC_VI, but net/core/dev.c dev_pick_tx()
> calls skb_set_queue_mapping() with 2 == BE
> (from ieee802_1d_to_ac, return by ieee80211_netdev_select_queue())
>
> Maybe I'm just confused?

Yep, it is indeed confusing. If you compiled rt2x00 with debug output have a
look at the queue setup in dmesg:

[ 47.590000] phy0 -> rt2x00mac_conf_tx: Info - Configured TX queue 0 - CWmin: 3, CWmax: 4, Aifs: 2, TXop: 102.
[ 47.590000] phy0 -> rt2x00mac_conf_tx: Info - Configured TX queue 1 - CWmin: 4, CWmax: 5, Aifs: 2, TXop: 188.
[ 47.590000] phy0 -> rt2x00mac_conf_tx: Info - Configured TX queue 2 - CWmin: 5, CWmax: 10, Aifs: 3, TXop: 0.
[ 47.590000] phy0 -> rt2x00mac_conf_tx: Info - Configured TX queue 3 - CWmin: 5, CWmax: 10, Aifs: 7, TXop: 0.

As you can see queue 2 gets the parameters for AC_BE assigned whereas queue 0
gets AC_VO assigned. So the naming within rt2x00 is not consitent with the
numbering within mac80211. However, since we configure the queue parameters
this should really just be a naming issue.

And btw. queue 0 is used for managment frames as well.

HTH,
Helmut

2010-11-14 08:59:37

by Julian Calaby

[permalink] [raw]
Subject: Re: [PATCH 3/9] rt2x00: Clean up Kconfig for RT2800 devices.

IVD,

A minor nit:

On Sun, Nov 14, 2010 at 05:10, Ivo van Doorn <[email protected]> wrote:
> From: Gertjan van Wingerde <[email protected]>
>
> General clean up of the Kconfig part for RT28XX devices.
> Also remove the indications of non functional support for rt27xx/rt28xx/rt30xx
> devices, as this is no longer true. They just work fine.
> Finally, remove the experimental indications for rt27xx/rt28xx/rt30xx devices
> as that is no longer true. Keep the experimental indications for rt33xx/rt35xx
> devices, though.
>
> Signed-off-by: Gertjan van Wingerde <[email protected]>
> Acked-by: Helmut Schaa <[email protected]>
> Signed-off-by: Ivo van Doorn <[email protected]>
> ---
> ?drivers/net/wireless/rt2x00/Kconfig | ? 47 ++++++++++++++--------------------
> ?1 files changed, 19 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/Kconfig b/drivers/net/wireless/rt2x00/Kconfig
> index f0f0152..f2be1d3 100644
> --- a/drivers/net/wireless/rt2x00/Kconfig
> +++ b/drivers/net/wireless/rt2x00/Kconfig
> @@ -154,30 +150,24 @@ config RT2800USB
> ? ? ? ?select RT2X00_LIB_CRYPTO
> ? ? ? ?select CRC_CCITT
> ? ? ? ?---help---
> - ? ? ? ? This adds experimental support for rt2800 wireless chipset family.
> + ? ? ? ? This adds support for rt27xx/rt28xx wireless chipset family.
> ? ? ? ? ?Supported chips: RT2770, RT2870 & RT3070.

Isn't RT3070 support selected below?

> - ? ? ? ? Known issues:
> - ? ? ? ? - support for RT2870 chips doesn't work with 802.11n APs yet
> - ? ? ? ? - support for RT3070 chips is non-functional at the moment
> -
> ? ? ? ? ?When compiled as a module, this driver will be called "rt2800usb.ko".
>
> ?if RT2800USB
>
> ?config RT2800USB_RT30XX
> - ? ? ? bool "rt2800usb - Include support for rt30xx (USB) devices"
> + ? ? ? bool "rt2800usb - Include support for rt30xx devices"
> ? ? ? ?default y
> ? ? ? ?---help---
> ? ? ? ? ?This adds support for rt30xx wireless chipset family to the
> ? ? ? ? ?rt2800usb driver.
> ? ? ? ? ?Supported chips: RT3070, RT3071 & RT3072

I.e. here?

> - ? ? ? ? Support for these devices is non-functional at the moment and is
> - ? ? ? ? intended for testers and developers.
> -
> ?config RT2800USB_RT33XX
> - ? ? ? bool "rt2800usb - Include support for rt33xx (USB) devices"
> + ? ? ? bool "rt2800usb - Include support for rt33xx devices (EXPERIMENTAL)"
> + ? ? ? depends on EXPERIMENTAL
> ? ? ? ?default n
> ? ? ? ?---help---
> ? ? ? ? ?This adds support for rt33xx wireless chipset family to the

Thanks,

--
Julian Calaby

Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

2010-11-17 08:47:06

by Helmut Schaa

[permalink] [raw]
Subject: Re: [PATCH 9/9] rt2x00: Modify rt2x00queue_remove_l2pad to make skb->data two-byte alignment

Am Dienstag 16 November 2010 schrieb Helmut Schaa:
> Am Samstag 13 November 2010 schrieb Ivo van Doorn:
> > From: RA-Jay Hung <[email protected]>
> >
> > When send out skb data to mac80211, orignal code will cause mac80211
> > unaligned access, so modify code to make mac80211 can natural access.
>
> And this introduces the same panic again :(
>
> The problem is the following:
>
> We don't pass the skb in the same state back to mac80211 as we got it.
>
> When inserting the l2pad we're moving the header and thus reduce headroom.
> This patch modifies the bahavior during l2pad removal to not move the header
> back into its old position but instead moves the payload. Thus the skb keeps
> the reduced headroom. If this skb gets requeued into rt2x00 (which can happen
> when the frame wasn't acked and the according STA is known to e in powersave
> mode) the header and payload get aligned again further reducing headroom which
> results in a too small headroom for the TXWI and thus a skb_under_panic.

Hmm, John merged that patch already. However, I would prefer if it would get
reverted due to the occasional panics in AP mode.

Jay, I didn't notice any performance degradation on MIPS, on which architecture
did you test?

Jay, Ivo, any objections against reverting this one?

Thanks,
Helmut

2010-11-13 18:14:15

by Ivo Van Doorn

[permalink] [raw]
Subject: [PATCH 4/9] rt2x00: Remove RT30XX Kconfig variables.

From: Gertjan van Wingerde <[email protected]>

Enabling of RT30xx devices via Kconfig variables was introduced when these
devices weren't properly supported yet.
Now that that they are properly supported and functional, we can remove these
Kconfig variables for RT30xx devices and simply enable them whenever rt2800pci
and/or rt2800usb is enabled.

Signed-off-by: Gertjan van Wingerde <[email protected]>
Acked-by: Helmut Schaa <[email protected]>
Signed-off-by: Ivo van Doorn <[email protected]>
---
drivers/net/wireless/rt2x00/Kconfig | 29 ++----
drivers/net/wireless/rt2x00/rt2800pci.c | 10 +-
drivers/net/wireless/rt2x00/rt2800usb.c | 167 ++++++++++++++-----------------
3 files changed, 85 insertions(+), 121 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/Kconfig b/drivers/net/wireless/rt2x00/Kconfig
index f2be1d3..a6939cc 100644
--- a/drivers/net/wireless/rt2x00/Kconfig
+++ b/drivers/net/wireless/rt2x00/Kconfig
@@ -64,7 +64,7 @@ config RT2800PCI_SOC
default y

config RT2800PCI
- tristate "Ralink rt27xx/rt28xx (PCI/PCIe/PCMCIA) support"
+ tristate "Ralink rt27xx/rt28xx/rt30xx (PCI/PCIe/PCMCIA) support"
depends on RT2800PCI_PCI || RT2800PCI_SOC
select RT2800_LIB
select RT2X00_LIB_PCI if RT2800PCI_PCI
@@ -75,21 +75,14 @@ config RT2800PCI
select CRC_CCITT
select EEPROM_93CX6
---help---
- This adds support for rt27xx/rt28xx wireless chipset family.
- Supported chips: RT2760, RT2790, RT2860, RT2880, RT2890 & RT3052
+ This adds support for rt27xx/rt28xx/rt30xx wireless chipset family.
+ Supported chips: RT2760, RT2790, RT2860, RT2880, RT2890, RT3052,
+ RT3090, RT3091 & RT3092

When compiled as a module, this driver will be called "rt2800pci.ko".

if RT2800PCI

-config RT2800PCI_RT30XX
- bool "rt2800pci - Include support for rt30xx devices"
- default y
- ---help---
- This adds support for rt30xx wireless chipset family to the
- rt2800pci driver.
- Supported chips: RT3090, RT3091 & RT3092
-
config RT2800PCI_RT33XX
bool "rt2800pci - Include support for rt33xx devices (EXPERIMENTAL)"
depends on EXPERIMENTAL
@@ -141,7 +134,7 @@ config RT73USB
When compiled as a module, this driver will be called rt73usb.

config RT2800USB
- tristate "Ralink rt27xx/rt28xx (USB) support"
+ tristate "Ralink rt27xx/rt28xx/rt30xx (USB) support"
depends on USB
select RT2800_LIB
select RT2X00_LIB_USB
@@ -150,21 +143,13 @@ config RT2800USB
select RT2X00_LIB_CRYPTO
select CRC_CCITT
---help---
- This adds support for rt27xx/rt28xx wireless chipset family.
- Supported chips: RT2770, RT2870 & RT3070.
+ This adds support for rt27xx/rt28xx/rt30xx wireless chipset family.
+ Supported chips: RT2770, RT2870 & RT3070, RT3071 & RT3072

When compiled as a module, this driver will be called "rt2800usb.ko".

if RT2800USB

-config RT2800USB_RT30XX
- bool "rt2800usb - Include support for rt30xx devices"
- default y
- ---help---
- This adds support for rt30xx wireless chipset family to the
- rt2800usb driver.
- Supported chips: RT3070, RT3071 & RT3072
-
config RT2800USB_RT33XX
bool "rt2800usb - Include support for rt33xx devices (EXPERIMENTAL)"
depends on EXPERIMENTAL
diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index 6642f13..97f4df6 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -1037,6 +1037,9 @@ static DEFINE_PCI_DEVICE_TABLE(rt2800pci_device_table) = {
{ PCI_DEVICE(0x1814, 0x0681), PCI_DEVICE_DATA(&rt2800pci_ops) },
{ PCI_DEVICE(0x1814, 0x0701), PCI_DEVICE_DATA(&rt2800pci_ops) },
{ PCI_DEVICE(0x1814, 0x0781), PCI_DEVICE_DATA(&rt2800pci_ops) },
+ { PCI_DEVICE(0x1814, 0x3090), PCI_DEVICE_DATA(&rt2800pci_ops) },
+ { PCI_DEVICE(0x1814, 0x3091), PCI_DEVICE_DATA(&rt2800pci_ops) },
+ { PCI_DEVICE(0x1814, 0x3092), PCI_DEVICE_DATA(&rt2800pci_ops) },
{ PCI_DEVICE(0x1432, 0x7708), PCI_DEVICE_DATA(&rt2800pci_ops) },
{ PCI_DEVICE(0x1432, 0x7727), PCI_DEVICE_DATA(&rt2800pci_ops) },
{ PCI_DEVICE(0x1432, 0x7728), PCI_DEVICE_DATA(&rt2800pci_ops) },
@@ -1044,13 +1047,8 @@ static DEFINE_PCI_DEVICE_TABLE(rt2800pci_device_table) = {
{ PCI_DEVICE(0x1432, 0x7748), PCI_DEVICE_DATA(&rt2800pci_ops) },
{ PCI_DEVICE(0x1432, 0x7758), PCI_DEVICE_DATA(&rt2800pci_ops) },
{ PCI_DEVICE(0x1432, 0x7768), PCI_DEVICE_DATA(&rt2800pci_ops) },
- { PCI_DEVICE(0x1a3b, 0x1059), PCI_DEVICE_DATA(&rt2800pci_ops) },
-#ifdef CONFIG_RT2800PCI_RT30XX
- { PCI_DEVICE(0x1814, 0x3090), PCI_DEVICE_DATA(&rt2800pci_ops) },
- { PCI_DEVICE(0x1814, 0x3091), PCI_DEVICE_DATA(&rt2800pci_ops) },
- { PCI_DEVICE(0x1814, 0x3092), PCI_DEVICE_DATA(&rt2800pci_ops) },
{ PCI_DEVICE(0x1462, 0x891a), PCI_DEVICE_DATA(&rt2800pci_ops) },
-#endif
+ { PCI_DEVICE(0x1a3b, 0x1059), PCI_DEVICE_DATA(&rt2800pci_ops) },
#ifdef CONFIG_RT2800PCI_RT33XX
{ PCI_DEVICE(0x1814, 0x3390), PCI_DEVICE_DATA(&rt2800pci_ops) },
#endif
diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index 61852c5..2933bf1 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -641,11 +641,19 @@ static struct usb_device_id rt2800usb_device_table[] = {
/* Abocom */
{ USB_DEVICE(0x07b8, 0x2870), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x07b8, 0x2770), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x07b8, 0x3070), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x07b8, 0x3071), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x07b8, 0x3072), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x1482, 0x3c09), USB_DEVICE_DATA(&rt2800usb_ops) },
+ /* AirTies */
+ { USB_DEVICE(0x1eda, 0x2310), USB_DEVICE_DATA(&rt2800usb_ops) },
/* Allwin */
{ USB_DEVICE(0x8516, 0x2070), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x8516, 0x2770), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x8516, 0x2870), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x8516, 0x3070), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x8516, 0x3071), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x8516, 0x3072), USB_DEVICE_DATA(&rt2800usb_ops) },
/* Amit */
{ USB_DEVICE(0x15c5, 0x0008), USB_DEVICE_DATA(&rt2800usb_ops) },
/* Askey */
@@ -654,8 +662,13 @@ static struct usb_device_id rt2800usb_device_table[] = {
{ USB_DEVICE(0x0b05, 0x1731), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x0b05, 0x1732), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x0b05, 0x1742), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0b05, 0x1784), USB_DEVICE_DATA(&rt2800usb_ops) },
/* AzureWave */
{ USB_DEVICE(0x13d3, 0x3247), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x13d3, 0x3273), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x13d3, 0x3305), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x13d3, 0x3307), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x13d3, 0x3321), USB_DEVICE_DATA(&rt2800usb_ops) },
/* Belkin */
{ USB_DEVICE(0x050d, 0x8053), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x050d, 0x805c), USB_DEVICE_DATA(&rt2800usb_ops) },
@@ -666,6 +679,7 @@ static struct usb_device_id rt2800usb_device_table[] = {
{ USB_DEVICE(0x14b2, 0x3c06), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x14b2, 0x3c07), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x14b2, 0x3c09), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x14b2, 0x3c12), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x14b2, 0x3c23), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x14b2, 0x3c25), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x14b2, 0x3c27), USB_DEVICE_DATA(&rt2800usb_ops) },
@@ -674,113 +688,27 @@ static struct usb_device_id rt2800usb_device_table[] = {
{ USB_DEVICE(0x07aa, 0x002f), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x07aa, 0x003c), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x07aa, 0x003f), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* D-Link */
- { USB_DEVICE(0x07d1, 0x3c09), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x07d1, 0x3c11), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Edimax */
- { USB_DEVICE(0x7392, 0x7717), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x7392, 0x7718), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* EnGenius */
- { USB_DEVICE(0x1740, 0x9701), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x1740, 0x9702), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Gigabyte */
- { USB_DEVICE(0x1044, 0x800b), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Hawking */
- { USB_DEVICE(0x0e66, 0x0001), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x0e66, 0x0003), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x0e66, 0x0009), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x0e66, 0x000b), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x0e66, 0x0013), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x0e66, 0x0017), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x0e66, 0x0018), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Linksys */
- { USB_DEVICE(0x1737, 0x0070), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x1737, 0x0071), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Logitec */
- { USB_DEVICE(0x0789, 0x0162), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x0789, 0x0163), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x0789, 0x0164), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Motorola */
- { USB_DEVICE(0x100d, 0x9031), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* MSI */
- { USB_DEVICE(0x0db0, 0x6899), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Philips */
- { USB_DEVICE(0x0471, 0x200f), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Planex */
- { USB_DEVICE(0x2019, 0xed06), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Ralink */
- { USB_DEVICE(0x148f, 0x2770), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x148f, 0x2870), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Samsung */
- { USB_DEVICE(0x04e8, 0x2018), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Siemens */
- { USB_DEVICE(0x129b, 0x1828), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Sitecom */
- { USB_DEVICE(0x0df6, 0x0017), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x0df6, 0x002b), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x0df6, 0x002c), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x0df6, 0x002d), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x0df6, 0x0039), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x0df6, 0x003b), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x0df6, 0x003d), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x0df6, 0x003f), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* SMC */
- { USB_DEVICE(0x083a, 0x6618), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x083a, 0x7512), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x083a, 0x7522), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x083a, 0x8522), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x083a, 0xa618), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x083a, 0xb522), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Sparklan */
- { USB_DEVICE(0x15a9, 0x0006), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Sweex */
- { USB_DEVICE(0x177f, 0x0302), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* U-Media*/
- { USB_DEVICE(0x157e, 0x300e), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* ZCOM */
- { USB_DEVICE(0x0cde, 0x0022), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x0cde, 0x0025), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Zinwell */
- { USB_DEVICE(0x5a57, 0x0280), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x5a57, 0x0282), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Zyxel */
- { USB_DEVICE(0x0586, 0x3416), USB_DEVICE_DATA(&rt2800usb_ops) },
-#ifdef CONFIG_RT2800USB_RT30XX
- /* Abocom */
- { USB_DEVICE(0x07b8, 0x3070), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x07b8, 0x3071), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x07b8, 0x3072), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* AirTies */
- { USB_DEVICE(0x1eda, 0x2310), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Allwin */
- { USB_DEVICE(0x8516, 0x3070), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x8516, 0x3071), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x8516, 0x3072), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* ASUS */
- { USB_DEVICE(0x0b05, 0x1784), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* AzureWave */
- { USB_DEVICE(0x13d3, 0x3273), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x13d3, 0x3305), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x13d3, 0x3307), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x13d3, 0x3321), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Conceptronic */
- { USB_DEVICE(0x14b2, 0x3c12), USB_DEVICE_DATA(&rt2800usb_ops) },
- /* Corega */
{ USB_DEVICE(0x18c5, 0x0012), USB_DEVICE_DATA(&rt2800usb_ops) },
/* D-Link */
+ { USB_DEVICE(0x07d1, 0x3c09), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x07d1, 0x3c0a), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x07d1, 0x3c0d), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x07d1, 0x3c0e), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x07d1, 0x3c0f), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x07d1, 0x3c11), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x07d1, 0x3c16), USB_DEVICE_DATA(&rt2800usb_ops) },
/* Draytek */
{ USB_DEVICE(0x07fa, 0x7712), USB_DEVICE_DATA(&rt2800usb_ops) },
/* Edimax */
{ USB_DEVICE(0x7392, 0x7711), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x7392, 0x7717), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x7392, 0x7718), USB_DEVICE_DATA(&rt2800usb_ops) },
/* Encore */
{ USB_DEVICE(0x203d, 0x1480), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x203d, 0x14a9), USB_DEVICE_DATA(&rt2800usb_ops) },
/* EnGenius */
+ { USB_DEVICE(0x1740, 0x9701), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x1740, 0x9702), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x1740, 0x9703), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x1740, 0x9705), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x1740, 0x9706), USB_DEVICE_DATA(&rt2800usb_ops) },
@@ -788,19 +716,37 @@ static struct usb_device_id rt2800usb_device_table[] = {
{ USB_DEVICE(0x1740, 0x9708), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x1740, 0x9709), USB_DEVICE_DATA(&rt2800usb_ops) },
/* Gigabyte */
+ { USB_DEVICE(0x1044, 0x800b), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x1044, 0x800d), USB_DEVICE_DATA(&rt2800usb_ops) },
+ /* Hawking */
+ { USB_DEVICE(0x0e66, 0x0001), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0e66, 0x0003), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0e66, 0x0009), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0e66, 0x000b), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0e66, 0x0013), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0e66, 0x0017), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0e66, 0x0018), USB_DEVICE_DATA(&rt2800usb_ops) },
/* I-O DATA */
{ USB_DEVICE(0x04bb, 0x0945), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x04bb, 0x0947), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x04bb, 0x0948), USB_DEVICE_DATA(&rt2800usb_ops) },
+ /* Linksys */
+ { USB_DEVICE(0x1737, 0x0070), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x1737, 0x0071), USB_DEVICE_DATA(&rt2800usb_ops) },
/* Logitec */
+ { USB_DEVICE(0x0789, 0x0162), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0789, 0x0163), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0789, 0x0164), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x0789, 0x0166), USB_DEVICE_DATA(&rt2800usb_ops) },
+ /* Motorola */
+ { USB_DEVICE(0x100d, 0x9031), USB_DEVICE_DATA(&rt2800usb_ops) },
/* MSI */
{ USB_DEVICE(0x0db0, 0x3820), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x0db0, 0x3821), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x0db0, 0x3822), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x0db0, 0x3870), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x0db0, 0x3871), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0db0, 0x6899), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x0db0, 0x821a), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x0db0, 0x822a), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x0db0, 0x822b), USB_DEVICE_DATA(&rt2800usb_ops) },
@@ -815,30 +761,65 @@ static struct usb_device_id rt2800usb_device_table[] = {
/* Pegatron */
{ USB_DEVICE(0x1d4d, 0x000c), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x1d4d, 0x000e), USB_DEVICE_DATA(&rt2800usb_ops) },
+ /* Philips */
+ { USB_DEVICE(0x0471, 0x200f), USB_DEVICE_DATA(&rt2800usb_ops) },
/* Planex */
{ USB_DEVICE(0x2019, 0xab25), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x2019, 0xed06), USB_DEVICE_DATA(&rt2800usb_ops) },
/* Quanta */
{ USB_DEVICE(0x1a32, 0x0304), USB_DEVICE_DATA(&rt2800usb_ops) },
/* Ralink */
{ USB_DEVICE(0x148f, 0x2070), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x148f, 0x2770), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x148f, 0x2870), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x148f, 0x3070), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x148f, 0x3071), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x148f, 0x3072), USB_DEVICE_DATA(&rt2800usb_ops) },
+ /* Samsung */
+ { USB_DEVICE(0x04e8, 0x2018), USB_DEVICE_DATA(&rt2800usb_ops) },
+ /* Siemens */
+ { USB_DEVICE(0x129b, 0x1828), USB_DEVICE_DATA(&rt2800usb_ops) },
/* Sitecom */
+ { USB_DEVICE(0x0df6, 0x0017), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0df6, 0x002b), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0df6, 0x002c), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0df6, 0x002d), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0df6, 0x0039), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0df6, 0x003b), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0df6, 0x003d), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x0df6, 0x003e), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0df6, 0x003f), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x0df6, 0x0040), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x0df6, 0x0042), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x0df6, 0x0047), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x0df6, 0x0048), USB_DEVICE_DATA(&rt2800usb_ops) },
/* SMC */
+ { USB_DEVICE(0x083a, 0x6618), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x083a, 0x7511), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x083a, 0x7512), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x083a, 0x7522), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x083a, 0x8522), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x083a, 0xa618), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x083a, 0xa701), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x083a, 0xa702), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x083a, 0xa703), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x083a, 0xb522), USB_DEVICE_DATA(&rt2800usb_ops) },
+ /* Sparklan */
+ { USB_DEVICE(0x15a9, 0x0006), USB_DEVICE_DATA(&rt2800usb_ops) },
+ /* Sweex */
+ { USB_DEVICE(0x177f, 0x0302), USB_DEVICE_DATA(&rt2800usb_ops) },
+ /* U-Media*/
+ { USB_DEVICE(0x157e, 0x300e), USB_DEVICE_DATA(&rt2800usb_ops) },
+ /* ZCOM */
+ { USB_DEVICE(0x0cde, 0x0022), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x0cde, 0x0025), USB_DEVICE_DATA(&rt2800usb_ops) },
/* Zinwell */
+ { USB_DEVICE(0x5a57, 0x0280), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x5a57, 0x0282), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x5a57, 0x0283), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x5a57, 0x5257), USB_DEVICE_DATA(&rt2800usb_ops) },
-#endif
+ /* Zyxel */
+ { USB_DEVICE(0x0586, 0x3416), USB_DEVICE_DATA(&rt2800usb_ops) },
#ifdef CONFIG_RT2800USB_RT33XX
/* Ralink */
{ USB_DEVICE(0x148f, 0x3370), USB_DEVICE_DATA(&rt2800usb_ops) },
--
1.7.2.3


2010-11-16 02:17:32

by RA-Jay Hung

[permalink] [raw]
Subject: RE: [PATCH 7/9] rt2x00: Fix rt2800 USB TX Path DMA issue

Hi,

> These patches appear to have finally brought some life in to the rt307x chips in
> terms of stability and usability. There are no more duplicate (very few) or lost
> packets and overall, the driver is improved immensely, however the latency is
> increased compared to the STA driver. Pinging my gateway with STA yields about
> 2-3ms average, whereas the rt2800usb is about 8-9ms. The throughput is also
> about 20% less. Do you think this can be corrected at some point and do you
> anticipate further improvements in this direction?

Yes, we will continue improve functionally and throughput of Ralink chips, thanks.

jay
CONFIDENTIALITY STATEMENT : The information, attachments and any rights attaching in this e-mail are confidential and privileged; it is intended only for the individual or entity named as the recipient hereof.Any disclosure, copying, distribution, dissemination or use of the contents of this e-mail by persons other than the intended recipient is STRICTLY PROHIBITED and may violate applicable laws.If you have received this e-mail in error, please delete the original message and notify us by return email or collect call immediately. Thank you.

2010-11-14 09:00:55

by Julian Calaby

[permalink] [raw]
Subject: Re: [PATCH 3/9] rt2x00: Clean up Kconfig for RT2800 devices.

On Sun, Nov 14, 2010 at 19:59, Julian Calaby <[email protected]> wrote:
> IVD,
>
> A minor nit:

Disregard this, I hadn't read patch #4 =)

Thanks,

--
Julian Calaby

Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

2010-11-22 08:14:45

by RA-Jay Hung

[permalink] [raw]
Subject: RE: [PATCH 9/9] rt2x00: Modify rt2x00queue_remove_l2pad to make skb->data two-byte alignment

Hi,
> > Jay, could you please run a few more performance tests with and without this
> > patch to track down if this issue is really the cause for the performance
> > degradation?

I check sniffer and mac80211 code, I think bad TX throughput should be my environment has a lot APs and more packets will collides because of without protection in TX direction. So the patch is not related to this issue. Helmut,
We can revert this patch. Sorry for inconvenience.

> >> Basic idea is to no longer work on the original skb that mac80211
> >> supplied us, but to
> >> use a copy of that skb. This would prevent us from having to undo any
> >> changes we did,
> >> as we can simply return the original skb to mac80211 (which wasn't
> >> modified in the first
> >> place).
> >> I'm not sure how this would impact performance, but it would allow us
> >> a lot less copying
> >> around to undo the changes done before uploading to the HW.
> >
> > But cloning the skb would double the amount of memory needed to transmit each
> > frame. Not sure though if that behaves better or not. Might be worth a try.
> >
> >> However, I won't be able to look into that opportunity before the weekend.
> >>
> >> Helmut, can you wait that long and hold off reverting until then?
> >
>
> OK. Find attached the patch I cooked up. AFAICS the driver still works correctly,
> but unfortunately I am unable to test performance and throughput of the driver
> with this patch.
>
> Jay and Helmut, can you test this patch before I submit it?
>
I think original code should recover the original skb state, so I think we do not need to copy again to send back to mac80211, and one more thing. Could you submit below
patch you send us before to rt2x00.git. I think it is more correct in payload = 0 case.

void rt2x00queue_remove_l2pad(struct sk_buff *skb, unsigned int header_length)
{
- unsigned int l2pad = L2PAD_SIZE(header_length);
+ unsigned int payload_length = skb->len - header_length;
+ unsigned int l2pad = payload_length ? L2PAD_SIZE(header_length) : 0;

if (!l2pad)
return;

Jay
CONFIDENTIALITY STATEMENT : The information, attachments and any rights attaching in this e-mail are confidential and privileged; it is intended only for the individual or entity named as the recipient hereof.Any disclosure, copying, distribution, dissemination or use of the contents of this e-mail by persons other than the intended recipient is STRICTLY PROHIBITED and may violate applicable laws.If you have received this e-mail in error, please delete the original message and notify us by return email or collect call immediately. Thank you.

2010-11-18 01:47:28

by David Ellingsworth

[permalink] [raw]
Subject: Re: [rt2x00-users] [PATCH 9/9] rt2x00: Modify rt2x00queue_remove_l2pad to make skb->data two-byte alignment

On Wed, Nov 17, 2010 at 11:41 AM, Helmut Schaa
<[email protected]> wrote:
> Am Mittwoch 17 November 2010 schrieb Gertjan van Wingerde:
>> On Wed, Nov 17, 2010 at 4:07 PM, John W. Linville
>> <[email protected]> wrote:
>> > On Wed, Nov 17, 2010 at 11:48:04AM +0100, Helmut Schaa wrote:
>> >
>> >> John, should I send a follow-up (with a nice description why it this is needed)
>> >> or are you simply reverting this one?
>> >
>> > Is there no chance for a fix in short order?
>
> I don't have a clever idea on how to fix that without reverting this commit.
> IMO this patch is just not correct as it passes the skb back to mac80211 with
> reduced headroom (due to the header & payload alignment) which causes trouble
> when mac80211 requeues the frame. Of course just requesting 4 additional bytes
> headroom would "fix" the symptoms but sounds like a hack to me.
>
>> I may have an idea on how we can fix this, without incurring the
>> performance penalty.
>
> I'm still not convinced that this is really the root cause for the
> performance issues Jay noticed. AFAIK mac80211 doesn't access the payload
> anymore when reporting the frame back (with some exceptions like monitor
> interfaces).
>
> Jay, could you please run a few more performance tests with and without this
> patch to track down if this issue is really the cause for the performance
> degradation?
>
>> Basic idea is to no longer work on the original skb that mac80211
>> supplied us, but to
>> use a copy of that skb. This would prevent us from having to undo any
>> changes we did,
>> as we can simply return the original skb to mac80211 (which wasn't
>> modified in the first
>> place).
>> I'm not sure how this would impact performance, but it would allow us
>> a lot less copying
>> around to undo the changes done before uploading to the HW.
>
> But cloning the skb would double the amount of memory needed to transmit each
> frame. Not sure though if that behaves better or not. Might be worth a try.

I saw this and the thought of using a circular buffer came to mind.
Any possibility of using one to copy and modify the skbs after
receiving them from mac80211 until passed to the hardware? It seems to
me that this would at least reduce the amount of memory consumed by
the driver needed to duplicate every skb. I honestly don't know much
about network drivers, so I might be completely off the mark here.
What do you think?

Regards,

David Ellingsworth

2010-11-16 16:00:08

by Johannes Stezenbach

[permalink] [raw]
Subject: Re: [PATCH 7/9] rt2x00: Fix rt2800 USB TX Path DMA issue

On Mon, Nov 15, 2010 at 01:45:24AM -0800, Walter Goldens wrote:
>
> These patches appear to have finally brought some life in to the rt307x chips
> in terms of stability and usability. There are no more duplicate (very few)
> or lost packets and overall, the driver is improved immensely, however the
> latency is increased compared to the STA driver. Pinging my gateway with STA
> yields about 2-3ms average, whereas the rt2800usb is about 8-9ms. The
> throughput is also about 20% less.

I'm runnung rt2800usb (RT3070) in AP mode, and when pinging
from the client I get:

[12631.727724] phy0 -> rt2x00usb_watchdog_tx_status: Warning - TX queue 2 status timed out, invoke forced tx handler
[12632.727451] phy0 -> rt2x00usb_watchdog_tx_status: Warning - TX queue 2 status timed out, invoke forced tx handler
[12638.728027] phy0 -> rt2x00usb_watchdog_tx_status: Warning - TX queue 2 status timed out, invoke forced tx handler
[12674.732633] phy0 -> rt2x00usb_watchdog_tx_status: Warning - TX queue 0 status timed out, invoke forced tx handler
[12676.732855] phy0 -> rt2x00usb_watchdog_tx_status: Warning - TX queue 0 status timed out, invoke forced tx handler
[12679.733581] phy0 -> rt2x00usb_watchdog_tx_status: Warning - TX queue 0 status timed out, invoke forced tx handler
[12779.745991] phy0 -> rt2x00usb_watchdog_tx_status: Warning - TX queue 0 status timed out, invoke forced tx handler
etc.

I wonder why it is using TX queue 2? Isn't this QID_AC_VI, i.e.
it should only be used when some programs uses setsockopt SO_PRIORITY
to request QoS for video data?


Lokking through the code, in rt2x00mac_tx():
enum data_queue_qid qid = skb_get_queue_mapping(skb);
is passed to rt2x00queue_get_queue() which uses
enum data_queue_qid has 2 == QID_AC_VI, but net/core/dev.c dev_pick_tx()
calls skb_set_queue_mapping() with 2 == BE
(from ieee802_1d_to_ac, return by ieee80211_netdev_select_queue())

Maybe I'm just confused?


Queue stats after some flood pinging:

# cat /sys/kernel/debug/ieee80211/phy0/rt2800usb/queue/queue
qid count limit length index dma done done
14 199365 128 128 69 69 69
0 785 64 1 18 18 17
1 0 64 0 0 0 0
2 196406 64 0 54 54 54
3 0 64 0 0 0 0
16 0 8 0 0 0 0


Johannes

2010-11-13 18:14:25

by Ivo Van Doorn

[permalink] [raw]
Subject: [PATCH 9/9] rt2x00: Modify rt2x00queue_remove_l2pad to make skb->data two-byte alignment

From: RA-Jay Hung <[email protected]>

When send out skb data to mac80211, orignal code will cause mac80211
unaligned access, so modify code to make mac80211 can natural access.

Signed-off-by: RA-Jay Hung <[email protected]>
Signed-off-by: Ivo van Doorn <[email protected]>
---
drivers/net/wireless/rt2x00/rt2x00queue.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
index dc54317..a3d79c7 100644
--- a/drivers/net/wireless/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
@@ -204,8 +204,10 @@ void rt2x00queue_remove_l2pad(struct sk_buff *skb, unsigned int header_length)
if (!l2pad)
return;

- memmove(skb->data + l2pad, skb->data, header_length);
- skb_pull(skb, l2pad);
+ memmove(skb->data + header_length, skb->data + header_length + l2pad,
+ skb->len - header_length - l2pad);
+
+ skb_trim(skb, skb->len - l2pad);
}

static void rt2x00queue_create_tx_descriptor_seq(struct queue_entry *entry,
--
1.7.2.3


2010-11-13 18:14:18

by Ivo Van Doorn

[permalink] [raw]
Subject: [PATCH 6/9] rt2x00: Use ioremap for SoC devices instead of KSEG1ADDR.

From: Gertjan van Wingerde <[email protected]>

Make the code a bit more portable to architectures that do not support
KSEG1ADDR.

Signed-off-by: Gertjan van Wingerde <[email protected]>
Tested-by: Helmut Schaa <[email protected]>
Signed-off-by: Ivo van Doorn <[email protected]>
---
drivers/net/wireless/rt2x00/rt2800pci.c | 4 +++-
drivers/net/wireless/rt2x00/rt2x00soc.c | 6 ++++--
2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index b0946f8..433c7f3 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -87,9 +87,11 @@ static void rt2800pci_mcu_status(struct rt2x00_dev *rt2x00dev, const u8 token)
#if defined(CONFIG_RALINK_RT288X) || defined(CONFIG_RALINK_RT305X)
static void rt2800pci_read_eeprom_soc(struct rt2x00_dev *rt2x00dev)
{
- u32 *base_addr = (u32 *) KSEG1ADDR(0x1F040000); /* XXX for RT3052 */
+ void __iomem *base_addr = ioremap(0x1F040000, EEPROM_SIZE);

memcpy_fromio(rt2x00dev->eeprom, base_addr, EEPROM_SIZE);
+
+ iounmap(base_addr);
}
#else
static inline void rt2800pci_read_eeprom_soc(struct rt2x00_dev *rt2x00dev)
diff --git a/drivers/net/wireless/rt2x00/rt2x00soc.c b/drivers/net/wireless/rt2x00/rt2x00soc.c
index fc98063..2aa5c38 100644
--- a/drivers/net/wireless/rt2x00/rt2x00soc.c
+++ b/drivers/net/wireless/rt2x00/rt2x00soc.c
@@ -40,6 +40,8 @@ static void rt2x00soc_free_reg(struct rt2x00_dev *rt2x00dev)

kfree(rt2x00dev->eeprom);
rt2x00dev->eeprom = NULL;
+
+ iounmap(rt2x00dev->csr.base);
}

static int rt2x00soc_alloc_reg(struct rt2x00_dev *rt2x00dev)
@@ -51,9 +53,9 @@ static int rt2x00soc_alloc_reg(struct rt2x00_dev *rt2x00dev)
if (!res)
return -ENODEV;

- rt2x00dev->csr.base = (void __iomem *)KSEG1ADDR(res->start);
+ rt2x00dev->csr.base = ioremap(res->start, resource_size(res));
if (!rt2x00dev->csr.base)
- goto exit;
+ return -ENOMEM;

rt2x00dev->eeprom = kzalloc(rt2x00dev->ops->eeprom_size, GFP_KERNEL);
if (!rt2x00dev->eeprom)
--
1.7.2.3


2010-11-13 18:14:22

by Ivo Van Doorn

[permalink] [raw]
Subject: [PATCH 8/9] rt2x00: Fix header_length in rt2x00lib_txdone

From: RA-Jay Hung <[email protected]>

Put the assignment of header_length after pull out extra tx headroom

Signed-off-by: RA-Jay Hung <[email protected]>
Signed-off-by: Ivo van Doorn <[email protected]>
---
drivers/net/wireless/rt2x00/rt2x00dev.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
index 3afa2a3..c879f9a 100644
--- a/drivers/net/wireless/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
@@ -250,10 +250,9 @@ void rt2x00lib_txdone(struct queue_entry *entry,
struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(entry->skb);
struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
enum data_queue_qid qid = skb_get_queue_mapping(entry->skb);
- unsigned int header_length = ieee80211_get_hdrlen_from_skb(entry->skb);
+ unsigned int header_length, i;
u8 rate_idx, rate_flags, retry_rates;
u8 skbdesc_flags = skbdesc->flags;
- unsigned int i;
bool success;

/*
@@ -272,6 +271,11 @@ void rt2x00lib_txdone(struct queue_entry *entry,
skbdesc->flags &= ~SKBDESC_DESC_IN_SKB;

/*
+ * Determine the length of 802.11 header.
+ */
+ header_length = ieee80211_get_hdrlen_from_skb(entry->skb);
+
+ /*
* Remove L2 padding which was added during
*/
if (test_bit(DRIVER_REQUIRE_L2PAD, &rt2x00dev->flags))
--
1.7.2.3


2010-11-16 16:53:17

by Johannes Stezenbach

[permalink] [raw]
Subject: Re: [PATCH 7/9] rt2x00: Fix rt2800 USB TX Path DMA issue

On Tue, Nov 16, 2010 at 05:42:08PM +0100, Helmut Schaa wrote:
> Am Dienstag 16 November 2010 schrieb Johannes Stezenbach:
> >
> > Hm, yes, rt2x00mac_conf_tx() has the same issue that it calls
> > rt2x00queue_get_queue() with an argument which is a mac80211 AC,
> > but expects an enum data_queue_qid. So one could says one
> > bug cancels the other out.
>
> Right. We could of course add a mac80211-queue <-> rt2x00-queue mapping in
> the appropriate places or rename the QID_* identifiers accordingly. Not sure
> if it's worth it ...

Hm, reducing confusion enhances maintainability.

> > > And btw. queue 0 is used for managment frames as well.
> >
> > Queue 0 means QID_AC_BE? Or AC_VI?
>
> It's AC_VO.

Ah, right got it now. This explains the output of
/sys/kernel/debug/ieee80211/phy0/rt2800usb/queue/queue,
qid 0 is used by management frames and queue 2 for BE data.


Thanks,
Johannes

2010-11-16 16:43:10

by Helmut Schaa

[permalink] [raw]
Subject: Re: [PATCH 7/9] rt2x00: Fix rt2800 USB TX Path DMA issue

Am Dienstag 16 November 2010 schrieb Johannes Stezenbach:
> On Tue, Nov 16, 2010 at 05:11:51PM +0100, Helmut Schaa wrote:
> > Am Dienstag 16 November 2010 schrieb Johannes Stezenbach:
> > >
> > > I wonder why it is using TX queue 2? Isn't this QID_AC_VI, i.e.
> > > it should only be used when some programs uses setsockopt SO_PRIORITY
> > > to request QoS for video data?
> > >
> > >
> > > Lokking through the code, in rt2x00mac_tx():
> > > enum data_queue_qid qid = skb_get_queue_mapping(skb);
> > > is passed to rt2x00queue_get_queue() which uses
> > > enum data_queue_qid has 2 == QID_AC_VI, but net/core/dev.c dev_pick_tx()
> > > calls skb_set_queue_mapping() with 2 == BE
> > > (from ieee802_1d_to_ac, return by ieee80211_netdev_select_queue())
> > >
> > > Maybe I'm just confused?
> >
> > Yep, it is indeed confusing. If you compiled rt2x00 with debug output have a
> > look at the queue setup in dmesg:
> >
> > [ 47.590000] phy0 -> rt2x00mac_conf_tx: Info - Configured TX queue 0 - CWmin: 3, CWmax: 4, Aifs: 2, TXop: 102.
> > [ 47.590000] phy0 -> rt2x00mac_conf_tx: Info - Configured TX queue 1 - CWmin: 4, CWmax: 5, Aifs: 2, TXop: 188.
> > [ 47.590000] phy0 -> rt2x00mac_conf_tx: Info - Configured TX queue 2 - CWmin: 5, CWmax: 10, Aifs: 3, TXop: 0.
> > [ 47.590000] phy0 -> rt2x00mac_conf_tx: Info - Configured TX queue 3 - CWmin: 5, CWmax: 10, Aifs: 7, TXop: 0.
> >
> > As you can see queue 2 gets the parameters for AC_BE assigned whereas queue 0
> > gets AC_VO assigned. So the naming within rt2x00 is not consitent with the
> > numbering within mac80211. However, since we configure the queue parameters
> > this should really just be a naming issue.
>
> Hm, yes, rt2x00mac_conf_tx() has the same issue that it calls
> rt2x00queue_get_queue() with an argument which is a mac80211 AC,
> but expects an enum data_queue_qid. So one could says one
> bug cancels the other out.

Right. We could of course add a mac80211-queue <-> rt2x00-queue mapping in
the appropriate places or rename the QID_* identifiers accordingly. Not sure
if it's worth it ...

> > And btw. queue 0 is used for managment frames as well.
>
> Queue 0 means QID_AC_BE? Or AC_VI?

It's AC_VO.

Helmut

2010-11-17 10:17:11

by RA-Jay Hung

[permalink] [raw]
Subject: RE: [PATCH 9/9] rt2x00: Modify rt2x00queue_remove_l2pad to make skb->data two-byte alignment

Hi,

> > When inserting the l2pad we're moving the header and thus reduce headroom.
> > This patch modifies the bahavior during l2pad removal to not move the header
> > back into its old position but instead moves the payload. Thus the skb keeps
> > the reduced headroom. If this skb gets requeued into rt2x00 (which can happen
> > when the frame wasn't acked and the according STA is known to e in powersave
> > mode) the header and payload get aligned again further reducing headroom which
> > results in a too small headroom for the TXWI and thus a skb_under_panic.
>
> Hmm, John merged that patch already. However, I would prefer if it would get
> reverted due to the occasional panics in AP mode.
>
> Jay, I didn't notice any performance degradation on MIPS, on which architecture
> did you test?

My test environment as below
Architecture is AMD Athlon 64 X2 Dual Core, and use rt3070 to test throughput.
Before not patch code, the throughput just only 5, 6M, but after patch,
throughput can achieve about 40M on open air. How about your test?

>
> Jay, Ivo, any objections against reverting this one?
I agree to revert this one because this will cause tool small headroom of your mention.
We need to supply another patch when resolve all issue. Thanks.

Jay
CONFIDENTIALITY STATEMENT : The information, attachments and any rights attaching in this e-mail are confidential and privileged; it is intended only for the individual or entity named as the recipient hereof.Any disclosure, copying, distribution, dissemination or use of the contents of this e-mail by persons other than the intended recipient is STRICTLY PROHIBITED and may violate applicable laws.If you have received this e-mail in error, please delete the original message and notify us by return email or collect call immediately. Thank you.

2010-11-13 18:14:10

by Ivo Van Doorn

[permalink] [raw]
Subject: [PATCH 2/9] rt2x00: Add initial support for RT3370/RT3390 devices.

From: Gertjan van Wingerde <[email protected]>

Modified from Eddy's patch by adding the RT3370 USB support as well.

Signed-off-by: Gertjan van Wingerde <[email protected]>
Cc: Eddy Tsai <[email protected]>
Signed-off-by: Ivo van Doorn <[email protected]>
---
drivers/net/wireless/rt2x00/Kconfig | 22 ++++++++++++++++++++++
drivers/net/wireless/rt2x00/rt2800.h | 1 +
drivers/net/wireless/rt2x00/rt2800lib.c | 9 ++++++---
drivers/net/wireless/rt2x00/rt2800pci.c | 3 +++
drivers/net/wireless/rt2x00/rt2800usb.c | 10 +++++++---
5 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/Kconfig b/drivers/net/wireless/rt2x00/Kconfig
index eea1ef2..f0f0152 100644
--- a/drivers/net/wireless/rt2x00/Kconfig
+++ b/drivers/net/wireless/rt2x00/Kconfig
@@ -96,6 +96,17 @@ config RT2800PCI_RT30XX
Support for these devices is non-functional at the moment and is
intended for testers and developers.

+config RT2800PCI_RT33XX
+ bool "rt2800pci - Include support for rt33xx (PCI/PCIe/PCMCIA) devices"
+ default n
+ ---help---
+ This adds support for rt33xx wireless chipset family to the
+ rt2800pci driver.
+ Supported chips: RT3390
+
+ Support for these devices is non-functional at the moment and is
+ intended for testers and developers.
+
config RT2800PCI_RT35XX
bool "rt2800pci - Include support for rt35xx (PCI/PCIe/PCMCIA) devices"
default n
@@ -165,6 +176,17 @@ config RT2800USB_RT30XX
Support for these devices is non-functional at the moment and is
intended for testers and developers.

+config RT2800USB_RT33XX
+ bool "rt2800usb - Include support for rt33xx (USB) devices"
+ default n
+ ---help---
+ This adds support for rt33xx wireless chipset family to the
+ rt2800usb driver.
+ Supported chips: RT3370
+
+ Support for these devices is non-functional at the moment and is
+ intended for testers and developers.
+
config RT2800USB_RT35XX
bool "rt2800usb - Include support for rt35xx (USB) devices"
default n
diff --git a/drivers/net/wireless/rt2x00/rt2800.h b/drivers/net/wireless/rt2x00/rt2800.h
index 002224c..a81c437 100644
--- a/drivers/net/wireless/rt2x00/rt2800.h
+++ b/drivers/net/wireless/rt2x00/rt2800.h
@@ -47,6 +47,7 @@
* RF3021 2.4G 1T2R
* RF3022 2.4G 2T2R
* RF3052 2.4G 2T2R
+ * RF3320 2.4G 1T1R
*/
#define RF2820 0x0001
#define RF2850 0x0002
diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index b5d2eba..ce8df66 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -1544,7 +1544,8 @@ static void rt2800_config_channel(struct rt2x00_dev *rt2x00dev,
rt2x00_rf(rt2x00dev, RF3020) ||
rt2x00_rf(rt2x00dev, RF3021) ||
rt2x00_rf(rt2x00dev, RF3022) ||
- rt2x00_rf(rt2x00dev, RF3052))
+ rt2x00_rf(rt2x00dev, RF3052) ||
+ rt2x00_rf(rt2x00dev, RF3320))
rt2800_config_channel_rf3xxx(rt2x00dev, conf, rf, info);
else
rt2800_config_channel_rf2xxx(rt2x00dev, conf, rf, info);
@@ -3012,7 +3013,8 @@ int rt2800_init_eeprom(struct rt2x00_dev *rt2x00dev)
!rt2x00_rf(rt2x00dev, RF2020) &&
!rt2x00_rf(rt2x00dev, RF3021) &&
!rt2x00_rf(rt2x00dev, RF3022) &&
- !rt2x00_rf(rt2x00dev, RF3052)) {
+ !rt2x00_rf(rt2x00dev, RF3052) &&
+ !rt2x00_rf(rt2x00dev, RF3320)) {
ERROR(rt2x00dev, "Invalid RF chipset detected.\n");
return -ENODEV;
}
@@ -3276,7 +3278,8 @@ int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
} else if (rt2x00_rf(rt2x00dev, RF3020) ||
rt2x00_rf(rt2x00dev, RF2020) ||
rt2x00_rf(rt2x00dev, RF3021) ||
- rt2x00_rf(rt2x00dev, RF3022)) {
+ rt2x00_rf(rt2x00dev, RF3022) ||
+ rt2x00_rf(rt2x00dev, RF3320)) {
spec->num_channels = 14;
spec->channels = rf_vals_3x;
} else if (rt2x00_rf(rt2x00dev, RF3052)) {
diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index 5f3a018..6642f13 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -1051,6 +1051,9 @@ static DEFINE_PCI_DEVICE_TABLE(rt2800pci_device_table) = {
{ PCI_DEVICE(0x1814, 0x3092), PCI_DEVICE_DATA(&rt2800pci_ops) },
{ PCI_DEVICE(0x1462, 0x891a), PCI_DEVICE_DATA(&rt2800pci_ops) },
#endif
+#ifdef CONFIG_RT2800PCI_RT33XX
+ { PCI_DEVICE(0x1814, 0x3390), PCI_DEVICE_DATA(&rt2800pci_ops) },
+#endif
#ifdef CONFIG_RT2800PCI_RT35XX
{ PCI_DEVICE(0x1814, 0x3060), PCI_DEVICE_DATA(&rt2800pci_ops) },
{ PCI_DEVICE(0x1814, 0x3062), PCI_DEVICE_DATA(&rt2800pci_ops) },
diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index 389ecba..61852c5 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -839,6 +839,13 @@ static struct usb_device_id rt2800usb_device_table[] = {
{ USB_DEVICE(0x5a57, 0x0283), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x5a57, 0x5257), USB_DEVICE_DATA(&rt2800usb_ops) },
#endif
+#ifdef CONFIG_RT2800USB_RT33XX
+ /* Ralink */
+ { USB_DEVICE(0x148f, 0x3370), USB_DEVICE_DATA(&rt2800usb_ops) },
+ { USB_DEVICE(0x148f, 0x8070), USB_DEVICE_DATA(&rt2800usb_ops) },
+ /* Sitecom */
+ { USB_DEVICE(0x0df6, 0x0050), USB_DEVICE_DATA(&rt2800usb_ops) },
+#endif
#ifdef CONFIG_RT2800USB_RT35XX
/* Allwin */
{ USB_DEVICE(0x8516, 0x3572), USB_DEVICE_DATA(&rt2800usb_ops) },
@@ -851,12 +858,9 @@ static struct usb_device_id rt2800usb_device_table[] = {
/* I-O DATA */
{ USB_DEVICE(0x04bb, 0x0944), USB_DEVICE_DATA(&rt2800usb_ops) },
/* Ralink */
- { USB_DEVICE(0x148f, 0x3370), USB_DEVICE_DATA(&rt2800usb_ops) },
{ USB_DEVICE(0x148f, 0x3572), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x148f, 0x8070), USB_DEVICE_DATA(&rt2800usb_ops) },
/* Sitecom */
{ USB_DEVICE(0x0df6, 0x0041), USB_DEVICE_DATA(&rt2800usb_ops) },
- { USB_DEVICE(0x0df6, 0x0050), USB_DEVICE_DATA(&rt2800usb_ops) },
/* Zinwell */
{ USB_DEVICE(0x5a57, 0x0284), USB_DEVICE_DATA(&rt2800usb_ops) },
#endif
--
1.7.2.3


2010-11-13 18:14:17

by Ivo Van Doorn

[permalink] [raw]
Subject: [PATCH 5/9] rt2x00: Remove unneccessary internal Kconfig symbols.

From: Gertjan van Wingerde <[email protected]>

CONFIG_RT2800PCI_PCI and CONFIG_RT2800PCI_SOC are strictly not needed
as we can check the dependent symbols directly in the rest of Kconfig
and the code, so clean up the Kconfig namespace a bit.

Signed-off-by: Gertjan van Wingerde <[email protected]>
Signed-off-by: Ivo van Doorn <[email protected]>
---
drivers/net/wireless/rt2x00/Kconfig | 16 ++-----------
drivers/net/wireless/rt2x00/rt2800pci.c | 34 +++++++++++++++---------------
2 files changed, 20 insertions(+), 30 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/Kconfig b/drivers/net/wireless/rt2x00/Kconfig
index a6939cc..ade3025 100644
--- a/drivers/net/wireless/rt2x00/Kconfig
+++ b/drivers/net/wireless/rt2x00/Kconfig
@@ -53,22 +53,12 @@ config RT61PCI

When compiled as a module, this driver will be called rt61pci.

-config RT2800PCI_PCI
- boolean
- depends on PCI
- default y
-
-config RT2800PCI_SOC
- boolean
- depends on RALINK_RT288X || RALINK_RT305X
- default y
-
config RT2800PCI
tristate "Ralink rt27xx/rt28xx/rt30xx (PCI/PCIe/PCMCIA) support"
- depends on RT2800PCI_PCI || RT2800PCI_SOC
+ depends on PCI || RALINK_RT288X || RALINK_RT305X
select RT2800_LIB
- select RT2X00_LIB_PCI if RT2800PCI_PCI
- select RT2X00_LIB_SOC if RT2800PCI_SOC
+ select RT2X00_LIB_PCI if PCI
+ select RT2X00_LIB_SOC if RALINK_RT288X || RALINK_RT305X
select RT2X00_LIB_HT
select RT2X00_LIB_FIRMWARE
select RT2X00_LIB_CRYPTO
diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index 97f4df6..b0946f8 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -84,7 +84,7 @@ static void rt2800pci_mcu_status(struct rt2x00_dev *rt2x00dev, const u8 token)
rt2800_register_write(rt2x00dev, H2M_MAILBOX_CID, ~0);
}

-#ifdef CONFIG_RT2800PCI_SOC
+#if defined(CONFIG_RALINK_RT288X) || defined(CONFIG_RALINK_RT305X)
static void rt2800pci_read_eeprom_soc(struct rt2x00_dev *rt2x00dev)
{
u32 *base_addr = (u32 *) KSEG1ADDR(0x1F040000); /* XXX for RT3052 */
@@ -95,9 +95,9 @@ static void rt2800pci_read_eeprom_soc(struct rt2x00_dev *rt2x00dev)
static inline void rt2800pci_read_eeprom_soc(struct rt2x00_dev *rt2x00dev)
{
}
-#endif /* CONFIG_RT2800PCI_SOC */
+#endif /* CONFIG_RALINK_RT288X || CONFIG_RALINK_RT305X */

-#ifdef CONFIG_RT2800PCI_PCI
+#ifdef CONFIG_PCI
static void rt2800pci_eepromregister_read(struct eeprom_93cx6 *eeprom)
{
struct rt2x00_dev *rt2x00dev = eeprom->data;
@@ -181,7 +181,7 @@ static inline int rt2800pci_efuse_detect(struct rt2x00_dev *rt2x00dev)
static inline void rt2800pci_read_eeprom_efuse(struct rt2x00_dev *rt2x00dev)
{
}
-#endif /* CONFIG_RT2800PCI_PCI */
+#endif /* CONFIG_PCI */

/*
* Firmware functions
@@ -1031,7 +1031,7 @@ static const struct rt2x00_ops rt2800pci_ops = {
/*
* RT2800pci module information.
*/
-#ifdef CONFIG_RT2800PCI_PCI
+#ifdef CONFIG_PCI
static DEFINE_PCI_DEVICE_TABLE(rt2800pci_device_table) = {
{ PCI_DEVICE(0x1814, 0x0601), PCI_DEVICE_DATA(&rt2800pci_ops) },
{ PCI_DEVICE(0x1814, 0x0681), PCI_DEVICE_DATA(&rt2800pci_ops) },
@@ -1061,19 +1061,19 @@ static DEFINE_PCI_DEVICE_TABLE(rt2800pci_device_table) = {
#endif
{ 0, }
};
-#endif /* CONFIG_RT2800PCI_PCI */
+#endif /* CONFIG_PCI */

MODULE_AUTHOR(DRV_PROJECT);
MODULE_VERSION(DRV_VERSION);
MODULE_DESCRIPTION("Ralink RT2800 PCI & PCMCIA Wireless LAN driver.");
MODULE_SUPPORTED_DEVICE("Ralink RT2860 PCI & PCMCIA chipset based cards");
-#ifdef CONFIG_RT2800PCI_PCI
+#ifdef CONFIG_PCI
MODULE_FIRMWARE(FIRMWARE_RT2860);
MODULE_DEVICE_TABLE(pci, rt2800pci_device_table);
-#endif /* CONFIG_RT2800PCI_PCI */
+#endif /* CONFIG_PCI */
MODULE_LICENSE("GPL");

-#ifdef CONFIG_RT2800PCI_SOC
+#if defined(CONFIG_RALINK_RT288X) || defined(CONFIG_RALINK_RT305X)
static int rt2800soc_probe(struct platform_device *pdev)
{
return rt2x00soc_probe(pdev, &rt2800pci_ops);
@@ -1090,9 +1090,9 @@ static struct platform_driver rt2800soc_driver = {
.suspend = rt2x00soc_suspend,
.resume = rt2x00soc_resume,
};
-#endif /* CONFIG_RT2800PCI_SOC */
+#endif /* CONFIG_RALINK_RT288X || CONFIG_RALINK_RT305X */

-#ifdef CONFIG_RT2800PCI_PCI
+#ifdef CONFIG_PCI
static struct pci_driver rt2800pci_driver = {
.name = KBUILD_MODNAME,
.id_table = rt2800pci_device_table,
@@ -1101,21 +1101,21 @@ static struct pci_driver rt2800pci_driver = {
.suspend = rt2x00pci_suspend,
.resume = rt2x00pci_resume,
};
-#endif /* CONFIG_RT2800PCI_PCI */
+#endif /* CONFIG_PCI */

static int __init rt2800pci_init(void)
{
int ret = 0;

-#ifdef CONFIG_RT2800PCI_SOC
+#if defined(CONFIG_RALINK_RT288X) || defined(CONFIG_RALINK_RT305X)
ret = platform_driver_register(&rt2800soc_driver);
if (ret)
return ret;
#endif
-#ifdef CONFIG_RT2800PCI_PCI
+#ifdef CONFIG_PCI
ret = pci_register_driver(&rt2800pci_driver);
if (ret) {
-#ifdef CONFIG_RT2800PCI_SOC
+#if defined(CONFIG_RALINK_RT288X) || defined(CONFIG_RALINK_RT305X)
platform_driver_unregister(&rt2800soc_driver);
#endif
return ret;
@@ -1127,10 +1127,10 @@ static int __init rt2800pci_init(void)

static void __exit rt2800pci_exit(void)
{
-#ifdef CONFIG_RT2800PCI_PCI
+#ifdef CONFIG_PCI
pci_unregister_driver(&rt2800pci_driver);
#endif
-#ifdef CONFIG_RT2800PCI_SOC
+#if defined(CONFIG_RALINK_RT288X) || defined(CONFIG_RALINK_RT305X)
platform_driver_unregister(&rt2800soc_driver);
#endif
}
--
1.7.2.3


2010-11-22 10:23:12

by Helmut Schaa

[permalink] [raw]
Subject: Re: [PATCH 9/9] rt2x00: Modify rt2x00queue_remove_l2pad to make skb->data two-byte alignment

Hi,

Am Montag 22 November 2010 schrieb Gertjan van Wingerde:
> On Mon, Nov 22, 2010 at 9:14 AM, RA-Jay Hung <[email protected]> wrote:
> >> > Jay, could you please run a few more performance tests with and without this
> >> > patch to track down if this issue is really the cause for the performance
> >> > degradation?
> >
> > I check sniffer and mac80211 code, I think bad TX throughput should be my
> > environment has a lot APs and more packets will collides because of without
> > protection in TX direction. So the patch is not related to this issue.
> > Helmut, We can revert this patch. Sorry for inconvenience.

Great, thanks for double-checking Jay.

> >> OK. Find attached the patch I cooked up. AFAICS the driver still works correctly,
> >> but unfortunately I am unable to test performance and throughput of the driver
> >> with this patch.
> >>
> >> Jay and Helmut, can you test this patch before I submit it?
> >>
> > I think original code should recover the original skb state, so I think we do not need to copy again to send back to mac80211, and one more thing. Could you submit below
> > patch you send us before to rt2x00.git. I think it is more correct in payload = 0 case.
> >
> > void rt2x00queue_remove_l2pad(struct sk_buff *skb, unsigned int header_length)
> > {
> > - unsigned int l2pad = L2PAD_SIZE(header_length);
> > + unsigned int payload_length = skb->len - header_length;
> > + unsigned int l2pad = payload_length ? L2PAD_SIZE(header_length) : 0;
> >
> > if (!l2pad)
> > return;
> >
>
> OK. Indeed if you feel we can simply revert the patch then that will
> be better. I'll send a patch tonight that reverts it together with the
> update update.

Sounds good to me,
Helmut

2010-11-17 16:42:18

by Helmut Schaa

[permalink] [raw]
Subject: Re: [PATCH 9/9] rt2x00: Modify rt2x00queue_remove_l2pad to make skb->data two-byte alignment

Am Mittwoch 17 November 2010 schrieb Gertjan van Wingerde:
> On Wed, Nov 17, 2010 at 4:07 PM, John W. Linville
> <[email protected]> wrote:
> > On Wed, Nov 17, 2010 at 11:48:04AM +0100, Helmut Schaa wrote:
> >
> >> John, should I send a follow-up (with a nice description why it this is needed)
> >> or are you simply reverting this one?
> >
> > Is there no chance for a fix in short order?

I don't have a clever idea on how to fix that without reverting this commit.
IMO this patch is just not correct as it passes the skb back to mac80211 with
reduced headroom (due to the header & payload alignment) which causes trouble
when mac80211 requeues the frame. Of course just requesting 4 additional bytes
headroom would "fix" the symptoms but sounds like a hack to me.

> I may have an idea on how we can fix this, without incurring the
> performance penalty.

I'm still not convinced that this is really the root cause for the
performance issues Jay noticed. AFAIK mac80211 doesn't access the payload
anymore when reporting the frame back (with some exceptions like monitor
interfaces).

Jay, could you please run a few more performance tests with and without this
patch to track down if this issue is really the cause for the performance
degradation?

> Basic idea is to no longer work on the original skb that mac80211
> supplied us, but to
> use a copy of that skb. This would prevent us from having to undo any
> changes we did,
> as we can simply return the original skb to mac80211 (which wasn't
> modified in the first
> place).
> I'm not sure how this would impact performance, but it would allow us
> a lot less copying
> around to undo the changes done before uploading to the HW.

But cloning the skb would double the amount of memory needed to transmit each
frame. Not sure though if that behaves better or not. Might be worth a try.

> However, I won't be able to look into that opportunity before the weekend.
>
> Helmut, can you wait that long and hold off reverting until then?

Sure.

Thanks,
Helmut

2010-11-16 16:34:50

by Johannes Stezenbach

[permalink] [raw]
Subject: Re: [PATCH 7/9] rt2x00: Fix rt2800 USB TX Path DMA issue

On Tue, Nov 16, 2010 at 05:11:51PM +0100, Helmut Schaa wrote:
> Am Dienstag 16 November 2010 schrieb Johannes Stezenbach:
> >
> > I wonder why it is using TX queue 2? Isn't this QID_AC_VI, i.e.
> > it should only be used when some programs uses setsockopt SO_PRIORITY
> > to request QoS for video data?
> >
> >
> > Lokking through the code, in rt2x00mac_tx():
> > enum data_queue_qid qid = skb_get_queue_mapping(skb);
> > is passed to rt2x00queue_get_queue() which uses
> > enum data_queue_qid has 2 == QID_AC_VI, but net/core/dev.c dev_pick_tx()
> > calls skb_set_queue_mapping() with 2 == BE
> > (from ieee802_1d_to_ac, return by ieee80211_netdev_select_queue())
> >
> > Maybe I'm just confused?
>
> Yep, it is indeed confusing. If you compiled rt2x00 with debug output have a
> look at the queue setup in dmesg:
>
> [ 47.590000] phy0 -> rt2x00mac_conf_tx: Info - Configured TX queue 0 - CWmin: 3, CWmax: 4, Aifs: 2, TXop: 102.
> [ 47.590000] phy0 -> rt2x00mac_conf_tx: Info - Configured TX queue 1 - CWmin: 4, CWmax: 5, Aifs: 2, TXop: 188.
> [ 47.590000] phy0 -> rt2x00mac_conf_tx: Info - Configured TX queue 2 - CWmin: 5, CWmax: 10, Aifs: 3, TXop: 0.
> [ 47.590000] phy0 -> rt2x00mac_conf_tx: Info - Configured TX queue 3 - CWmin: 5, CWmax: 10, Aifs: 7, TXop: 0.
>
> As you can see queue 2 gets the parameters for AC_BE assigned whereas queue 0
> gets AC_VO assigned. So the naming within rt2x00 is not consitent with the
> numbering within mac80211. However, since we configure the queue parameters
> this should really just be a naming issue.

Hm, yes, rt2x00mac_conf_tx() has the same issue that it calls
rt2x00queue_get_queue() with an argument which is a mac80211 AC,
but expects an enum data_queue_qid. So one could says one
bug cancels the other out.

> And btw. queue 0 is used for managment frames as well.

Queue 0 means QID_AC_BE? Or AC_VI?


Johannes

2010-11-15 09:45:26

by Walter Goldens

[permalink] [raw]
Subject: Re: [PATCH 7/9] rt2x00: Fix rt2800 USB TX Path DMA issue

> Signed-off-by: RA-Jay Hung <[email protected]>
> Signed-off-by: Ivo van Doorn <[email protected]>

These patches appear to have finally brought some life in to the rt307x chips in terms of stability and usability. There are no more duplicate (very few) or lost packets and overall, the driver is improved immensely, however the latency is increased compared to the STA driver. Pinging my gateway with STA yields about 2-3ms average, whereas the rt2800usb is about 8-9ms. The throughput is also about 20% less. Do you think this can be corrected at some point and do you anticipate further improvements in this direction?

Walter




2010-11-13 18:14:12

by Ivo Van Doorn

[permalink] [raw]
Subject: [PATCH 3/9] rt2x00: Clean up Kconfig for RT2800 devices.

From: Gertjan van Wingerde <[email protected]>

General clean up of the Kconfig part for RT28XX devices.
Also remove the indications of non functional support for rt27xx/rt28xx/rt30xx
devices, as this is no longer true. They just work fine.
Finally, remove the experimental indications for rt27xx/rt28xx/rt30xx devices
as that is no longer true. Keep the experimental indications for rt33xx/rt35xx
devices, though.

Signed-off-by: Gertjan van Wingerde <[email protected]>
Acked-by: Helmut Schaa <[email protected]>
Signed-off-by: Ivo van Doorn <[email protected]>
---
drivers/net/wireless/rt2x00/Kconfig | 47 ++++++++++++++--------------------
1 files changed, 19 insertions(+), 28 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/Kconfig b/drivers/net/wireless/rt2x00/Kconfig
index f0f0152..f2be1d3 100644
--- a/drivers/net/wireless/rt2x00/Kconfig
+++ b/drivers/net/wireless/rt2x00/Kconfig
@@ -64,8 +64,8 @@ config RT2800PCI_SOC
default y

config RT2800PCI
- tristate "Ralink rt28xx/rt30xx/rt35xx (PCI/PCIe/PCMCIA) support (EXPERIMENTAL)"
- depends on (RT2800PCI_PCI || RT2800PCI_SOC) && EXPERIMENTAL
+ tristate "Ralink rt27xx/rt28xx (PCI/PCIe/PCMCIA) support"
+ depends on RT2800PCI_PCI || RT2800PCI_SOC
select RT2800_LIB
select RT2X00_LIB_PCI if RT2800PCI_PCI
select RT2X00_LIB_SOC if RT2800PCI_SOC
@@ -75,29 +75,24 @@ config RT2800PCI
select CRC_CCITT
select EEPROM_93CX6
---help---
- This adds support for rt2800/rt3000/rt3500 wireless chipset family.
+ This adds support for rt27xx/rt28xx wireless chipset family.
Supported chips: RT2760, RT2790, RT2860, RT2880, RT2890 & RT3052

- This driver is non-functional at the moment and is intended for
- developers.
-
When compiled as a module, this driver will be called "rt2800pci.ko".

if RT2800PCI

config RT2800PCI_RT30XX
- bool "rt2800pci - Include support for rt30xx (PCI/PCIe/PCMCIA) devices"
+ bool "rt2800pci - Include support for rt30xx devices"
default y
---help---
This adds support for rt30xx wireless chipset family to the
rt2800pci driver.
Supported chips: RT3090, RT3091 & RT3092

- Support for these devices is non-functional at the moment and is
- intended for testers and developers.
-
config RT2800PCI_RT33XX
- bool "rt2800pci - Include support for rt33xx (PCI/PCIe/PCMCIA) devices"
+ bool "rt2800pci - Include support for rt33xx devices (EXPERIMENTAL)"
+ depends on EXPERIMENTAL
default n
---help---
This adds support for rt33xx wireless chipset family to the
@@ -108,7 +103,8 @@ config RT2800PCI_RT33XX
intended for testers and developers.

config RT2800PCI_RT35XX
- bool "rt2800pci - Include support for rt35xx (PCI/PCIe/PCMCIA) devices"
+ bool "rt2800pci - Include support for rt35xx devices (EXPERIMENTAL)"
+ depends on EXPERIMENTAL
default n
---help---
This adds support for rt35xx wireless chipset family to the
@@ -145,8 +141,8 @@ config RT73USB
When compiled as a module, this driver will be called rt73usb.

config RT2800USB
- tristate "Ralink rt2800 (USB) support (EXPERIMENTAL)"
- depends on USB && EXPERIMENTAL
+ tristate "Ralink rt27xx/rt28xx (USB) support"
+ depends on USB
select RT2800_LIB
select RT2X00_LIB_USB
select RT2X00_LIB_HT
@@ -154,30 +150,24 @@ config RT2800USB
select RT2X00_LIB_CRYPTO
select CRC_CCITT
---help---
- This adds experimental support for rt2800 wireless chipset family.
+ This adds support for rt27xx/rt28xx wireless chipset family.
Supported chips: RT2770, RT2870 & RT3070.

- Known issues:
- - support for RT2870 chips doesn't work with 802.11n APs yet
- - support for RT3070 chips is non-functional at the moment
-
When compiled as a module, this driver will be called "rt2800usb.ko".

if RT2800USB

config RT2800USB_RT30XX
- bool "rt2800usb - Include support for rt30xx (USB) devices"
+ bool "rt2800usb - Include support for rt30xx devices"
default y
---help---
This adds support for rt30xx wireless chipset family to the
rt2800usb driver.
Supported chips: RT3070, RT3071 & RT3072

- Support for these devices is non-functional at the moment and is
- intended for testers and developers.
-
config RT2800USB_RT33XX
- bool "rt2800usb - Include support for rt33xx (USB) devices"
+ bool "rt2800usb - Include support for rt33xx devices (EXPERIMENTAL)"
+ depends on EXPERIMENTAL
default n
---help---
This adds support for rt33xx wireless chipset family to the
@@ -188,7 +178,8 @@ config RT2800USB_RT33XX
intended for testers and developers.

config RT2800USB_RT35XX
- bool "rt2800usb - Include support for rt35xx (USB) devices"
+ bool "rt2800usb - Include support for rt35xx devices (EXPERIMENTAL)"
+ depends on EXPERIMENTAL
default n
---help---
This adds support for rt35xx wireless chipset family to the
@@ -202,9 +193,9 @@ config RT2800USB_UNKNOWN
bool "rt2800usb - Include support for unknown (USB) devices"
default n
---help---
- This adds support for rt2800 family devices that are known to
- have a rt2800 family chipset, but for which the exact chipset
- is unknown.
+ This adds support for rt2800usb devices that are known to
+ have a rt28xx family compatible chipset, but for which the exact
+ chipset is unknown.

Support status for these devices is unknown, and enabling these
devices may or may not work.
--
1.7.2.3


2010-11-16 15:46:15

by Helmut Schaa

[permalink] [raw]
Subject: Re: [PATCH 9/9] rt2x00: Modify rt2x00queue_remove_l2pad to make skb->data two-byte alignment

Am Samstag 13 November 2010 schrieb Ivo van Doorn:
> From: RA-Jay Hung <[email protected]>
>
> When send out skb data to mac80211, orignal code will cause mac80211
> unaligned access, so modify code to make mac80211 can natural access.

And this introduces the same panic again :(

The problem is the following:

We don't pass the skb in the same state back to mac80211 as we got it.

When inserting the l2pad we're moving the header and thus reduce headroom.
This patch modifies the bahavior during l2pad removal to not move the header
back into its old position but instead moves the payload. Thus the skb keeps
the reduced headroom. If this skb gets requeued into rt2x00 (which can happen
when the frame wasn't acked and the according STA is known to e in powersave
mode) the header and payload get aligned again further reducing headroom which
results in a too small headroom for the TXWI and thus a skb_under_panic.

Helmut

> Signed-off-by: RA-Jay Hung <[email protected]>
> Signed-off-by: Ivo van Doorn <[email protected]>
> ---
> drivers/net/wireless/rt2x00/rt2x00queue.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
> index dc54317..a3d79c7 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00queue.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
> @@ -204,8 +204,10 @@ void rt2x00queue_remove_l2pad(struct sk_buff *skb, unsigned int header_length)
> if (!l2pad)
> return;
>
> - memmove(skb->data + l2pad, skb->data, header_length);
> - skb_pull(skb, l2pad);
> + memmove(skb->data + header_length, skb->data + header_length + l2pad,
> + skb->len - header_length - l2pad);
> +
> + skb_trim(skb, skb->len - l2pad);
> }
>
> static void rt2x00queue_create_tx_descriptor_seq(struct queue_entry *entry,
>


2010-11-17 15:34:46

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH 9/9] rt2x00: Modify rt2x00queue_remove_l2pad to make skb->data two-byte alignment

On Wed, Nov 17, 2010 at 4:07 PM, John W. Linville
<[email protected]> wrote:
> On Wed, Nov 17, 2010 at 11:48:04AM +0100, Helmut Schaa wrote:
>
>> John, should I send a follow-up (with a nice description why it this is needed)
>> or are you simply reverting this one?
>
> Is there no chance for a fix in short order?
>

I may have an idea on how we can fix this, without incurring the
performance penalty.
Basic idea is to no longer work on the original skb that mac80211
supplied us, but to
use a copy of that skb. This would prevent us from having to undo any
changes we did,
as we can simply return the original skb to mac80211 (which wasn't
modified in the first
place).
I'm not sure how this would impact performance, but it would allow us
a lot less copying
around to undo the changes done before uploading to the HW.

However, I won't be able to look into that opportunity before the weekend.

Helmut, can you wait that long and hold off reverting until then?

---
Gertjan

2010-11-13 18:14:21

by Ivo Van Doorn

[permalink] [raw]
Subject: [PATCH 7/9] rt2x00: Fix rt2800 USB TX Path DMA issue

From: RA-Jay Hung <[email protected]>

rt2800usb chips need to add 1~3 bytes zero padding after each 802.11 header & payload,
and at the end need to add 4 bytes zero padding whether doing TX bulk aggregation or not,

TXINFO_W0_USB_DMA_TX_PKT_LEN in TXINFO must include 1-3 bytes padding after 802.11 header & payload
but do not include 4 bytes end zero padding.

In rt2800usb_get_tx_data_len do not consider multiple of the USB packet size case, sometimes this will
cause USB DMA problem.

Signed-off-by: RA-Jay Hung <[email protected]>
Signed-off-by: Ivo van Doorn <[email protected]>
---
drivers/net/wireless/rt2x00/rt2800usb.c | 39 ++++++++++++++++++++----------
1 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index 2933bf1..935b76d 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -307,8 +307,14 @@ static void rt2800usb_write_tx_desc(struct queue_entry *entry,
* Initialize TXINFO descriptor
*/
rt2x00_desc_read(txi, 0, &word);
+
+ /*
+ * The size of TXINFO_W0_USB_DMA_TX_PKT_LEN is
+ * TXWI + 802.11 header + L2 pad + payload + pad,
+ * so need to decrease size of TXINFO and USB end pad.
+ */
rt2x00_set_field32(&word, TXINFO_W0_USB_DMA_TX_PKT_LEN,
- entry->skb->len - TXINFO_DESC_SIZE);
+ entry->skb->len - TXINFO_DESC_SIZE - 4);
rt2x00_set_field32(&word, TXINFO_W0_WIV,
!test_bit(ENTRY_TXD_ENCRYPT_IV, &txdesc->flags));
rt2x00_set_field32(&word, TXINFO_W0_QSEL, 2);
@@ -326,22 +332,29 @@ static void rt2800usb_write_tx_desc(struct queue_entry *entry,
skbdesc->desc_len = TXINFO_DESC_SIZE + TXWI_DESC_SIZE;
}

-/*
- * TX data initialization
- */
-static int rt2800usb_get_tx_data_len(struct queue_entry *entry)
+static void rt2800usb_write_tx_data(struct queue_entry *entry,
+ struct txentry_desc *txdesc)
{
- int length;
+ u8 padding_len;

/*
- * The length _must_ include 4 bytes padding,
- * it should always be multiple of 4,
- * but it must _not_ be a multiple of the USB packet size.
+ * pad(1~3 bytes) is added after each 802.11 payload.
+ * USB end pad(4 bytes) is added at each USB bulk out packet end.
+ * TX frame format is :
+ * | TXINFO | TXWI | 802.11 header | L2 pad | payload | pad | USB end pad |
+ * |<------------- tx_pkt_len ------------->|
*/
- length = roundup(entry->skb->len + 4, 4);
- length += (4 * !(length % entry->queue->usb_maxpacket));
+ rt2800_write_tx_data(entry, txdesc);
+ padding_len = roundup(entry->skb->len + 4, 4) - entry->skb->len;
+ memset(skb_put(entry->skb, padding_len), 0, padding_len);
+}

- return length;
+/*
+ * TX data initialization
+ */
+static int rt2800usb_get_tx_data_len(struct queue_entry *entry)
+{
+ return entry->skb->len;
}

/*
@@ -579,7 +592,7 @@ static const struct rt2x00lib_ops rt2800usb_rt2x00_ops = {
.link_tuner = rt2800_link_tuner,
.watchdog = rt2800usb_watchdog,
.write_tx_desc = rt2800usb_write_tx_desc,
- .write_tx_data = rt2800_write_tx_data,
+ .write_tx_data = rt2800usb_write_tx_data,
.write_beacon = rt2800_write_beacon,
.get_tx_data_len = rt2800usb_get_tx_data_len,
.kick_tx_queue = rt2x00usb_kick_tx_queue,
--
1.7.2.3


2010-11-17 10:49:06

by Helmut Schaa

[permalink] [raw]
Subject: Re: [PATCH 9/9] rt2x00: Modify rt2x00queue_remove_l2pad to make skb->data two-byte alignment

Am Mittwoch 17 November 2010 schrieb RA-Jay Hung:
> > > When inserting the l2pad we're moving the header and thus reduce headroom.
> > > This patch modifies the bahavior during l2pad removal to not move the header
> > > back into its old position but instead moves the payload. Thus the skb keeps
> > > the reduced headroom. If this skb gets requeued into rt2x00 (which can happen
> > > when the frame wasn't acked and the according STA is known to e in powersave
> > > mode) the header and payload get aligned again further reducing headroom which
> > > results in a too small headroom for the TXWI and thus a skb_under_panic.
> >
> > Hmm, John merged that patch already. However, I would prefer if it would get
> > reverted due to the occasional panics in AP mode.
> >
> > Jay, I didn't notice any performance degradation on MIPS, on which architecture
> > did you test?
>
> My test environment as below
> Architecture is AMD Athlon 64 X2 Dual Core, and use rt3070 to test throughput.
> Before not patch code, the throughput just only 5, 6M, but after patch,
> throughput can achieve about 40M on open air. How about your test?

I tried with rt2800pci on MIPS32 in AP mode (STA <- eth -> AP <- wifi -> STA2).
An UDP iperf stream (STA -> STA2) was able to achieve up to 90Mbps while a TCP
stream maxed out at around 70Mbps (before and after this patch). STA2 is an
Intel HT20 11n client.

Hmm, you've been testing on USB. Maybe that makes a difference?

Nevertheless, such a performance drop on such a fast machine is likely caused
by something different, no? Did you have any warnings in dmesg (due to
watchdog timeouts for example)? Maybe the rate control algorithm made a wrong
decision and used a low tx rate?

> > Jay, Ivo, any objections against reverting this one?
>
> I agree to revert this one because this will cause tool small headroom of your mention.
> We need to supply another patch when resolve all issue. Thanks.

John, should I send a follow-up (with a nice description why it this is needed)
or are you simply reverting this one?

Thanks,
Helmut

2010-11-22 10:05:15

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH 9/9] rt2x00: Modify rt2x00queue_remove_l2pad to make skb->data two-byte alignment

On Mon, Nov 22, 2010 at 9:14 AM, RA-Jay Hung <[email protected]> wrote:
> Hi,
>> > Jay, could you please run a few more performance tests with and without this
>> > patch to track down if this issue is really the cause for the performance
>> > degradation?
>
> I check sniffer and mac80211 code, I think bad TX throughput should be my environment has a lot APs and more packets will collides because of without protection in TX direction. So the patch is not related to this issue. Helmut,
> We can revert this patch. Sorry for inconvenience.
>
>> >> Basic idea is to no longer work on the original skb that mac80211
>> >> supplied us, but to
>> >> use a copy of that skb. This would prevent us from having to undo any
>> >> changes we did,
>> >> as we can simply return the original skb to mac80211 (which wasn't
>> >> modified in the first
>> >> place).
>> >> I'm not sure how this would impact performance, but it would allow us
>> >> a lot less copying
>> >> around to undo the changes done before uploading to the HW.
>> >
>> > But cloning the skb would double the amount of memory needed to transmit each
>> > frame. Not sure though if that behaves better or not. Might be worth a try.
>> >
>> >> However, I won't be able to look into that opportunity before the weekend.
>> >>
>> >> Helmut, can you wait that long and hold off reverting until then?
>> >
>>
>> OK. Find attached the patch I cooked up. AFAICS the driver still works correctly,
>> but unfortunately I am unable to test performance and throughput of the driver
>> with this patch.
>>
>> Jay and Helmut, can you test this patch before I submit it?
>>
> I think original code should recover the original skb state, so I think we do not need to copy again to send back to mac80211, and one more thing. Could you submit below
> patch you send us before to rt2x00.git. I think it is more correct in payload = 0 case.
>
> ?void rt2x00queue_remove_l2pad(struct sk_buff *skb, unsigned int header_length)
> ?{
> - ? ? ? unsigned int l2pad = L2PAD_SIZE(header_length);
> + ? ? ? unsigned int payload_length = skb->len - header_length;
> + ? ? ? unsigned int l2pad = payload_length ? L2PAD_SIZE(header_length) : 0;
>
> ? ? ? ?if (!l2pad)
> ? ? ? ? ? ? ? ?return;
>

OK. Indeed if you feel we can simply revert the patch then that will
be better. I'll send a patch tonight that reverts it together with the
update update.

---
Gertjan

2010-11-17 15:14:53

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 9/9] rt2x00: Modify rt2x00queue_remove_l2pad to make skb->data two-byte alignment

On Wed, Nov 17, 2010 at 11:48:04AM +0100, Helmut Schaa wrote:

> John, should I send a follow-up (with a nice description why it this is needed)
> or are you simply reverting this one?

Is there no chance for a fix in short order?

--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-11-16 15:37:32

by Helmut Schaa

[permalink] [raw]
Subject: Re: [PATCH 8/9] rt2x00: Fix header_length in rt2x00lib_txdone

Am Samstag 13 November 2010 schrieb Ivo van Doorn:
> From: RA-Jay Hung <[email protected]>
>
> Put the assignment of header_length after pull out extra tx headroom

Thanks, this fixes an occasional panic for me in AP mode:

[ 133.980000] skb_under_panic: text:81933574 len:1550 put:16 head:80a86800 data:80a867fc tail:0x80a86e0a end:0x80a86ea0 dev:wlan0
[ 134.010000] Kernel bug detected[#1]:
[ 134.010000] Cpu 0
[ 134.010000] $ 0 : 00000000 80320000 00000089 00000001
[ 134.010000] $ 4 : 8020559c 00002a63 00000001 00002a63
[ 134.010000] $ 8 : 00000020 00000001 00000001 0000000a
[ 134.010000] $12 : 000000cd 00000009 00000000 3a699d00
[ 134.010000] $16 : 81909d00 80fac2f8 818f4aa8 80e7fba0
[ 134.010000] $20 : 81909d00 819036f8 819036e0 00000002
[ 134.010000] $24 : 00000000 80103130
[ 134.010000] $28 : 80e7e000 80e7fb58 00000001 8012c4f0
[ 134.010000] Hi : 00000000
[ 134.010000] Lo : cbd20000
[ 134.010000] epc : 8012c4f0 skb_push+0x6c/0x88
[ 134.010000] Not tainted
[ 134.010000] ra : 8012c4f0 skb_push+0x6c/0x88
[ 134.010000] Status: 1000e403 KERNEL EXL IE
[ 134.010000] Cause : 10800024
[ 134.010000] PrId : 0001964c (MIPS 24Kc)
[ 134.010000] Modules linked in: rt2800pci rt2800lib rt2x00soc rt2x00pci rt2x00lib mac80211 eeprom_93cx6 crc_itu_t crc_ccitt cfg80211 compat arc4 aes_generic deflate ecb cbc
[ 134.010000] Process irq/6-rt2800_wm (pid: 794, threadinfo=80e7e000, task=819811e0, tls=00000000)
[ 134.010000] Stack : 00005220 81933574 0000060e 00000010 80a86800 80a867fc 80a86e0a 80a86ea0
[ 134.010000] 81979000 81909d00 819036f8 81933574 80840000 819036f8 00000000 00000041
[ 134.010000] 8020f480 819a9a80 0000be88 00000002 001a05fc 00800030 00040000 0000000d
[ 134.010000] 00020007 00020005 00000003 00040000 00000006 00000004 001a0021 00000008
[ 134.010000] 819036f8 819036f8 819036e0 818f4aa8 00000000 81931cfc 80e7fc78 80f1831c
[ 134.010000] ...
[ 134.010000] Call Trace:
[ 134.010000] [<8012c4f0>] skb_push+0x6c/0x88
[ 134.010000] [<81933574>] rt2x00queue_write_tx_frame+0x218/0x344 [rt2x00lib]
[ 134.010000] [<81931cfc>] rt2x00mac_tx+0x258/0x2d4 [rt2x00lib]
[ 134.010000] [<80f19fa8>] __ieee80211_tx+0x14c/0x1e4 [mac80211]
[ 134.010000] [<80f1a118>] ieee80211_tx+0xd8/0x25c [mac80211]
[ 134.010000] [<80f1ae58>] ieee80211_tx_pending+0x11c/0x26c [mac80211]
[ 134.010000] [<8001e444>] tasklet_action+0x88/0xe4
[ 134.010000] [<8001ec10>] __do_softirq+0xb0/0x148
[ 134.010000] [<8001ecf0>] do_softirq+0x48/0x6c
[ 134.010000] [<8001ef78>] local_bh_enable+0x8c/0xa8
[ 134.010000] [<81930414>] rt2x00lib_rxdone+0x2d0/0x320 [rt2x00lib]
[ 134.010000] [<80eee2ec>] rt2x00pci_initialize+0x24c/0x270 [rt2x00pci]
[ 134.010000]
[ 134.010000]
[ 134.010000] Code: afa9001c 0c001571 afa20020 <0200000d> 0804b13d 00000000 8fbf002c 01001021 03e00008

Helmut

> Signed-off-by: RA-Jay Hung <[email protected]>
> Signed-off-by: Ivo van Doorn <[email protected]>
> ---
> drivers/net/wireless/rt2x00/rt2x00dev.c | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
> index 3afa2a3..c879f9a 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00dev.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
> @@ -250,10 +250,9 @@ void rt2x00lib_txdone(struct queue_entry *entry,
> struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(entry->skb);
> struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
> enum data_queue_qid qid = skb_get_queue_mapping(entry->skb);
> - unsigned int header_length = ieee80211_get_hdrlen_from_skb(entry->skb);
> + unsigned int header_length, i;
> u8 rate_idx, rate_flags, retry_rates;
> u8 skbdesc_flags = skbdesc->flags;
> - unsigned int i;
> bool success;
>
> /*
> @@ -272,6 +271,11 @@ void rt2x00lib_txdone(struct queue_entry *entry,
> skbdesc->flags &= ~SKBDESC_DESC_IN_SKB;
>
> /*
> + * Determine the length of 802.11 header.
> + */
> + header_length = ieee80211_get_hdrlen_from_skb(entry->skb);
> +
> + /*
> * Remove L2 padding which was added during
> */
> if (test_bit(DRIVER_REQUIRE_L2PAD, &rt2x00dev->flags))
>