| Message ID | 20231230045105.91351-6-sergeantsagara@protonmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | wifi: b43: Various QoS-related fixes | expand |
On Sat, 30 Dec 2023 04:51:51 +0000 Rahul Rameshbabu <sergeantsagara@protonmail.com> wrote: > bcm4331 appears to lack QoS support. I think that's rather unlikely. The firmware probably is just too old for this device. > +static const u16 b43_no_qos_chip_ids[] = { > + BCMA_CHIP_ID_BCM4331, > + 0, > +}; > + > +static bool b43_qos_not_supported(struct b43_wldev *dev) > +{ > + int idx; > + > + for (idx = 0; b43_no_qos_chip_ids[idx]; idx++) > + if (dev->dev->chip_id == b43_no_qos_chip_ids[idx]) > + return true; > + > + return false; > +} > + > static void b43_wireless_core_exit(struct b43_wldev *dev); > static int b43_wireless_core_init(struct b43_wldev *dev); > static struct b43_wldev * b43_wireless_core_stop(struct b43_wldev *dev); > @@ -2587,7 +2603,7 @@ static void b43_request_firmware(struct work_struct *work) > > start_ieee80211: > wl->hw->queues = B43_QOS_QUEUE_NUM; > - if (!modparam_qos || dev->fw.opensource) > + if (!modparam_qos || dev->fw.opensource || b43_qos_not_supported(wl->current_dev)) This looks a bit over-engineered to me. Can we just instead do it like this, please? if (!modparam_qos || dev->fw.opensource || dev->dev->chip_id == BCMA_CHIP_ID_BCM4331) > wl->hw->queues = 1; > > err = ieee80211_register_hw(wl->hw);
On Sat, 30 Dec, 2023 14:45:23 +0100 Michael Büsch <m@bues.ch> wrote: > [[PGP Signed Part:Undecided]] > On Sat, 30 Dec 2023 04:51:51 +0000 > Rahul Rameshbabu <sergeantsagara@protonmail.com> wrote: > >> bcm4331 appears to lack QoS support. > > I think that's rather unlikely. > The firmware probably is just too old for this device. I just retested with newer firmware released on 2012-08-15. I still see the same issue with QoS. This appears to be the newest firmware I can acquire from http://lwfinger.com/b43-firmware/, which I extract from broadcom-wl-6.30.163.46. [ 11.661972] b43-phy0: Loading firmware version 784.2 (2012-08-15 21:35:19) [ 11.919942] b43-phy0: Loading firmware version 784.2 (2012-08-15 21:35:19) [ 13.717460] [drm] amdgpu kernel modesetting enabled. [ 13.717705] amdgpu: Virtual CRAT table created for CPU [ 13.717719] amdgpu: Topology: Add CPU node [ 13.776896] NET: Registered PF_PACKET protocol family [ 15.234058] b43-phy0: Loading firmware version 784.2 (2012-08-15 21:35:19) [ 15.319388] wlp3s0b1: authenticate with 1c:87:2c:6f:f4:e0 [ 15.333239] wlp3s0b1: send auth to 1c:87:2c:6f:f4:e0 (try 1/3) [ 15.341672] wlp3s0b1: authenticated [ 15.341921] wlp3s0b1: associate with 1c:87:2c:6f:f4:e0 (try 1/3) [ 15.346912] wlp3s0b1: RX AssocResp from 1c:87:2c:6f:f4:e0 (capab=0x411 status=0 aid=3) [ 15.347255] wlp3s0b1: associated > >> +static const u16 b43_no_qos_chip_ids[] = { >> + BCMA_CHIP_ID_BCM4331, >> + 0, >> +}; >> + >> +static bool b43_qos_not_supported(struct b43_wldev *dev) >> +{ >> + int idx; >> + >> + for (idx = 0; b43_no_qos_chip_ids[idx]; idx++) >> + if (dev->dev->chip_id == b43_no_qos_chip_ids[idx]) >> + return true; >> + >> + return false; >> +} >> + >> static void b43_wireless_core_exit(struct b43_wldev *dev); >> static int b43_wireless_core_init(struct b43_wldev *dev); >> static struct b43_wldev * b43_wireless_core_stop(struct b43_wldev *dev); >> @@ -2587,7 +2603,7 @@ static void b43_request_firmware(struct work_struct *work) >> >> start_ieee80211: >> wl->hw->queues = B43_QOS_QUEUE_NUM; >> - if (!modparam_qos || dev->fw.opensource) >> + if (!modparam_qos || dev->fw.opensource || b43_qos_not_supported(wl->current_dev)) > > This looks a bit over-engineered to me. > Can we just instead do it like this, please? > > if (!modparam_qos || dev->fw.opensource || dev->dev->chip_id == BCMA_CHIP_ID_BCM4331) > Ack. >> wl->hw->queues = 1; >> >> err = ieee80211_register_hw(wl->hw); -- Thanks, Rahul Rameshbabu
On Sat, 30 Dec 2023 17:10:26 +0000 Rahul Rameshbabu <sergeantsagara@protonmail.com> wrote: > > The firmware probably is just too old for this device. > > I just retested with newer firmware released on 2012-08-15. I still see > the same issue with QoS. This appears to be the newest firmware I can > acquire from http://lwfinger.com/b43-firmware/, which I extract from > broadcom-wl-6.30.163.46. It's still pretty old. Nobody has worked on getting a newer version extracted for a long time. But I'm fine with just disabling QoS on this device. Upgrading to a newer firmware probably is a bigger change. That could be done later.
diff --git a/drivers/net/wireless/broadcom/b43/main.c b/drivers/net/wireless/broadcom/b43/main.c index b6ac1526c0e8..39906bebef7b 100644 --- a/drivers/net/wireless/broadcom/b43/main.c +++ b/drivers/net/wireless/broadcom/b43/main.c @@ -359,6 +359,22 @@ static struct ieee80211_supported_band b43_band_2ghz_limited = { .n_bitrates = b43_g_ratetable_size, }; +static const u16 b43_no_qos_chip_ids[] = { + BCMA_CHIP_ID_BCM4331, + 0, +}; + +static bool b43_qos_not_supported(struct b43_wldev *dev) +{ + int idx; + + for (idx = 0; b43_no_qos_chip_ids[idx]; idx++) + if (dev->dev->chip_id == b43_no_qos_chip_ids[idx]) + return true; + + return false; +} + static void b43_wireless_core_exit(struct b43_wldev *dev); static int b43_wireless_core_init(struct b43_wldev *dev); static struct b43_wldev * b43_wireless_core_stop(struct b43_wldev *dev); @@ -2587,7 +2603,7 @@ static void b43_request_firmware(struct work_struct *work) start_ieee80211: wl->hw->queues = B43_QOS_QUEUE_NUM; - if (!modparam_qos || dev->fw.opensource) + if (!modparam_qos || dev->fw.opensource || b43_qos_not_supported(wl->current_dev)) wl->hw->queues = 1; err = ieee80211_register_hw(wl->hw);
Add b43_qos_not_supported functionality for disabling QoS. Currently, the function checks an array to see if the chip id matches a value in the array. If so, disable QoS. bcm4331 appears to lack QoS support. When queues that are not of the default "best effort" priority are selected, traffic appears to not transmit out of the hardware while no errors are returned. This behavior is present among all the other priority queues: video, voice, and background. While this can be worked around by setting a kernel parameter, the default behavior is problematic for most users and may be difficult to debug. This patch offers a working out-of-box experience for bcm4331 users. Log of the issue (using ssh low-priority traffic as an example): ssh -T -vvvv git@github.com OpenSSH_9.6p1, OpenSSL 3.0.12 24 Oct 2023 debug1: Reading configuration data /etc/ssh/ssh_config debug2: checking match for 'host * exec "/nix/store/q1c2flcykgr4wwg5a6h450hxbk4ch589-bash-5.2-p15/bin/bash -c '/nix/store/c015armnkhr6v18za0rypm7sh1i8js8w-gnupg-2.4.1/bin/gpg-connect-agent --quiet updatestartuptty /bye >/dev/null 2>&1'"' host github.com originally github.com debug3: /etc/ssh/ssh_config line 5: matched 'host "github.com"' debug1: Executing command: '/nix/store/q1c2flcykgr4wwg5a6h450hxbk4ch589-bash-5.2-p15/bin/bash -c '/nix/store/c015armnkhr6v18za0rypm7sh1i8js8w-gnupg-2.4.1/bin/gpg-connect-agent --quiet updatestartuptty /bye >/dev/null 2>&1'' debug3: command returned status 0 debug3: /etc/ssh/ssh_config line 5: matched 'exec "/nix/store/q1c2flcykgr4wwg5a6h450hxbk4ch589-bash-5.2-p15/bin/bash -c '/nix/store/c015armnkhr6v18za0r"' debug2: match found debug1: /etc/ssh/ssh_config line 9: Applying options for * debug3: expanded UserKnownHostsFile '~/.ssh/known_hosts' -> '/home/binary-eater/.ssh/known_hosts' debug3: expanded UserKnownHostsFile '~/.ssh/known_hosts2' -> '/home/binary-eater/.ssh/known_hosts2' debug2: resolving "github.com" port 22 debug3: resolve_host: lookup github.com:22 debug3: channel_clear_timeouts: clearing debug3: ssh_connect_direct: entering debug1: Connecting to github.com [192.30.255.113] port 22. debug3: set_sock_tos: set socket 3 IP_TOS 0x48 Fixes: e6f5b934fba8 ("b43: Add QOS support") Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com> --- drivers/net/wireless/broadcom/b43/main.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)