diff mbox series

wifi: mac80211: remove debugfs dir for virtual monitor

Message ID 20250204164240.370153-1-Alexander@wetzel-home.de
State New
Headers show
Series wifi: mac80211: remove debugfs dir for virtual monitor | expand

Commit Message

Alexander Wetzel Feb. 4, 2025, 4:42 p.m. UTC
Don't call ieee80211_debugfs_recreate_netdev() for virtual monitor
interface when deleting it.

The virtual monitor interface shouldn't have debugfs entries and trying
to update them will *create* them on deletion.

And when the virtual monitor interface is created/destroyed multiple
times we'll get warnings about debugfs name conflicts.

Signed-off-by: Alexander Wetzel <Alexander@wetzel-home.de>
---
That was a bit more tricky than expected:

local->monitor_sdata is normally already deleted when we call
drv_remove_interface. And we can't simply skip the call for all monitor
interfaces...

And one additional remark:
My iwlmvm card is not taking it well when I bring down the monitor interface
while TXing on it. The firmware crashes for my "Alder Lake-S PCH CNVi WiFi"
card.

But that also happens with unpatched kernels and so I don't care
much about that. I can make a bug report, if that's news and want you to
look into that.

Alexander
---
 net/mac80211/driver-ops.c | 10 ++++++++--
 net/mac80211/iface.c      | 11 ++++++-----
 2 files changed, 14 insertions(+), 7 deletions(-)

Comments

