From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
To: Stefan Lippers-Hollmann <s.l-h@gmx.de>,
Ping-Ke Shih <pkshih@realtek.com>
Cc: "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
Larry Finger <Larry.Finger@lwfinger.net>,
Christian Hewitt <chewitt@libreelec.tv>,
Johannes Berg <johannes@sipsolutions.net>
Subject: Re: [PATCH v7 12/12] wifi: rtlwifi: Enable the new rtl8192du driver
Date: Tue, 28 May 2024 02:06:23 +0300 [thread overview]
Message-ID: <29f850c5-4f61-466f-9a7a-437b05bc8251@gmail.com> (raw)
In-Reply-To: <20240527112534.4dbcdf75@mir>
On 27/05/2024 12:25, Stefan Lippers-Hollmann wrote:
> Hi
>
> On 2024-05-27, Ping-Ke Shih wrote:
>> Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>>> The RTL8192DU is an older Wifi 4 dual band chip. It comes in two
>>> flavours: single MAC single PHY (like most Realtek Wifi 4 USB devices),
>>> and dual MAC dual PHY.
> [...]
>>> Stefan Lippers-Hollmann tested the speed in the 5 GHz band with 40 MHz
>>> channel width: 173/99 megabits/second.
>>
>> Could you please confirm this description is still correct in v7?
>> If so, your Tested-by tag is welcome.
>
> I held back with a formal Tested-by for this, as rtlwifi needs
> some fixing after the chanctx changes
> (0a44dfc070749514b804ccac0b1fd38718f7daa1, "wifi: mac80211:
> simplify non-chanctx drivers") went in (for v6.9 and up).
>
> So, yes, rtl8192du v7 itself works fine (as did v6 and previous), but
> the chanctx changes do cause a performance regression, resulting in
> lower (and very inconsistent) throughput and larger packet loss,
> due to the rate control being confused. As Bitterblue found out,
> rtlwifi relies on the previous behaviour imposed by
>
> ieee80211_hw_config(local, ~0);
>
> As a consequence of the change to
>
> ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_LISTEN_INTERVAL |
> IEEE80211_CONF_CHANGE_MONITOR |
> IEEE80211_CONF_CHANGE_PS |
> IEEE80211_CONF_CHANGE_RETRY_LIMITS |
> IEEE80211_CONF_CHANGE_IDLE);
>
> mac80211 effectively sets a retry limit of 0x4, while rtlwifi
> expects a retry limit of 0x30 (in STA mode or 0x7 in AP mode),
> causing rtlwifi not to settle on a good fallback rate (and thereby
> losing packets).
>
> Disclaimer: the above issue and its explanation has been discovered
> by Bitterblue, I just did the git bisecting (rtl8192du on top of
> v6.8..v6.9) and performance testing to pinpoint the commit causing
> the regression.
>
> rtl8192du on top of v6.9+ (with the chanctx changes):
> $ iperf3 -c <remote_ipv6>
> [ ID] Interval Transfer Bitrate
> [ 5] 0.00-1.00 sec 384 KBytes 3.14 Mbits/sec
> [ 5] 1.00-2.00 sec 512 KBytes 4.19 Mbits/sec
> [ 5] 2.00-3.00 sec 1.00 MBytes 8.39 Mbits/sec
> [ 5] 3.00-4.00 sec 640 KBytes 5.24 Mbits/sec
> [ 5] 4.00-5.00 sec 0.00 Bytes 0.00 bits/sec
> [ 5] 5.00-6.00 sec 0.00 Bytes 0.00 bits/sec
> [ 5] 6.00-7.00 sec 256 KBytes 2.10 Mbits/sec
> [ 5] 7.00-8.00 sec 640 KBytes 5.25 Mbits/sec
> [ 5] 8.00-9.00 sec 512 KBytes 4.19 Mbits/sec
> [ 5] 9.00-10.00 sec 256 KBytes 2.10 Mbits/sec
> - - - - - - - - - - - - - - - - - - - - - - - - -
> [ ID] Interval Transfer Bitrate
> [ 5] 0.00-10.00 sec 4.12 MBytes 3.46 Mbits/sec receiver
>
> $ iperf3 -c <remote_ipv6> --reverse
> [ ID] Interval Transfer Bitrate Retr Cwnd
> [ 5] 0.00-1.00 sec 12.9 MBytes 108 Mbits/sec 0 629 KBytes
> [ 5] 1.00-2.00 sec 9.75 MBytes 81.8 Mbits/sec 0 1.13 MBytes
> [ 5] 2.00-3.00 sec 11.0 MBytes 92.3 Mbits/sec 0 1.63 MBytes
> [ 5] 3.00-4.00 sec 1.38 MBytes 11.5 Mbits/sec 1 1.67 MBytes
> [ 5] 4.00-5.00 sec 0.00 Bytes 0.00 bits/sec 1 1.39 KBytes
> [ 5] 5.00-6.00 sec 0.00 Bytes 0.00 bits/sec 1 1.39 KBytes
> [ 5] 6.00-7.00 sec 0.00 Bytes 0.00 bits/sec 0 1.39 KBytes
> [ 5] 7.00-8.00 sec 9.88 MBytes 82.8 Mbits/sec 0 2.12 MBytes
> [ 5] 8.00-9.00 sec 6.88 MBytes 57.7 Mbits/sec 3 2.32 MBytes
> [ 5] 9.00-10.00 sec 0.00 Bytes 0.00 bits/sec 0 2.32 MBytes
> - - - - - - - - - - - - - - - - - - - - - - - - -
> [ ID] Interval Transfer Bitrate Retr
> [ 5] 0.00-10.00 sec 51.8 MBytes 43.4 Mbits/sec 6 sender
>
> $ iperf3 -c <remote_ipv6> --bidir
> [ ID][Role] Interval Transfer Bitrate Retr Cwnd
> [ 5][RX-S] 0.00-1.00 sec 0.00 Bytes 0.00 bits/sec
> [ 8][TX-S] 0.00-1.00 sec 1.38 MBytes 11.5 Mbits/sec 0 90.6 KBytes
> [ 5][RX-S] 1.00-2.00 sec 0.00 Bytes 0.00 bits/sec
> [ 8][TX-S] 1.00-2.00 sec 4.12 MBytes 34.6 Mbits/sec 4 1.39 KBytes
> [ 5][RX-S] 2.00-3.00 sec 0.00 Bytes 0.00 bits/sec
> [ 8][TX-S] 2.00-3.00 sec 0.00 Bytes 0.00 bits/sec 1 1.39 KBytes
> [ 5][RX-S] 3.00-4.00 sec 0.00 Bytes 0.00 bits/sec
> [ 8][TX-S] 3.00-4.00 sec 0.00 Bytes 0.00 bits/sec 1 1.39 KBytes
> [ 5][RX-S] 4.00-5.00 sec 0.00 Bytes 0.00 bits/sec
> [ 8][TX-S] 4.00-5.00 sec 0.00 Bytes 0.00 bits/sec 0 1.39 KBytes
> [ 5][RX-S] 5.00-6.00 sec 0.00 Bytes 0.00 bits/sec
> [ 8][TX-S] 5.00-6.00 sec 0.00 Bytes 0.00 bits/sec 2 106 KBytes
> [ 5][RX-S] 6.00-7.00 sec 0.00 Bytes 0.00 bits/sec
> [ 8][TX-S] 6.00-7.00 sec 0.00 Bytes 0.00 bits/sec 0 106 KBytes
> [ 5][RX-S] 7.00-8.00 sec 0.00 Bytes 0.00 bits/sec
> [ 8][TX-S] 7.00-8.00 sec 0.00 Bytes 0.00 bits/sec 0 106 KBytes
> [ 5][RX-S] 8.00-9.00 sec 0.00 Bytes 0.00 bits/sec
> [ 8][TX-S] 8.00-9.00 sec 0.00 Bytes 0.00 bits/sec 0 106 KBytes
> [ 5][RX-S] 9.00-10.00 sec 0.00 Bytes 0.00 bits/sec
> [ 8][TX-S] 9.00-10.00 sec 1.25 MBytes 10.5 Mbits/sec 2 119 KBytes
> - - - - - - - - - - - - - - - - - - - - - - - - -
> [ ID][Role] Interval Transfer Bitrate Retr
> [ 5][RX-S] 0.00-10.01 sec 0.00 Bytes 0.00 bits/sec receiver
> [ 8][TX-S] 0.00-10.01 sec 6.75 MBytes 5.66 Mbits/sec 10 sender
>
> Again, rtl8192du v7 itself is 'fine' and could (can) achieve 180/100
> MBit/s before the chanctx changes, so merging v7 might be okay - but
> there needs to be a fix for rtlwifi as a whole (and maybe even other
> drivers) for v6.9+ and the chanctx changes that came with it (not
> every chipset supported by rtlwifi might be affected equally badly,
> e.g. rtl8192cu might not noticeably be, according to Bitterblue's
> testing).
>
> So with the (performance/ packet loss) caveat above:
>
> Tested-by: Stefan Lippers-Hollmann <s.l-h@gmx.de>
>
> I am running rtl8192du v7 on top of v6.9.2 now (and tested it on top
> of wireless-next as of last Friday (2024-05-24)), with the performance
> figures above.
>
> Regards
> Stefan Lippers-Hollmann
>
> --
> This is the result of my git bisecting:
>
> $ git bisect good
> 0a44dfc070749514b804ccac0b1fd38718f7daa1 is the first bad commit
> commit 0a44dfc070749514b804ccac0b1fd38718f7daa1
> Author: Johannes Berg <johannes.berg@intel.com>
> Date: Mon Jan 29 19:34:38 2024 +0100
>
> wifi: mac80211: simplify non-chanctx drivers
>
> There are still surprisingly many non-chanctx drivers, but in
> mac80211 that code is a bit awkward. Simplify this by having
> those drivers assign 'emulated' ops, so that the mac80211 code
> can be more unified between non-chanctx/chanctx drivers. This
> cuts the number of places caring about it by about 15, which
> are scattered across - now they're fewer and no longer in the
> channel context handling.
>
> Link: https://msgid.link/20240129194108.6d0ead50f5cf.I60d093b2fc81ca1853925a4d0ac3a2337d5baa5b@changeid
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>
> drivers/net/wireless/admtek/adm8211.c | 4 +
> drivers/net/wireless/ath/ar5523/ar5523.c | 4 +
> drivers/net/wireless/ath/ath5k/mac80211-ops.c | 4 +
> drivers/net/wireless/ath/ath9k/htc_drv_main.c | 4 +
> drivers/net/wireless/ath/ath9k/main.c | 4 +
> drivers/net/wireless/ath/carl9170/main.c | 4 +
> drivers/net/wireless/ath/wcn36xx/main.c | 4 +
> drivers/net/wireless/atmel/at76c50x-usb.c | 4 +
> drivers/net/wireless/broadcom/b43/main.c | 4 +
> drivers/net/wireless/broadcom/b43legacy/main.c | 4 +
> drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c | 4 +
> drivers/net/wireless/intel/iwlegacy/3945-mac.c | 4 +
> drivers/net/wireless/intel/iwlegacy/4965-mac.c | 4 +
> drivers/net/wireless/intel/iwlwifi/dvm/mac80211.c | 4 +
> drivers/net/wireless/intersil/p54/main.c | 4 +
> drivers/net/wireless/marvell/libertas_tf/main.c | 4 +
> drivers/net/wireless/marvell/mwl8k.c | 4 +
> drivers/net/wireless/mediatek/mt76/mt7603/main.c | 4 +
> drivers/net/wireless/mediatek/mt76/mt76x0/pci.c | 4 +
> drivers/net/wireless/mediatek/mt76/mt76x0/usb.c | 4 +
> drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c | 4 +
> drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c | 4 +
> drivers/net/wireless/mediatek/mt76/mt792x_core.c | 7 +-
> drivers/net/wireless/mediatek/mt76/mt7996/main.c | 4 +
> drivers/net/wireless/mediatek/mt7601u/main.c | 4 +
> drivers/net/wireless/purelifi/plfxlc/mac.c | 4 +
> drivers/net/wireless/ralink/rt2x00/rt2400pci.c | 4 +
> drivers/net/wireless/ralink/rt2x00/rt2500pci.c | 4 +
> drivers/net/wireless/ralink/rt2x00/rt2500usb.c | 4 +
> drivers/net/wireless/ralink/rt2x00/rt2800pci.c | 4 +
> drivers/net/wireless/ralink/rt2x00/rt2800soc.c | 4 +
> drivers/net/wireless/ralink/rt2x00/rt2800usb.c | 4 +
> drivers/net/wireless/ralink/rt2x00/rt61pci.c | 4 +
> drivers/net/wireless/ralink/rt2x00/rt73usb.c | 4 +
> drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c | 4 +
> drivers/net/wireless/realtek/rtl818x/rtl8187/dev.c | 4 +
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 4 +
> drivers/net/wireless/realtek/rtlwifi/core.c | 4 +
> drivers/net/wireless/realtek/rtw88/mac80211.c | 4 +
> drivers/net/wireless/realtek/rtw89/core.c | 7 +-
> drivers/net/wireless/rsi/rsi_91x_mac80211.c | 4 +
> drivers/net/wireless/st/cw1200/main.c | 4 +
> drivers/net/wireless/ti/wl1251/main.c | 4 +
> drivers/net/wireless/virtual/mac80211_hwsim.c | 4 +
> drivers/net/wireless/zydas/zd1211rw/zd_mac.c | 4 +
> drivers/staging/vt6655/device_main.c | 4 +
> drivers/staging/vt6656/main_usb.c | 4 +
> include/net/mac80211.h | 13 ++
> net/mac80211/cfg.c | 42 +++----
> net/mac80211/chan.c | 111 +++--------------
> net/mac80211/ieee80211_i.h | 9 +-
> net/mac80211/iface.c | 6 +-
> net/mac80211/main.c | 205 +++++++++++++++++++++++++------
> net/mac80211/mlme.c | 3 +-
> net/mac80211/offchannel.c | 21 ++--
> net/mac80211/scan.c | 18 +--
> net/mac80211/tx.c | 2 -
> net/mac80211/util.c | 27 ++--
> 58 files changed, 444 insertions(+), 207 deletions(-)
>
> $ git bisect log
> git bisect start
> # Status: warte auf guten und schlechten Commit
> # bad: [a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6] Linux 6.9
> git bisect bad a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6
> # Status: warte auf gute(n) Commit(s), schlechter Commit bekannt
> # good: [e8f897f4afef0031fe618a8e94127a0934896aba] Linux 6.8
> git bisect good e8f897f4afef0031fe618a8e94127a0934896aba
> # bad: [480e035fc4c714fb5536e64ab9db04fedc89e910] Merge tag 'drm-next-2024-03-13' of https://gitlab.freedesktop.org/drm/kernel
> git bisect bad 480e035fc4c714fb5536e64ab9db04fedc89e910
> # bad: [9187210eee7d87eea37b45ea93454a88681894a4] Merge tag 'net-next-6.9' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next
> git bisect bad 9187210eee7d87eea37b45ea93454a88681894a4
> # bad: [9187210eee7d87eea37b45ea93454a88681894a4] Merge tag 'net-next-6.9' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next
> git bisect bad 9187210eee7d87eea37b45ea93454a88681894a4
> # good: [a01c9fe32378636ae65bec8047b5de3fdb2ba5c8] Merge tag 'nfsd-6.9' of git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux
> git bisect good a01c9fe32378636ae65bec8047b5de3fdb2ba5c8
> # bad: [ca61ba3885274a684c83d8a538eb77b30e38ee92] Merge branch 'rework-genet-mdioclocking'
> git bisect bad ca61ba3885274a684c83d8a538eb77b30e38ee92
> # good: [f42822f22b1c5f72c7e3497d9683f379ab0c5fe4] bnxt_en: Use firmware provided maximum filter counts.
> git bisect good f42822f22b1c5f72c7e3497d9683f379ab0c5fe4
> # bad: [e10cd2ddd89e8b3e61b49247067e79f7debec2f1] wifi: rtw89: load BB parameters to PHY-1
> git bisect bad e10cd2ddd89e8b3e61b49247067e79f7debec2f1
> # good: [2594e4d9e1a2d79bf7bb262974abaf5ef153e371] wifi: iwlwifi: prepare for reading SAR tables from UEFI
> git bisect good 2594e4d9e1a2d79bf7bb262974abaf5ef153e371
> # bad: [719036ae06d4bfdb65139e3947a8404dec298bc5] wifi: cfg80211: move puncturing validation code
> git bisect bad 719036ae06d4bfdb65139e3947a8404dec298bc5
> # good: [1209f487d452ff7e822dec30661fd6b5163fb8cf] wifi: rtl8xxxu: Add TP-Link TL-WN823N V2
> git bisect good 1209f487d452ff7e822dec30661fd6b5163fb8cf
> # good: [4dbd964f33aab6f99891b9610ad4b36cc215be0d] wifi: rtw89: 8922a: add chip_ops::rfk_hw_init
> git bisect good 4dbd964f33aab6f99891b9610ad4b36cc215be0d
> # good: [2fd53eb04c492eb9a2b06f994b36e5cf34ba7541] wifi: mac80211: remove unused MAX_MSG_LEN define
> git bisect good 2fd53eb04c492eb9a2b06f994b36e5cf34ba7541
> # bad: [0a44dfc070749514b804ccac0b1fd38718f7daa1] wifi: mac80211: simplify non-chanctx drivers
> git bisect bad 0a44dfc070749514b804ccac0b1fd38718f7daa1
> # good: [61f0261131c8dc2beeb6b34781a54788221081e9] wifi: mac80211: clean up band switch in duration
> git bisect good 61f0261131c8dc2beeb6b34781a54788221081e9
> # good: [2d9698dd32d086e47b8bff3df4322cc017c17b55] wifi: mac80211: clean up HE 6 GHz and EHT chandef parsing
> git bisect good 2d9698dd32d086e47b8bff3df4322cc017c17b55
> # first bad commit: [0a44dfc070749514b804ccac0b1fd38718f7daa1] wifi: mac80211: simplify non-chanctx drivers
That's all correct.
Johannes suggested that we should set hw->wiphy->retry_{long,short}
before ieee80211_register_hw(). So that would go in
_rtl_init_mac80211(). That has the added benefit of making the
true retry limits visible to userspace ("iw phy").
The problem is that setting hw->wiphy->retry_{long,short} is
not enough. rtlwifi still gets the default retry limits of 4
and 7, because ieee80211_register_hw() doesn't set
hw->conf.long_frame_max_tx_count (and hw->conf.short_frame_max_tx_count).
Johannes suggested moving this code from ieee80211_alloc_hw_nm()
to ieee80211_register_hw():
local->hw.conf.long_frame_max_tx_count = wiphy->retry_long;
local->hw.conf.short_frame_max_tx_count = wiphy->retry_short;
I didn't do this yet partly because I don't want to compile
the entire kernel, and partly because I'm not sure how to handle
the different retry limits for AP/IBSS mode and station mode.
Can we change hw->wiphy->retry_{long,short} any time, not just
before ieee80211_register_hw()? If yes, what is even the point
of hw->conf.{short,long}_frame_max_tx_count ? It would be simpler
if we can ignore them and use hw->wiphy->retry_{long,short}
directly.
What do y'all think?
next prev parent reply other threads:[~2024-05-27 23:06 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-23 14:42 [PATCH v7 00/12] wifi: rtlwifi: Add new rtl8192du driver Bitterblue Smith
2024-05-23 14:43 ` [PATCH v7 01/12] wifi: rtlwifi: rtl8192d: Use "rtl92d" prefix Bitterblue Smith
2024-05-30 2:50 ` Ping-Ke Shih
2024-05-23 14:44 ` [PATCH v7 02/12] wifi: rtlwifi: Add rtl8192du/table.{c,h} Bitterblue Smith
2024-05-23 14:44 ` [PATCH v7 03/12] wifi: rtlwifi: Add new members to struct rtl_priv for RTL8192DU Bitterblue Smith
2024-05-23 14:45 ` [PATCH v7 04/12] wifi: rtlwifi: Add rtl8192du/hw.{c,h} Bitterblue Smith
2024-05-23 14:46 ` [PATCH v7 05/12] wifi: rtlwifi: Add rtl8192du/phy.{c,h} Bitterblue Smith
2024-05-23 14:46 ` [PATCH v7 06/12] wifi: rtlwifi: Add rtl8192du/trx.{c,h} Bitterblue Smith
2024-05-23 14:46 ` [PATCH v7 07/12] wifi: rtlwifi: Add rtl8192du/rf.{c,h} Bitterblue Smith
2024-05-23 14:47 ` [PATCH v7 08/12] wifi: rtlwifi: Add rtl8192du/fw.{c,h} and rtl8192du/led.{c,h} Bitterblue Smith
2024-05-23 14:47 ` [PATCH v7 09/12] wifi: rtlwifi: Add rtl8192du/dm.{c,h} Bitterblue Smith
2024-05-23 14:48 ` [PATCH v7 10/12] wifi: rtlwifi: Constify rtl_hal_cfg.{ops,usb_interface_cfg} and rtl_priv.cfg Bitterblue Smith
2024-05-23 14:48 ` [PATCH v7 11/12] wifi: rtlwifi: Add rtl8192du/sw.c Bitterblue Smith
2024-05-23 14:49 ` [PATCH v7 12/12] wifi: rtlwifi: Enable the new rtl8192du driver Bitterblue Smith
2024-05-27 7:47 ` Ping-Ke Shih
2024-05-27 9:25 ` Stefan Lippers-Hollmann
2024-05-27 23:06 ` Bitterblue Smith [this message]
2024-05-28 2:39 ` Ping-Ke Shih
2024-05-28 6:59 ` Johannes Berg
2024-05-28 8:41 ` Ping-Ke Shih
2024-05-28 2:02 ` Ping-Ke Shih
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=29f850c5-4f61-466f-9a7a-437b05bc8251@gmail.com \
--to=rtl8821cerfe2@gmail.com \
--cc=Larry.Finger@lwfinger.net \
--cc=chewitt@libreelec.tv \
--cc=johannes@sipsolutions.net \
--cc=linux-wireless@vger.kernel.org \
--cc=pkshih@realtek.com \
--cc=s.l-h@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).