On Fri, 3 Nov 2023 06:55:47 +0100, Stanislaw Gruszka wrote:
>On Thu, Nov 02, 2023 at 08:36:04PM +0800, Shiji Yang wrote:
>> On Wed, 1 Nov 2023 10:07:10 +0100, Stanislaw Gruszka wrote:
>>
>> >On Sat, Oct 28, 2023 at 08:15:32PM +0800, Shiji Yang wrote:
>> >> When a hardware reset is triggered, all registers are reset, so all
>> >> queues are forced to stop in hardware interface. However, mac80211
>> >> will not automatically stop the queue. If we don't manually stop the
>> >> beacon queue, the queue will be deadlocked and unable to start again.
>> >> This patch fixes the issue where Apple devices cannot connect to the
>> >> AP after calling ieee80211_restart_hw().
>> >
>> >Should not this be solved in mac80211 then? ieee80211_restart_work
>> >does a lot o diffrent things, why beconing is not also
>> >stoped/started there ?
>> >
>> >Regards
>> >Stanislaw
>> >
>>
>> Hi! Thanks for your review.
>>
>> I think this issue is a bug of the rt2x00. When restart is called,
>Yes, I think you have right, this is rt2x00 issue.
>
>> mac80211 didn't call rt2x00mac_bss_info_changed() to update the
>> flag (This may be expected? I'm not sure. But all other Tx/Rx queues
>> are also manually disabled). And after resetting,
>> 'bss_conf->enable_beacon' and 'intf->enable_beacon' are still true.
>> Though mac80211 will call this function and try to enable the beacon
>> queue again. However, both 'if' and 'else if' blocks will never be
>> entered anymore because all conditions are false. This patch just
>> fixes this dead lock.
>Ok, I see.
>
>I don't remember how this supposed to work. I see we do
>
> for (i = 0; i < queue->limit; i++) {
> entry = &queue->entries[i];
> clear_bit(ENTRY_BCN_ASSIGNED, &entry->flags);
> }
>
>in rt2800_pre_reset_hw() But I think what should be done there is
>clear intf->enable_beacon for each interface.
Yes, idealy we should do that. But 'intf->enable_beacon' variable is
owned by mac80211, we can not access it here. So I can only say that
the current solution just a reasonable workaround. I made some little
changes in v2 patch will help developers to understand what happened
in rt2x00mac_bss_info_changed() after reset.
>
>Now I don't remember how I tested this, probably only in STA mode.
>
>> Maybe Kalle Valo knows if it's a mac80211 bug. This issue has been
>> here for several years.
>>
>> Looking forward to your reply.
>
>:-)
>
>> By the way, it seems that 'intf_beaconing' variable is useless. Does
>> it really can be increased to '2'? Maybe in multi ssid mode?
>
>Yes. When you can have multiple vif interfaces this variable
>can be bigger than 1. We advertise support for that for AP
>and mesh interfaces in rt2x00lib_set_if_combinations().
>
>> } else if (bss_conf->enable_beacon && !intf->enable_beacon) {
>> rt2x00dev->intf_beaconing++;
>> intf->enable_beacon = true;
>> /*
>> * Upload beacon to the H/W. This is only required on
>> * USB devices. PCI devices fetch beacons periodically.
>> */
>> if (rt2x00_is_usb(rt2x00dev))
>> rt2x00queue_update_beacon(rt2x00dev, vif);
>Hmm, maybe I also tested on AP USB, but don't remember.
>
>Thanks for explanations! Patch is ok for me.
>
>Regards
>Stanislaw
>
Regards,
Shiji Yang
