diff mbox series

wifi: ath12k: Abort scan before removing link interface to prevent duplicate deletion

Message ID 20250124093352.481-1-quic_lingbok@quicinc.com
State New
Headers show
Series wifi: ath12k: Abort scan before removing link interface to prevent duplicate deletion | expand

Commit Message

Lingbo Kong Jan. 24, 2025, 9:33 a.m. UTC 
Currently, when ath12k performs the remove link interface operation, if
there is an ongoing scan operation on the arvif, ath12k may execute the
remove link interface operation multiple times on the same arvif. This
occurs because, during the remove link operation, if a scan operation is
present on the arvif, ath12k may receive a WMI_SCAN_EVENT_COMPLETED event
from the firmware. Upon receiving this event, ath12k will continue to
execute the ath12k_scan_vdev_clean_work() function, performing the remove
link interface operation on the same arvif again.

To address this issue, before executing the remove link interface
operation, ath12k needs to check if there is an ongoing scan operation on
the current arvif. If such an operation exists, it should be aborted.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Lingbo Kong <quic_lingbok@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/mac.c | 5 +++++
 1 file changed, 5 insertions(+)


base-commit: 376673aa393c1c232299be3e910d7f2e6d974b2f

Comments

Lingbo Kong Feb. 7, 2025, 9:04 a.m. UTC | #1 
On 2025/1/24 17:33, Lingbo Kong wrote:
> Currently, when ath12k performs the remove link interface operation, if
> there is an ongoing scan operation on the arvif, ath12k may execute the
> remove link interface operation multiple times on the same arvif. This
> occurs because, during the remove link operation, if a scan operation is
> present on the arvif, ath12k may receive a WMI_SCAN_EVENT_COMPLETED event
> from the firmware. Upon receiving this event, ath12k will continue to
> execute the ath12k_scan_vdev_clean_work() function, performing the remove
> link interface operation on the same arvif again.
> 
> To address this issue, before executing the remove link interface
> operation, ath12k needs to check if there is an ongoing scan operation on
> the current arvif. If such an operation exists, it should be aborted.
> 
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Signed-off-by: Lingbo Kong <quic_lingbok@quicinc.com>
> ---
>   drivers/net/wireless/ath/ath12k/mac.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index 329c05003721..cdce88787182 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -9333,6 +9333,11 @@ ath12k_mac_op_unassign_vif_chanctx(struct ieee80211_hw *hw,
>   	    ar->num_started_vdevs == 1 && ar->monitor_vdev_created)
>   		ath12k_mac_monitor_stop(ar);
>   
> +	if (ar->scan.arvif == arvif && ar->scan.state == ATH12K_SCAN_RUNNING) {
> +		ath12k_scan_abort(ar);
> +		ar->scan.arvif = NULL;
> +	}
> +
>   	ath12k_mac_remove_link_interface(hw, arvif);
>   	ath12k_mac_unassign_link_vif(arvif);
>   }
> 
> base-commit: 376673aa393c1c232299be3e910d7f2e6d974b2f

hi jeff,
do you have any updates on this patch?🙂

/lingbo
Jeff Johnson Feb. 7, 2025, 6:53 p.m. UTC | #2 
On 2/7/2025 1:04 AM, Lingbo Kong wrote:
> 
> 
> On 2025/1/24 17:33, Lingbo Kong wrote:
>> Currently, when ath12k performs the remove link interface operation, if
>> there is an ongoing scan operation on the arvif, ath12k may execute the
>> remove link interface operation multiple times on the same arvif. This
>> occurs because, during the remove link operation, if a scan operation is
>> present on the arvif, ath12k may receive a WMI_SCAN_EVENT_COMPLETED event
>> from the firmware. Upon receiving this event, ath12k will continue to
>> execute the ath12k_scan_vdev_clean_work() function, performing the remove
>> link interface operation on the same arvif again.
>>
>> To address this issue, before executing the remove link interface
>> operation, ath12k needs to check if there is an ongoing scan operation on
>> the current arvif. If such an operation exists, it should be aborted.
>>
>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>
>> Signed-off-by: Lingbo Kong <quic_lingbok@quicinc.com>
>> ---
>>   drivers/net/wireless/ath/ath12k/mac.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
>> index 329c05003721..cdce88787182 100644
>> --- a/drivers/net/wireless/ath/ath12k/mac.c
>> +++ b/drivers/net/wireless/ath/ath12k/mac.c
>> @@ -9333,6 +9333,11 @@ ath12k_mac_op_unassign_vif_chanctx(struct ieee80211_hw *hw,
>>   	    ar->num_started_vdevs == 1 && ar->monitor_vdev_created)
>>   		ath12k_mac_monitor_stop(ar);
>>   
>> +	if (ar->scan.arvif == arvif && ar->scan.state == ATH12K_SCAN_RUNNING) {
>> +		ath12k_scan_abort(ar);
>> +		ar->scan.arvif = NULL;
>> +	}
>> +
>>   	ath12k_mac_remove_link_interface(hw, arvif);
>>   	ath12k_mac_unassign_link_vif(arvif);
>>   }
>>
>> base-commit: 376673aa393c1c232299be3e910d7f2e6d974b2f
> 
> hi jeff,
> do you have any updates on this patch?🙂