Jeff Johnson Feb. 26, 2025, 6:24 p.m. UTC | #1
On 2/4/2025 8:42 AM, Alexander Wetzel wrote:
> Don't call ieee80211_debugfs_recreate_netdev() for virtual monitor
> interface when deleting it.
> 
> The virtual monitor interface shouldn't have debugfs entries and trying
> to update them will *create* them on deletion.
> 
> And when the virtual monitor interface is created/destroyed multiple
> times we'll get warnings about debugfs name conflicts.
> 
> Signed-off-by: Alexander Wetzel <Alexander@wetzel-home.de>
> ---
> That was a bit more tricky than expected:
> 
> local->monitor_sdata is normally already deleted when we call
> drv_remove_interface. And we can't simply skip the call for all monitor
> interfaces...
> 
> And one additional remark:
> My iwlmvm card is not taking it well when I bring down the monitor interface
> while TXing on it. The firmware crashes for my "Alder Lake-S PCH CNVi WiFi"
> card.
> 
> But that also happens with unpatched kernels and so I don't care
> much about that. I can make a bug report, if that's news and want you to
> look into that.
> 
> Alexander
> ---
>  net/mac80211/driver-ops.c | 10 ++++++++--
>  net/mac80211/iface.c      | 11 ++++++-----
>  2 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/net/mac80211/driver-ops.c b/net/mac80211/driver-ops.c
> index 299d38e9e863..2fc60e1e77a5 100644
> --- a/net/mac80211/driver-ops.c
> +++ b/net/mac80211/driver-ops.c
> @@ -116,8 +116,14 @@ void drv_remove_interface(struct ieee80211_local *local,
>  
>  	sdata->flags &= ~IEEE80211_SDATA_IN_DRIVER;
>  
> -	/* Remove driver debugfs entries */
> -	ieee80211_debugfs_recreate_netdev(sdata, sdata->vif.valid_links);
> +	/*
> +	 * Remove driver debugfs entries.
> +	 * The virtual monitor interface doesn't get a debugfs
> +	 * entry, so it's exempt here.
> +	 */
> +	if (sdata != local->monitor_sdata)
> +		ieee80211_debugfs_recreate_netdev(sdata,
> +						  sdata->vif.valid_links);

With make W=1 C=1 I'm seeing the following which I think is a result of this
patch:

net/mac80211/driver-ops.c:124:19: error: incompatible types in comparison expression (different address spaces):
net/mac80211/driver-ops.c:124:19:    struct ieee80211_sub_if_data *
net/mac80211/driver-ops.c:124:19:    struct ieee80211_sub_if_data [noderef] __rcu *
Alexander Wetzel Feb. 27, 2025, 7:30 a.m. UTC | #2
>> Alexander
>> ---
>>   net/mac80211/driver-ops.c | 10 ++++++++--
>>   net/mac80211/iface.c      | 11 ++++++-----
>>   2 files changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/net/mac80211/driver-ops.c b/net/mac80211/driver-ops.c
>> index 299d38e9e863..2fc60e1e77a5 100644
>> --- a/net/mac80211/driver-ops.c
>> +++ b/net/mac80211/driver-ops.c
>> @@ -116,8 +116,14 @@ void drv_remove_interface(struct ieee80211_local *local,
>>   
>>   	sdata->flags &= ~IEEE80211_SDATA_IN_DRIVER;
>>   
>> -	/* Remove driver debugfs entries */
>> -	ieee80211_debugfs_recreate_netdev(sdata, sdata->vif.valid_links);
>> +	/*
>> +	 * Remove driver debugfs entries.
>> +	 * The virtual monitor interface doesn't get a debugfs
>> +	 * entry, so it's exempt here.
>> +	 */
>> +	if (sdata != local->monitor_sdata)
>> +		ieee80211_debugfs_recreate_netdev(sdata,
>> +						  sdata->vif.valid_links);
> 
> With make W=1 C=1 I'm seeing the following which I think is a result of this
> patch:
> 
> net/mac80211/driver-ops.c:124:19: error: incompatible types in comparison expression (different address spaces):
> net/mac80211/driver-ops.c:124:19:    struct ieee80211_sub_if_data *
> net/mac80211/driver-ops.c:124:19:    struct ieee80211_sub_if_data [noderef] __rcu *
> 
> 

That should be fixed with
https://lore.kernel.org/linux-wireless/20250213214330.6113-1-Alexander@wetzel-home.de/

It has been accepted in the last days, but looks like it was too late 
for wt-2025-02-25.

Alexander
diff mbox series

Patch

diff --git a/net/mac80211/driver-ops.c b/net/mac80211/driver-ops.c
index 299d38e9e863..2fc60e1e77a5 100644
--- a/net/mac80211/driver-ops.c
+++ b/net/mac80211/driver-ops.c
@@ -116,8 +116,14 @@  void drv_remove_interface(struct ieee80211_local *local,
 
 	sdata->flags &= ~IEEE80211_SDATA_IN_DRIVER;
 
-	/* Remove driver debugfs entries */
-	ieee80211_debugfs_recreate_netdev(sdata, sdata->vif.valid_links);
+	/*
+	 * Remove driver debugfs entries.
+	 * The virtual monitor interface doesn't get a debugfs
+	 * entry, so it's exempt here.
+	 */
+	if (sdata != local->monitor_sdata)
+		ieee80211_debugfs_recreate_netdev(sdata,
+						  sdata->vif.valid_links);
 
 	trace_drv_remove_interface(local, sdata);
 	local->ops->remove_interface(&local->hw, &sdata->vif);
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 7d3ebfcb8c2b..e627e2df3038 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1203,16 +1203,17 @@  void ieee80211_del_virtual_monitor(struct ieee80211_local *local)
 		return;
 	}
 
-	RCU_INIT_POINTER(local->monitor_sdata, NULL);
-	mutex_unlock(&local->iflist_mtx);
-
-	synchronize_net();
-
+	clear_bit(SDATA_STATE_RUNNING, &sdata->state);
 	ieee80211_link_release_channel(&sdata->deflink);
 
 	if (ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF))
 		drv_remove_interface(local, sdata);
 
+	RCU_INIT_POINTER(local->monitor_sdata, NULL);
+	mutex_unlock(&local->iflist_mtx);
+
+	synchronize_net();
+
 	kfree(sdata);
 }