Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tx aggregation for iwm driver (7000/8000/9000) #454

Merged
merged 7 commits into from
Dec 19, 2020

Conversation

pigworlds
Copy link
Contributor

This is the work to enable Tx aggregation (AMPDU) for iwm driver.
This is based on the previous partial ampdu implementation by zxysd.

A test on Intel Wireless-AC 9560 card show around 30mbps tx based on iperf3 test. Other cards are untested.
Note, this change also need to use along with #453, or otherwise rate is not stable enough when tx aggregation is enabled.

@zxystd
Copy link
Collaborator

zxystd commented Dec 16, 2020

Nice job! I will test it tonight if I have spare time and my friends will also test it. BTW, I just noticed that OpenBSD developer @stspdotname had also implemented Tx agg for iwm driver but there are someone reported that it will caused firmware error(https://www.mail-archive.com/tech@openbsd.org/msg58503.html), have you do some different optimize things or reproduced the same error? Maybe we need more test on it. Thank you for the contribution again.

@zxystd
Copy link
Collaborator

zxystd commented Dec 16, 2020

Tested with ac9260 and it works, although still have problems. And when I merge the changes to my 40mhz support branch, the speed comes double, actually over 120mbps download now. Can't wait for ampdu gen2 branch(WIP 80%), and soon we can start support for 80mhz(11ac).
Also, I will do more test about these commit.

@williambj1 williambj1 added the enhancement New feature or request label Dec 16, 2020
@pigworlds
Copy link
Contributor Author

@zxystd if you look at the commits, you will see the this work also include the partial patch coming from @stspdotname you're referring to.

In your original work, there is an issue around TX_CMD_FLG_SEQ_CTL flag. There are issues around update/reset the scheduler on the tx response handling. With both fixes taken from @stspdotname's patch, this works fairly well on my Intel Wireless-AC 8265 card.

Now come down to the 9000 series, I do exhibit the firmware crash with both patch sets. The firmware crash after a few packets send. After a bit trial and error, I spot a patch in iwlwifi. That's the secret sauce to make firmware happy on 9000 series. The patch mentioned it is backward compatible, and I would assume it will continue to work on 7000/8000 series.

This PR #454 include all the patches above. With this patchset, I can turn on Tx AMPDU on Intel Wireless-AC 9560 card. There are still two issues are observed.

  • The tx rate is not stable on 9000 series. I can observed gaps between tx, and quite a bit re-transmission at upper TCP level. With the set of upstream patches from OpenBSD Sync openbsd net80211 #453 the issue seems gone. I am not sure exactly which patch make it work.
  • The tx rate seems sensitive to tx failures and retries. The rate adaption algorithm tend to bring down the MCS rate. During the iperf test, the MCS rate is high in the beginning then slowly going down to my environment.
    The MiRA algorithm used seems limited to 20MHz and HT rates only. I haven't look into the details. I would presume certain amount work need to be done here to support more VHT rates and various configuration. This can be a prerequisites to achieve 11ac

Thanks again on take time look into this patch and the great work. Looking forward to see your ht40 and vht enhancements.

@seungjunProgramming
Copy link

seungjunProgramming commented Dec 18, 2020

@pigworlds @zxystd
now testing on Big Sur 11.1 and with AC-8265
Might take some time since I am trying to test it several times before checking it is stable!

@seungjunProgramming
Copy link

seungjunProgramming commented Dec 18, 2020

Test complete!
Download Speed improve: about 17%
Upload Speed improve: 0%

Tested about 10times.
No stability issues
Thankyou for this great commit!

@seungjunProgramming
Copy link

Download speed goes up to 103mb/s in AC-8265

@Playragnarok
Copy link

How can i use this pr

@williambj1
Copy link
Contributor

@Playragnarok This pull request will eventually get merged, please be patient and use other sources for personal help. The comment section in this PR is for reviewing only.

@pigworlds
Copy link
Contributor Author

I noticed the rate adaption is one of the factors impacting tx throughput. This can be either the algorithm used or the way it is plumbed through the driver.

I did a simple test by using a fixed MCS 15 and avoid rate adaption. The result is amazing. On a 9560 card, it can tx sustained at 30mbps without AMPDU, 105mbps with AMPDU on 20MHz, and 195mbps with AMPDU on 40MHz.

This area can be someone interesting to look.

@zxystd
Copy link
Collaborator

zxystd commented Dec 19, 2020

tested with ac3165 and ac8265, works unless, merged. thank you.

@zxystd zxystd merged commit 1dfaada into OpenIntelWireless:master Dec 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

5 participants