while this patch applies cleanly to ath/main, it does not apply cleanly to the
current ath/pending, conflicting with:

https://msgid.link/20250204-unlink_link_arvif_from_chanctx-v2-8-764fb5973c1a@oss.qualcomm.com

that patch relocates the adjacent lines:
   	ath12k_mac_remove_link_interface(hw, arvif);
   	ath12k_mac_unassign_link_vif(arvif);

so should this logic stay in ath12k_mac_op_unassign_vif_chanctx() or should it
also be relocated to ath12k_mac_op_change_vif_links()?

/jeff
Lingbo Kong Feb. 10, 2025, 2:52 a.m. UTC | #3 
On 2025/2/8 2:53, Jeff Johnson wrote:
> On 2/7/2025 1:04 AM, Lingbo Kong wrote:
>>
>>
>> On 2025/1/24 17:33, Lingbo Kong wrote:
>>> Currently, when ath12k performs the remove link interface operation, if
>>> there is an ongoing scan operation on the arvif, ath12k may execute the
>>> remove link interface operation multiple times on the same arvif. This
>>> occurs because, during the remove link operation, if a scan operation is
>>> present on the arvif, ath12k may receive a WMI_SCAN_EVENT_COMPLETED event
>>> from the firmware. Upon receiving this event, ath12k will continue to
>>> execute the ath12k_scan_vdev_clean_work() function, performing the remove
>>> link interface operation on the same arvif again.
>>>
>>> To address this issue, before executing the remove link interface
>>> operation, ath12k needs to check if there is an ongoing scan operation on
>>> the current arvif. If such an operation exists, it should be aborted.
>>>
>>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>>
>>> Signed-off-by: Lingbo Kong <quic_lingbok@quicinc.com>
>>> ---
>>>    drivers/net/wireless/ath/ath12k/mac.c | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
>>> index 329c05003721..cdce88787182 100644
>>> --- a/drivers/net/wireless/ath/ath12k/mac.c
>>> +++ b/drivers/net/wireless/ath/ath12k/mac.c
>>> @@ -9333,6 +9333,11 @@ ath12k_mac_op_unassign_vif_chanctx(struct ieee80211_hw *hw,
>>>    	    ar->num_started_vdevs == 1 && ar->monitor_vdev_created)
>>>    		ath12k_mac_monitor_stop(ar);
>>>    
>>> +	if (ar->scan.arvif == arvif && ar->scan.state == ATH12K_SCAN_RUNNING) {
>>> +		ath12k_scan_abort(ar);
>>> +		ar->scan.arvif = NULL;
>>> +	}
>>> +
>>>    	ath12k_mac_remove_link_interface(hw, arvif);
>>>    	ath12k_mac_unassign_link_vif(arvif);
>>>    }
>>>
>>> base-commit: 376673aa393c1c232299be3e910d7f2e6d974b2f
>>
>> hi jeff,
>> do you have any updates on this patch?🙂
> 
> while this patch applies cleanly to ath/main, it does not apply cleanly to the
> current ath/pending, conflicting with:
> 
> https://msgid.link/20250204-unlink_link_arvif_from_chanctx-v2-8-764fb5973c1a@oss.qualcomm.com
> 
> that patch relocates the adjacent lines:
>     	ath12k_mac_remove_link_interface(hw, arvif);
>     	ath12k_mac_unassign_link_vif(arvif);
> 
> so should this logic stay in ath12k_mac_op_unassign_vif_chanctx() or should it
> also be relocated to ath12k_mac_op_change_vif_links()?
> 
> /jeff

hi, jeff, this logic should stay in ath12k_mac_op_unassign_vif_chanctx().

should i resend a new version of the patch based on 
https://msgid.link/20250204-unlink_link_arvif_from_chanctx-v2-8-764fb5973c1a@oss.qualcomm.com 
?

/lingbok
Jeff Johnson Feb. 11, 2025, 1:10 a.m. UTC | #4 
On 2/9/2025 6:52 PM, Lingbo Kong wrote:
> should i resend a new version of the patch based on 
> https://msgid.link/20250204-unlink_link_arvif_from_chanctx-v2-8-764fb5973c1a@oss.qualcomm.com 

suggest you wait until the pending branch is promoted

/jeff
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 329c05003721..cdce88787182 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -9333,6 +9333,11 @@  ath12k_mac_op_unassign_vif_chanctx(struct ieee80211_hw *hw,
 	    ar->num_started_vdevs == 1 && ar->monitor_vdev_created)
 		ath12k_mac_monitor_stop(ar);
 
+	if (ar->scan.arvif == arvif && ar->scan.state == ATH12K_SCAN_RUNNING) {
+		ath12k_scan_abort(ar);
+		ar->scan.arvif = NULL;
+	}
+
 	ath12k_mac_remove_link_interface(hw, arvif);
 	ath12k_mac_unassign_link_vif(arvif);
 }