| Message ID | 20250204122617.8976-1-Alexander@wetzel-home.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | wifi: mac80211: Debugfs handling for virtual monitor | expand |
On Tue, 2025-02-04 at 13:26 +0100, Alexander Wetzel wrote: > Create/delete debugfs entry for the virtual monitor interface according > to the state of the virtual monitor interface. > > This fixes the following issues: > 1) Debugfs is not getting an entry when the virtual monitor interface > is created. > 2) Instead debugfs gets an entry when it's deleted. > 3) And debugfs warns, that it already has directory for the virtual > monitor interface every time the virtual monitor interface is > removed again after the first time. > Started wondering, maybe it should just not have debugfs at all? That'd also avoid the potential name clash, and there isn't really anything of value there in the first place? johannes
On 04.02.25 13:53, Johannes Berg wrote: > On Tue, 2025-02-04 at 13:26 +0100, Alexander Wetzel wrote: >> Create/delete debugfs entry for the virtual monitor interface according >> to the state of the virtual monitor interface. >> >> This fixes the following issues: >> 1) Debugfs is not getting an entry when the virtual monitor interface >> is created. >> 2) Instead debugfs gets an entry when it's deleted. >> 3) And debugfs warns, that it already has directory for the virtual >> monitor interface every time the virtual monitor interface is >> removed again after the first time. >> > > Started wondering, maybe it should just not have debugfs at all? That'd > also avoid the potential name clash, and there isn't really anything of > value there in the first place? Knowing that we do have an internal virtual monitor interface may be interesting in itself. But that's hardly something we can call "real value":-) And that way we would solve any name conflict... Not creating it would just require us to skip ieee80211_debugfs_recreate_netdev() in drv_remove_interface() for virtual monitor. When you prefer that, I can send an alternate patch doing that. Do you want that as standalone patch or as a v2 of with a new name? Alexander
On Tue, 2025-02-04 at 14:51 +0100, Alexander Wetzel wrote: > > Started wondering, maybe it should just not have debugfs at all? That'd > > also avoid the potential name clash, and there isn't really anything of > > value there in the first place? > > Knowing that we do have an internal virtual monitor interface may be > interesting in itself. But that's hardly something we can call "real > value":-) And that way we would solve any name conflict... :) > Not creating it would just require us to skip > ieee80211_debugfs_recreate_netdev() in drv_remove_interface() for > virtual monitor. Right, should be simple. > When you prefer that, I can send an alternate patch doing that. > Do you want that as standalone patch or as a v2 of with a new name? I don't really care, let's do whatever makes more sense? In this case I'd argue it probably makes more sense as "remove debugfs dir for virtual monitor" or something like that? johannes
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c index 7d3ebfcb8c2b..bee11f5da16c 100644 --- a/net/mac80211/iface.c +++ b/net/mac80211/iface.c @@ -1153,8 +1153,6 @@ int ieee80211_add_virtual_monitor(struct ieee80211_local *local) } } - set_bit(SDATA_STATE_RUNNING, &sdata->state); - ret = ieee80211_check_queues(sdata, NL80211_IFTYPE_MONITOR); if (ret) { kfree(sdata); @@ -1177,6 +1175,9 @@ int ieee80211_add_virtual_monitor(struct ieee80211_local *local) return ret; } + ieee80211_debugfs_recreate_netdev(sdata, false); + set_bit(SDATA_STATE_RUNNING, &sdata->state); + skb_queue_head_init(&sdata->skb_queue); skb_queue_head_init(&sdata->status_queue); wiphy_work_init(&sdata->work, ieee80211_iface_work); @@ -1203,6 +1204,7 @@ void ieee80211_del_virtual_monitor(struct ieee80211_local *local) return; } + clear_bit(SDATA_STATE_RUNNING, &sdata->state); RCU_INIT_POINTER(local->monitor_sdata, NULL); mutex_unlock(&local->iflist_mtx); @@ -1213,6 +1215,7 @@ void ieee80211_del_virtual_monitor(struct ieee80211_local *local) if (ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF)) drv_remove_interface(local, sdata); + ieee80211_debugfs_remove_netdev(sdata); kfree(sdata); }
Create/delete debugfs entry for the virtual monitor interface according to the state of the virtual monitor interface. This fixes the following issues: 1) Debugfs is not getting an entry when the virtual monitor interface is created. 2) Instead debugfs gets an entry when it's deleted. 3) And debugfs warns, that it already has directory for the virtual monitor interface every time the virtual monitor interface is removed again after the first time. Signed-off-by: Alexander Wetzel <Alexander@wetzel-home.de> --- Changes compared to the RFC patch in https://lore.kernel.org/linux-wireless/20250127162625.20747-3-Alexander@wetzel-home.de/ Moved debugfs and SDATA_STATE_RUNNING to the end of ieee80211_add_virtual_monitor() --- net/mac80211/iface.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)