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@xxxxxxxxxxxxx>
>>>>> Sent: Wednesday, April 5, 2023 11:31 PM
>>>>> To: Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx>
>>>>> Cc: Jes Sorensen <Jes.Sorensen@xxxxxxxxx>; Kalle Valo <kvalo@xxxxxxxxxx>; Ping-Ke Shih
>>>>> <pkshih@xxxxxxxxxxx>; Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>; linux-wireless@xxxxxxxxxxxxxxx
>>>>> 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.
