Re: [PATCH v2] wifi: mac80211: re-order unassigning channel in activate links

[Aditya Kumar Singh <quic_adisi@quicinc.com>]

On 12/6/24 15:37, Johannes Berg wrote:
> On Thu, 2024-12-05 at 20:43 +0530, Aditya Kumar Singh wrote:
>> On 12/5/24 18:30, Johannes Berg wrote:
>>> On Thu, 2024-12-05 at 12:43 +0100, Johannes Berg wrote:
>>>>>
>>>>> Therefore, re-order the logic so that stations are handled first and then
>>>>> channel is unassigned.
>>>>>
>>>>
>>>> This causes memory leaks in my tests with iwlwifi.
>>>>
>>>
>>> And also firmware crashes because the station is removed while it's
>>> still being used.
>>>
>>
>> So is this exposing some underlying issue with iwlwifi?
> 
> I don't think so?
> 
>> Or this change
>> will break drivers which does not group multiple hardware into single
>> wiphy?
> 
> Not necessarily, but it breaks iwlwifi because of the changed order of
> operations, and what it does with the firmware.
> 
> I think the issue here is that we treat link active == link has channel
> context in iwlwifi, and an active link in client mode requires a station
> in firmware. Otherwise you cannot even deactivate a link, since that
> requires sending an NDP to the AP, but if you don't have the AP STA you
> can't do that ...

Fair enough ...

> 
> I guess the driver could be changed to treat station links as active
> when they have the AP STA entry, but that seems ... difficult and
> strange, it would make it different between AP and client modes?
> 
> Looking at your commit message more, I wonder if it really even makes
> sense to *delete* the link when the channel context is unassigned,
> rather than (similarly to iwlwifi) deactivating it and deleting it later
> when it's actually removed (change_vif_links)? You do know which
> hardware it is/was on, after all. And these two operations can *never*
> be atomic. Removing the STAs first might be something that's appropriate
> for AP mode, but I guess I'm more with iwlwifi here in that it doesn't
> seem quite right for client mode?

I see your point. I need to experiment and see whether this way works or 
not for ath12k. Let me try that out.

> 
>> Also, how about non-ML scenario in iwlwifi? There, first station is
>> removed and then the interface goes down right?
> 
> It's not so much about the interface but the link, it seems.
> 

Sure..


-- 
Aditya