| Message ID | 20230322171905.492855-1-martin.kaistra@linutronix.de |
|---|---|
| Headers | show |
| Series | wifi: rtl8xxxu: Add AP mode support for 8188f | expand |
On 22/03/2023 19:18, Martin Kaistra wrote: > This series intends to bring AP mode support to the rtl8xxxu driver, > more specifically for the 8188f, because this is the HW I have. > The work is based on the vendor driver as I do not have access to > datasheets. > > This is an RFC, so that there can be a discussion first before > potentially implementing support for the other chips in this driver, if > required. > Hi! I ran into some problems while testing this. First, a null pointer dereference in rtl8xxxu_config_filter() when turning on the AP. priv->vif was NULL: if (priv->vif->type != NL80211_IFTYPE_AP) { I changed it like this: if (priv->vif && priv->vif->type != NL80211_IFTYPE_AP) { Then I was able to turn on the AP and connect my phone to it. However, the system froze after a few seconds. I had `journalctl --follow` running. The last thing printed before the system froze was the DHCP stuff (discover, offer, request, ack). The phone said it was connected, but speedtest.net didn't have time to load before the freeze. My system is a laptop with RTL8822CE internal wifi card connected to my ISP's router. The connections are managed by NetworkManager 1.42.4-1, which uses wpa_supplicant 2:2.10-8 and dnsmasq 2.89-1. The operating system is Arch Linux running kernel 6.2.5-arch1-1. I used Plasma's NetworkManager applet to create a new "Wi-Fi (shared)" connection with mode "Access Point", band 2.4 GHz, channel 1, no encryption, and "IPv4 is required for this connection". Unrelated to anything, just out of curiosity, what brand/model is your RTL8188FU?
Hi, Am 23.03.23 um 18:12 schrieb Bitterblue Smith: > On 22/03/2023 19:18, Martin Kaistra wrote: >> This series intends to bring AP mode support to the rtl8xxxu driver, >> more specifically for the 8188f, because this is the HW I have. >> The work is based on the vendor driver as I do not have access to >> datasheets. >> >> This is an RFC, so that there can be a discussion first before >> potentially implementing support for the other chips in this driver, if >> required. >> > > Hi! > > I ran into some problems while testing this. > > First, a null pointer dereference in rtl8xxxu_config_filter() when > turning on the AP. priv->vif was NULL: > > if (priv->vif->type != NL80211_IFTYPE_AP) { > > I changed it like this: > > if (priv->vif && priv->vif->type != NL80211_IFTYPE_AP) { > > Then I was able to turn on the AP and connect my phone to it. However, > the system froze after a few seconds. I had `journalctl --follow` > running. The last thing printed before the system froze was the DHCP > stuff (discover, offer, request, ack). The phone said it was connected, > but speedtest.net didn't have time to load before the freeze. > > My system is a laptop with RTL8822CE internal wifi card connected to my > ISP's router. The connections are managed by NetworkManager 1.42.4-1, > which uses wpa_supplicant 2:2.10-8 and dnsmasq 2.89-1. The operating > system is Arch Linux running kernel 6.2.5-arch1-1. > > I used Plasma's NetworkManager applet to create a new "Wi-Fi (shared)" > connection with mode "Access Point", band 2.4 GHz, channel 1, no > encryption, and "IPv4 is required for this connection". Thanks for your bug report. I did use hostapd for testing, I will try to see, if I can reproduce any problems when using NetworkManager. > > > Unrelated to anything, just out of curiosity, what brand/model is your > RTL8188FU? This should be the one I have: https://www.fn-link.com/6188S-UF-Wi-Fi-Module-pd41531470.html (Fn-Link 6188S-UF) Martin
Am 06.04.23 um 02:42 schrieb Ping-Ke Shih: > > >> -----Original Message----- >> From: Martin Kaistra <martin.kaistra@linutronix.de> >> Sent: Wednesday, April 5, 2023 11:31 PM >> To: Bitterblue Smith <rtl8821cerfe2@gmail.com> >> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih >> <pkshih@realtek.com>; Sebastian Andrzej Siewior <bigeasy@linutronix.de>; linux-wireless@vger.kernel.org >> Subject: Re: [RFC PATCH 00/14] wifi: rtl8xxxu: Add AP mode support for 8188f >> >> Am 23.03.23 um 18:12 schrieb Bitterblue Smith: >>> On 22/03/2023 19:18, Martin Kaistra wrote: >>> Then I was able to turn on the AP and connect my phone to it. However, >>> the system froze after a few seconds. I had `journalctl --follow` >>> running. The last thing printed before the system froze was the DHCP >>> stuff (discover, offer, request, ack). The phone said it was connected, >>> but speedtest.net didn't have time to load before the freeze. >> >> Hi >> >> I could reproduce a frozen system with my hostapd setup, though it >> doesn't happen reliably and I don't have an error message when it happens. >> > > Using netcat would help to capture useful messages when system gets frozen. > > Ping-Ke > Thanks. I got a trace by using netconsole and netcat. It is a NULL pointer dereference in rtl8xxxu_fill_txdesc_v2(): if (rate_flags & IEEE80211_TX_RC_MCS && !ieee80211_is_mgmt(hdr->frame_control)) rate = tx_info->control.rates[0].idx + DESC_RATE_MCS0; else rate = tx_rate->hw_value; <-- error happens here if (rtl8xxxu_debug & RTL8XXXU_DEBUG_TX) dev_info(dev, "%s: TX rate: %d, pkt size %u\n", This happens when ieee80211_get_tx_rate() hits the WARN_ON_ONCE and maybe also when c->control.rates[0].idx has another invalid value. See my previous comments about HAS_RATE_CONTROL.
> -----Original Message----- > From: Martin Kaistra <martin.kaistra@linutronix.de> > Sent: Wednesday, April 5, 2023 11:31 PM > To: Bitterblue Smith <rtl8821cerfe2@gmail.com> > Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih > <pkshih@realtek.com>; Sebastian Andrzej Siewior <bigeasy@linutronix.de>; linux-wireless@vger.kernel.org > Subject: Re: [RFC PATCH 00/14] wifi: rtl8xxxu: Add AP mode support for 8188f > > Am 23.03.23 um 18:12 schrieb Bitterblue Smith: > > On 22/03/2023 19:18, Martin Kaistra wrote: > >> This series intends to bring AP mode support to the rtl8xxxu driver, > >> more specifically for the 8188f, because this is the HW I have. > >> The work is based on the vendor driver as I do not have access to > >> datasheets. > >> > >> This is an RFC, so that there can be a discussion first before > >> potentially implementing support for the other chips in this driver, if > >> required. > >> > > > > Hi! > > > > I ran into some problems while testing this. > > > > First, a null pointer dereference in rtl8xxxu_config_filter() when > > turning on the AP. priv->vif was NULL: > > > > if (priv->vif->type != NL80211_IFTYPE_AP) { > > > > I changed it like this: > > > > if (priv->vif && priv->vif->type != NL80211_IFTYPE_AP) { > > > > Then I was able to turn on the AP and connect my phone to it. However, > > the system froze after a few seconds. I had `journalctl --follow` > > running. The last thing printed before the system froze was the DHCP > > stuff (discover, offer, request, ack). The phone said it was connected, > > but speedtest.net didn't have time to load before the freeze. > > Hi > > I could reproduce a frozen system with my hostapd setup, though it > doesn't happen reliably and I don't have an error message when it happens. > > What I can see on the other hand, are WARNING messages which happen > sometimes in include/net/mac80211.h:2936 (ieee80211_get_tx_rate()). > This might be unrelated, I am not sure. > > Is this function even supposed to work in combination with > HAS_RATE_CONTROL set? I'm not familiar with rate control of mac80211, so I didn't have comment about rate control and HAS_RATE_CONTROL before. Simply checking rate_control_get_rate() that is to fill info->control.rates[], but it doesn't do it if HAS_RATE_CONTROL is set: if (ieee80211_hw_check(&sdata->local->hw, HAS_RATE_CONTROL)) return; So, I think it will not work with HAS_RATE_CONTROL set. Fortunately, rtl8xxxu only works on 2 GHz band, and only management frames use info->control.rates[] to decide TX rate, so always TX management frames with 1M CCK rate (hw_rate = 0) is fine. > Also, why are we putting rate into txdesc for all > packets (ie. also when USE_DRIVER_RATE is not set) when the firmware > sets the rate based on the rate_mask? Conceptually, if USE_DRIVER_RATE is set, rate info of txdesc is adopted. Otherwise, rate register controlled by firmware is adopted. That means if USE_DRIVER_RATE isn't set, rate info of txdesc is ignored. rtl8xxxu_update_rate_mask() is to tell firmware the all supported rate mask, and firmware choose proper TX and retry rate, and set to register. Ping-Ke
On 06/04/2023 18:09, Martin Kaistra wrote: > Am 06.04.23 um 02:42 schrieb Ping-Ke Shih: >> >> >>> -----Original Message----- >>> From: Martin Kaistra <martin.kaistra@linutronix.de> >>> Sent: Wednesday, April 5, 2023 11:31 PM >>> To: Bitterblue Smith <rtl8821cerfe2@gmail.com> >>> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih >>> <pkshih@realtek.com>; Sebastian Andrzej Siewior <bigeasy@linutronix.de>; linux-wireless@vger.kernel.org >>> Subject: Re: [RFC PATCH 00/14] wifi: rtl8xxxu: Add AP mode support for 8188f >>> >>> Am 23.03.23 um 18:12 schrieb Bitterblue Smith: >>>> On 22/03/2023 19:18, Martin Kaistra wrote: >>>> Then I was able to turn on the AP and connect my phone to it. However, >>>> the system froze after a few seconds. I had `journalctl --follow` >>>> running. The last thing printed before the system froze was the DHCP >>>> stuff (discover, offer, request, ack). The phone said it was connected, >>>> but speedtest.net didn't have time to load before the freeze. >>> >>> Hi >>> >>> I could reproduce a frozen system with my hostapd setup, though it >>> doesn't happen reliably and I don't have an error message when it happens. >>> >> >> Using netcat would help to capture useful messages when system gets frozen. >> >> Ping-Ke >> > > Thanks. I got a trace by using netconsole and netcat. It is a NULL pointer dereference in rtl8xxxu_fill_txdesc_v2(): > > > if (rate_flags & IEEE80211_TX_RC_MCS && > !ieee80211_is_mgmt(hdr->frame_control)) > rate = tx_info->control.rates[0].idx + DESC_RATE_MCS0; > else > rate = tx_rate->hw_value; <-- error happens here > > if (rtl8xxxu_debug & RTL8XXXU_DEBUG_TX) > dev_info(dev, "%s: TX rate: %d, pkt size %u\n", > > This happens when ieee80211_get_tx_rate() hits the WARN_ON_ONCE and maybe also when c->control.rates[0].idx has another invalid value. > See my previous comments about HAS_RATE_CONTROL. Good job finding the problem! ieee80211_get_tx_rate() is used since the initial commit, so only Jes may know why. I assume we only ever need to use the rate from mac80211 for frame injection (IEEE80211_TX_CTRL_RATE_INJECT, currently ignored). If c->control.rates[0].idx is negative, is c->control.rates[0].flags also unusable? ieee80211_get_rts_cts_rate() probably causes the same problem.
Am 09.04.23 um 14:41 schrieb Bitterblue Smith: > On 06/04/2023 18:09, Martin Kaistra wrote: >> Am 06.04.23 um 02:42 schrieb Ping-Ke Shih: >>> >>> >>>> -----Original Message----- >>>> From: Martin Kaistra <martin.kaistra@linutronix.de> >>>> Sent: Wednesday, April 5, 2023 11:31 PM >>>> To: Bitterblue Smith <rtl8821cerfe2@gmail.com> >>>> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih >>>> <pkshih@realtek.com>; Sebastian Andrzej Siewior <bigeasy@linutronix.de>; linux-wireless@vger.kernel.org >>>> Subject: Re: [RFC PATCH 00/14] wifi: rtl8xxxu: Add AP mode support for 8188f >>>> >>>> Am 23.03.23 um 18:12 schrieb Bitterblue Smith: >>>>> On 22/03/2023 19:18, Martin Kaistra wrote: >>>>> Then I was able to turn on the AP and connect my phone to it. However, >>>>> the system froze after a few seconds. I had `journalctl --follow` >>>>> running. The last thing printed before the system froze was the DHCP >>>>> stuff (discover, offer, request, ack). The phone said it was connected, >>>>> but speedtest.net didn't have time to load before the freeze. >>>> >>>> Hi >>>> >>>> I could reproduce a frozen system with my hostapd setup, though it >>>> doesn't happen reliably and I don't have an error message when it happens. >>>> >>> >>> Using netcat would help to capture useful messages when system gets frozen. >>> >>> Ping-Ke >>> >> >> Thanks. I got a trace by using netconsole and netcat. It is a NULL pointer dereference in rtl8xxxu_fill_txdesc_v2(): >> >> >> if (rate_flags & IEEE80211_TX_RC_MCS && >> !ieee80211_is_mgmt(hdr->frame_control)) >> rate = tx_info->control.rates[0].idx + DESC_RATE_MCS0; >> else >> rate = tx_rate->hw_value; <-- error happens here >> >> if (rtl8xxxu_debug & RTL8XXXU_DEBUG_TX) >> dev_info(dev, "%s: TX rate: %d, pkt size %u\n", >> >> This happens when ieee80211_get_tx_rate() hits the WARN_ON_ONCE and maybe also when c->control.rates[0].idx has another invalid value. >> See my previous comments about HAS_RATE_CONTROL. > > Good job finding the problem! > > ieee80211_get_tx_rate() is used since the initial commit, so only Jes > may know why. I assume we only ever need to use the rate from mac80211 > for frame injection (IEEE80211_TX_CTRL_RATE_INJECT, currently ignored). > > If c->control.rates[0].idx is negative, is c->control.rates[0].flags > also unusable? control.rates[0].flags seems to be 0 normally in all my tests when HAS_RATE_CONTROL is enabled, and when control.rates[0].idx is negative, I see some random values which do not look like real flags. > > ieee80211_get_rts_cts_rate() probably causes the same problem. Yes, I agree. How should proceed? I have a v2 of this patch series ready, do I need to add a patch to remove the calls to ieee80211_get_tx_rate() and just set rate in txdesc to 0 or should we handle this separately?
On 12/04/2023 13:02, Martin Kaistra wrote: > Am 09.04.23 um 14:41 schrieb Bitterblue Smith: >> On 06/04/2023 18:09, Martin Kaistra wrote: >>> Am 06.04.23 um 02:42 schrieb Ping-Ke Shih: >>>> >>>> >>>>> -----Original Message----- >>>>> From: Martin Kaistra <martin.kaistra@linutronix.de> >>>>> Sent: Wednesday, April 5, 2023 11:31 PM >>>>> To: Bitterblue Smith <rtl8821cerfe2@gmail.com> >>>>> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih >>>>> <pkshih@realtek.com>; Sebastian Andrzej Siewior <bigeasy@linutronix.de>; linux-wireless@vger.kernel.org >>>>> Subject: Re: [RFC PATCH 00/14] wifi: rtl8xxxu: Add AP mode support for 8188f >>>>> >>>>> Am 23.03.23 um 18:12 schrieb Bitterblue Smith: >>>>>> On 22/03/2023 19:18, Martin Kaistra wrote: >>>>>> Then I was able to turn on the AP and connect my phone to it. However, >>>>>> the system froze after a few seconds. I had `journalctl --follow` >>>>>> running. The last thing printed before the system froze was the DHCP >>>>>> stuff (discover, offer, request, ack). The phone said it was connected, >>>>>> but speedtest.net didn't have time to load before the freeze. >>>>> >>>>> Hi >>>>> >>>>> I could reproduce a frozen system with my hostapd setup, though it >>>>> doesn't happen reliably and I don't have an error message when it happens. >>>>> >>>> >>>> Using netcat would help to capture useful messages when system gets frozen. >>>> >>>> Ping-Ke >>>> >>> >>> Thanks. I got a trace by using netconsole and netcat. It is a NULL pointer dereference in rtl8xxxu_fill_txdesc_v2(): >>> >>> >>> if (rate_flags & IEEE80211_TX_RC_MCS && >>> !ieee80211_is_mgmt(hdr->frame_control)) >>> rate = tx_info->control.rates[0].idx + DESC_RATE_MCS0; >>> else >>> rate = tx_rate->hw_value; <-- error happens here >>> >>> if (rtl8xxxu_debug & RTL8XXXU_DEBUG_TX) >>> dev_info(dev, "%s: TX rate: %d, pkt size %u\n", >>> >>> This happens when ieee80211_get_tx_rate() hits the WARN_ON_ONCE and maybe also when c->control.rates[0].idx has another invalid value. >>> See my previous comments about HAS_RATE_CONTROL. >> >> Good job finding the problem! >> >> ieee80211_get_tx_rate() is used since the initial commit, so only Jes >> may know why. I assume we only ever need to use the rate from mac80211 >> for frame injection (IEEE80211_TX_CTRL_RATE_INJECT, currently ignored). >> >> If c->control.rates[0].idx is negative, is c->control.rates[0].flags >> also unusable? > > control.rates[0].flags seems to be 0 normally in all my tests when HAS_RATE_CONTROL is enabled, and when control.rates[0].idx is negative, I see some random values which do not look like real flags. > >> >> ieee80211_get_rts_cts_rate() probably causes the same problem. > > Yes, I agree. > > How should proceed? I have a v2 of this patch series ready, do I need to add a patch to remove the calls to ieee80211_get_tx_rate() and just set rate in txdesc to 0 or should we handle this separately? Adding a patch to the series sounds good to me. Remove the calls to ieee80211_get_tx_rate() and ieee80211_get_rts_cts_rate(), remove tx_info->control.rates[0].flags, send management and control frames with 1M. About ieee80211_get_rts_cts_rate(), we can go back to sending RTS with the 24M rate, like the vendor drivers do. It's unclear why commit a748a11038f8 ("rtl8xxxu: Obtain RTS rates from mac80211") changed this.
This series intends to bring AP mode support to the rtl8xxxu driver, more specifically for the 8188f, because this is the HW I have. The work is based on the vendor driver as I do not have access to datasheets. This is an RFC, so that there can be a discussion first before potentially implementing support for the other chips in this driver, if required. Also while doing some measurements with iperf3 to compare with the vendor driver, I saw, that TCP traffic from AP to STA is slower than in the vendor driver. For UDP it looks fine. I hope I can get some help to fix this. * vendor driver: without 802.11n: UDP (AP -> STA): 27 Mbits/sec UDP (STA -> AP): 33 Mbits/sec TCP (AP -> STA): 24 Mbits/sec TCP (STA -> AP): 26 Mbits/sec with 802.11n: UDP (AP -> STA): 51 Mbits/sec UDP (STA -> AP): 35 Mbits/sec TCP (AP -> STA): 40 Mbits/sec TCP (STA -> AP): 36 Mbits/sec * rtl8xxxu: without 802.11n: UDP (AP -> STA): 25 Mbits/sec UDP (STA -> AP): 31 Mbits/sec TCP (AP -> STA): 3 Mbits/sec ! TCP (STA -> AP): 25 Mbits/sec with 802.11n: UDP (AP -> STA): 41 Mbits/sec UDP (STA -> AP): 36 Mbits/sec TCP (AP -> STA): 3 Mbits/sec ! TCP (STA -> AP): 32 Mbits/sec Thanks, Martin Martin Kaistra (14): wifi: rtl8xxxu: Add start_ap() callback wifi: rtl8xxxu: Select correct queue for beacon frames wifi: rtl8xxxu: Add beacon functions wifi: rtl8xxxu: Add set_tim() callback wifi: rtl8xxxu: Allow setting rts threshold to -1 wifi: rtl8xxxu: Allow creating interface in AP mode wifi: rtl8xxxu: Add parameter macid to update_rate_mask wifi: rtl8xxxu: Actually use macid in rtl8xxxu_gen2_report_connect wifi: rtl8xxxu: Add parameter role to report_connect wifi: rtl8xxxu: Add sta_add() callback wifi: rtl8xxxu: Put the macid in txdesc wifi: rtl8xxxu: Enable hw seq for all non-qos frames wifi: rtl8xxxu: Clean up filter configuration wifi: rtl8xxxu: Declare AP mode support for 8188f .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 28 ++- .../realtek/rtl8xxxu/rtl8xxxu_8188e.c | 3 +- .../realtek/rtl8xxxu/rtl8xxxu_8188f.c | 1 + .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 226 +++++++++++++++--- .../wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h | 2 + 5 files changed, 222 insertions(+), 38 deletions(-)