| Message ID | 20240626045216.3754013-9-quic_adisi@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | wifi: cfg80211/mac80211: add DFS support for MLO | expand |
On Wed, 2024-06-26 at 10:22 +0530, Aditya Kumar Singh wrote: > > Hence, in order to support DFS with MLO, do the following changes - > * Add channel pointer as an argument to the function > ieee80211_radar_detected(). During MLO, drivers would have to pass on > which channel radar is detected. Makes sense, maybe? > * In order to pass on this channel information to the radar detected > worker later on, introduce a linked list 'radar_info' in the structure > local. > * When driver calls radar detected, a node is created and added to this > list and work is scheduled. The work handler takes care to process each > node and take further action. Not sure I like that so much, it adds book-keeping and all kinds of extra things. Couldn't we just have a flag in the channel context or so - there must be one, after all? And perhaps pass the chanctx from the driver instead of the channel? Actually, we're already having to do a channel/chanctx lookup in ieee80211_dfs_radar_detected_work() so it seems pretty weird to add more complex logic to it... Please consider just passing the chanctx, and then we can set a flag there, and not have any of this. johannes
On 6/26/24 18:05, Johannes Berg wrote: > On Wed, 2024-06-26 at 10:22 +0530, Aditya Kumar Singh wrote: >> >> Hence, in order to support DFS with MLO, do the following changes - >> * Add channel pointer as an argument to the function >> ieee80211_radar_detected(). During MLO, drivers would have to pass on >> which channel radar is detected. > > Makes sense, maybe? > >> * In order to pass on this channel information to the radar detected >> worker later on, introduce a linked list 'radar_info' in the structure >> local. >> * When driver calls radar detected, a node is created and added to this >> list and work is scheduled. The work handler takes care to process each >> node and take further action. > > Not sure I like that so much, it adds book-keeping and all kinds of > extra things. > > Couldn't we just have a flag in the channel context or so - there must > be one, after all? And perhaps pass the chanctx from the driver instead > of the channel? Not really. So this linked list thing came into picture for drivers supporting split-band 5 GHz wiphy and both of them are grouped together for MLO. Now, each one of them will use different chanctx as such and there is a possibility of radar being detected simultaneously. > > Actually, we're already having to do a channel/chanctx lookup in > ieee80211_dfs_radar_detected_work() so it seems pretty weird to add more > complex logic to it... > > Please consider just passing the chanctx, and then we can set a flag > there, and not have any of this. Could do but, logic in worker will be little bit complex? for each ctx in local: if ctx radar_detected flag is set: append to local ctx list/array num_ctx++ if num_ctx > 1 : if wiphy supports mlo: for each local ctx list/array: call cfg80211_radar_event with the ctx chandef else: warn that mulit channel is not supported else: call cfg80211_radar_event with the first element in local ctx list/array chandef ----- This is because, in split-band devices, ieee80211_radar_detected can be called simultaneously with different channel contexts and then there is a possibility that before worker gets a chance to execute, both of the calls have marked their chanctx radar detected flags. - Aditya
On Thu, 2024-06-27 at 09:38 +0530, Aditya Kumar Singh wrote: > > Couldn't we just have a flag in the channel context or so - there must > > be one, after all? And perhaps pass the chanctx from the driver instead > > of the channel? > > Not really. So this linked list thing came into picture for drivers > supporting split-band 5 GHz wiphy and both of them are grouped together > for MLO. Now, each one of them will use different chanctx as such and > there is a possibility of radar being detected simultaneously. Right so ... I don't see how that answers my question in the negative? You necessarily have a channel context for each channel you're listening (even for radar) on [unless you have the extra radar chain API thing], so if radar is detected, can't you just set a flag in that chanctx and kick off the processing? > Could do but, logic in worker will be little bit complex? > > for each ctx in local: > if ctx radar_detected flag is set: > append to local ctx list/array > num_ctx++ > > if num_ctx > 1 : > if wiphy supports mlo: > for each local ctx list/array: > call cfg80211_radar_event with the ctx chandef > else: > warn that mulit channel is not supported > else: > call cfg80211_radar_event with the first element in local ctx > list/array chandef > > ----- > > This is because, in split-band devices, ieee80211_radar_detected can be > called simultaneously with different channel contexts and then there is > a possibility that before worker gets a chance to execute, both of the > calls have marked their chanctx radar detected flags. Yeah, but no need to make that complex - simply doing for each ctx in local: if ctx radar_detected flag is set: call cfg80211_radar_event() with ctx chandef clear ctx radar_detected flag no? You don't have to restrict the worker to processing a single one, in fact you must not since scheduling it again while scheduled (and not yet running) won't run it again. And if it races then worst case you have a worker run that does nothing? johannes
On 6/28/24 14:29, Johannes Berg wrote: > On Thu, 2024-06-27 at 09:38 +0530, Aditya Kumar Singh wrote: >>> Couldn't we just have a flag in the channel context or so - there must >>> be one, after all? And perhaps pass the chanctx from the driver instead >>> of the channel? >> >> Not really. So this linked list thing came into picture for drivers >> supporting split-band 5 GHz wiphy and both of them are grouped together >> for MLO. Now, each one of them will use different chanctx as such and >> there is a possibility of radar being detected simultaneously. > > Right so ... I don't see how that answers my question in the negative? > You necessarily have a channel context for each channel you're listening > (even for radar) on [unless you have the extra radar chain API thing], > so if radar is detected, can't you just set a flag in that chanctx and > kick off the processing? > Ah! okay. Now I see your point. Sorry partially misunderstood earlier. Yeah this can be done. >> Could do but, logic in worker will be little bit complex? >> >> for each ctx in local: >> if ctx radar_detected flag is set: >> append to local ctx list/array >> num_ctx++ >> >> if num_ctx > 1 : >> if wiphy supports mlo: >> for each local ctx list/array: >> call cfg80211_radar_event with the ctx chandef >> else: >> warn that mulit channel is not supported >> else: >> call cfg80211_radar_event with the first element in local ctx >> list/array chandef >> >> ----- >> >> This is because, in split-band devices, ieee80211_radar_detected can be >> called simultaneously with different channel contexts and then there is >> a possibility that before worker gets a chance to execute, both of the >> calls have marked their chanctx radar detected flags. > > Yeah, but no need to make that complex - simply doing > > for each ctx in local: > if ctx radar_detected flag is set: > call cfg80211_radar_event() with ctx chandef > clear ctx radar_detected flag > > no? > Yes got it. Thanks for clearing it out. > You don't have to restrict the worker to processing a single one, in > fact you must not since scheduling it again while scheduled (and not yet > running) won't run it again. And if it races then worst case you have a > worker run that does nothing? > Yes, true that. Sure so let me incorporate these changes and send next version soon. Thanks for the suggestions. - Aditya
On 6/26/24 18:05, Johannes Berg wrote: > On Wed, 2024-06-26 at 10:22 +0530, Aditya Kumar Singh wrote: >> >> Hence, in order to support DFS with MLO, do the following changes - >> * Add channel pointer as an argument to the function >> ieee80211_radar_detected(). During MLO, drivers would have to pass on >> which channel radar is detected. > > Makes sense, maybe? > >> * In order to pass on this channel information to the radar detected >> worker later on, introduce a linked list 'radar_info' in the structure >> local. >> * When driver calls radar detected, a node is created and added to this >> list and work is scheduled. The work handler takes care to process each >> node and take further action. > > Not sure I like that so much, it adds book-keeping and all kinds of > extra things. > > Couldn't we just have a flag in the channel context or so - there must > be one, after all? And perhaps pass the chanctx from the driver instead > of the channel? > > Actually, we're already having to do a channel/chanctx lookup in > ieee80211_dfs_radar_detected_work() so it seems pretty weird to add more > complex logic to it... > > Please consider just passing the chanctx, and then we can set a flag > there, and not have any of this. > So I was trying to implement above suggestion, and I see this - * Drivers don't have chanctx visible in it. Driver visible part is ieee80211_chanctx_conf (stored in chanctx) * In order to pass this from driver, I need to access each driver's per interface struct which should have ieee80211_vif. This will have bss_conf pointer and in turn which will have chanctx_conf pointer. (per_vif_struct)->vif->bss_conf->chanctx_conf. * I see for many drivers the place where ieee80211_radar_detected() is called, the interface level struct is not present. So making chanctx_conf mandatory argument to pass requires a lot of code changes across the drivers. * So in order to keep things simple, we'd have to allow drivers to pass NULL and let the current logic kick in. Iterate over all ctxs and all those. * If driver passes chanctx_conf, then while going through the ctx, if the flag is set, further process can immediately kick in and other num_ctx checks will be ignored. * Now if driver has clubbed multiple hardwares under single wiphy due to which num_ctx will be greater than 1, obviously such drivers are bound to pass a valid chanctx_conf or else the event will be dropped. Sounds fine?
On Sun, 2024-07-07 at 10:42 +0530, Aditya Kumar Singh wrote: > > So I was trying to implement above suggestion, and I see this - > > * Drivers don't have chanctx visible in it. Driver visible part is > ieee80211_chanctx_conf (stored in chanctx) Sure, but you can go back and forth from these in mac80211, so that's not really an issue. > * In order to pass this from driver, I need to access each driver's per > interface struct which should have ieee80211_vif. This will have > bss_conf pointer and in turn which will have chanctx_conf pointer. > (per_vif_struct)->vif->bss_conf->chanctx_conf. Depends on the driver, but maybe, yes. > * I see for many drivers the place where ieee80211_radar_detected() is > called, the interface level struct is not present. So making > chanctx_conf mandatory argument to pass requires a lot of code changes > across the drivers. > > * So in order to keep things simple, we'd have to allow drivers to pass > NULL and let the current logic kick in. Iterate over all ctxs and all those. Seems reasonable. I'd even go so far as saying that you get a WARN_ON if there are multiple at that point if NULL was passed, and we just set the flag on *all* of them since we can't know which was intended? For current drivers that's all fine, and for MLO/multi-radio drivers they'd just need to ensure they pass the struct. Perhaps even WARN immediately it if it's a multi-radio driver? Though you can't do that yet since I haven't landed that series yet, but I will soon. > * If driver passes chanctx_conf, then while going through the ctx, if > the flag is set, further process can immediately kick in and other > num_ctx checks will be ignored. > > * Now if driver has clubbed multiple hardwares under single wiphy due to > which num_ctx will be greater than 1, obviously such drivers are bound > to pass a valid chanctx_conf or else the event will be dropped. > > Sounds fine? > Sure! johannes
On 7/8/24 15:02, Johannes Berg wrote: > On Sun, 2024-07-07 at 10:42 +0530, Aditya Kumar Singh wrote: >> >> So I was trying to implement above suggestion, and I see this - >> >> * Drivers don't have chanctx visible in it. Driver visible part is >> ieee80211_chanctx_conf (stored in chanctx) > > Sure, but you can go back and forth from these in mac80211, so that's > not really an issue. > >> * In order to pass this from driver, I need to access each driver's per >> interface struct which should have ieee80211_vif. This will have >> bss_conf pointer and in turn which will have chanctx_conf pointer. >> (per_vif_struct)->vif->bss_conf->chanctx_conf. > > Depends on the driver, but maybe, yes. > >> * I see for many drivers the place where ieee80211_radar_detected() is >> called, the interface level struct is not present. So making >> chanctx_conf mandatory argument to pass requires a lot of code changes >> across the drivers. >> >> * So in order to keep things simple, we'd have to allow drivers to pass >> NULL and let the current logic kick in. Iterate over all ctxs and all those. > > Seems reasonable. I'd even go so far as saying that you get a WARN_ON if > there are multiple at that point if NULL was passed, and we just set the > flag on *all* of them since we can't know which was intended? > > For current drivers that's all fine, and for MLO/multi-radio drivers > they'd just need to ensure they pass the struct. > > Perhaps even WARN immediately it if it's a multi-radio driver? Though > you can't do that yet since I haven't landed that series yet, but I will > soon. > Yup got it. Will do as discussed and send next version soon. >> * If driver passes chanctx_conf, then while going through the ctx, if >> the flag is set, further process can immediately kick in and other >> num_ctx checks will be ignored. >> >> * Now if driver has clubbed multiple hardwares under single wiphy due to >> which num_ctx will be greater than 1, obviously such drivers are bound >> to pass a valid chanctx_conf or else the event will be dropped. >> >> Sounds fine? >> > > Sure! :) Thanks for your inputs. > > johannes
On 7/8/24 15:20, Aditya Kumar Singh wrote: > On 7/8/24 15:02, Johannes Berg wrote: >> On Sun, 2024-07-07 at 10:42 +0530, Aditya Kumar Singh wrote: >>> >>> So I was trying to implement above suggestion, and I see this - >>> >>> * Drivers don't have chanctx visible in it. Driver visible part is >>> ieee80211_chanctx_conf (stored in chanctx) >> >> Sure, but you can go back and forth from these in mac80211, so that's >> not really an issue. >> >>> * In order to pass this from driver, I need to access each driver's per >>> interface struct which should have ieee80211_vif. This will have >>> bss_conf pointer and in turn which will have chanctx_conf pointer. >>> (per_vif_struct)->vif->bss_conf->chanctx_conf. >> >> Depends on the driver, but maybe, yes. >> >>> * I see for many drivers the place where ieee80211_radar_detected() is >>> called, the interface level struct is not present. So making >>> chanctx_conf mandatory argument to pass requires a lot of code changes >>> across the drivers. >>> >>> * So in order to keep things simple, we'd have to allow drivers to pass >>> NULL and let the current logic kick in. Iterate over all ctxs and all >>> those. >> >> Seems reasonable. I'd even go so far as saying that you get a WARN_ON if >> there are multiple at that point if NULL was passed, and we just set the >> flag on *all* of them since we can't know which was intended? >> >> For current drivers that's all fine, and for MLO/multi-radio drivers >> they'd just need to ensure they pass the struct. >> >> Perhaps even WARN immediately it if it's a multi-radio driver? Though >> you can't do that yet since I haven't landed that series yet, but I will >> soon. >> > > Yup got it. Will do as discussed and send next version soon. > So, I was trying as discussed above but I see concurrency issue now. In order to mark or let's say to iterate over *all* ctxs (in case NULL is passed by driver), that part of code ideally should be under wiphy lock right? But ieee80211_radar_detected() is called in an interrupt context. Hence, can not take wiphy lock there :( Now, obviously there is a work scheduled from ieee80211_radar_detected() but to make the information (chanctx) available in the worker, we need some infra to pass that info, correct? And given ieee80211_radar_detected() can be called multiple times back to back, we can not just have one member in some struct or else that will get over -written with the latest called chanctx. So in the end, looks like linked list only is useful? On every ieee80211_radar_detected() call, add a node to a head pointer and schedule the work if not already. In worker, go through all the nodes and get the required chanctx and process further. For drivers which will still pass NULL (non-MLO), for them go with the existing logic. >>> * If driver passes chanctx_conf, then while going through the ctx, if >>> the flag is set, further process can immediately kick in and other >>> num_ctx checks will be ignored. >>> >>> * Now if driver has clubbed multiple hardwares under single wiphy due to >>> which num_ctx will be greater than 1, obviously such drivers are bound >>> to pass a valid chanctx_conf or else the event will be dropped. >>> >>> Sounds fine? >>> >> >> Sure! > > :) Thanks for your inputs. > >> >> johannes >
On Tue, 2024-07-09 at 14:24 +0530, Aditya Kumar Singh wrote: > > So, I was trying as discussed above but I see concurrency issue now. > > In order to mark or let's say to iterate over *all* ctxs (in case NULL > is passed by driver), that part of code ideally should be under wiphy > lock right? But ieee80211_radar_detected() is called in an interrupt > context. Hence, can not take wiphy lock there :( Not necessarily, ieee80211_iter_chan_contexts_atomic() also iterates with RCU protection, for example. johannes
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c index b93a64bf8190..35bfe7232e95 100644 --- a/drivers/net/wireless/ath/ath10k/debug.c +++ b/drivers/net/wireless/ath/ath10k/debug.c @@ -3,7 +3,7 @@ * Copyright (c) 2005-2011 Atheros Communications Inc. * Copyright (c) 2011-2017 Qualcomm Atheros, Inc. * Copyright (c) 2018, The Linux Foundation. All rights reserved. - * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. + * Copyright (c) 2022, 2024 Qualcomm Innovation Center, Inc. All rights reserved. */ #include <linux/module.h> @@ -1774,7 +1774,7 @@ static ssize_t ath10k_write_simulate_radar(struct file *file, if (!arvif->is_started) return -EINVAL; - ieee80211_radar_detected(ar->hw); + ieee80211_radar_detected(ar->hw, NULL); return count; } diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c index 3bf67b2ecd6d..21109e0842f1 100644 --- a/drivers/net/wireless/ath/ath10k/mac.c +++ b/drivers/net/wireless/ath/ath10k/mac.c @@ -1437,7 +1437,7 @@ static void ath10k_recalc_radar_detection(struct ath10k *ar) * by indicating that radar was detected. */ ath10k_warn(ar, "failed to start CAC: %d\n", ret); - ieee80211_radar_detected(ar->hw); + ieee80211_radar_detected(ar->hw, NULL); } } diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c index fe2344598364..4861179b2217 100644 --- a/drivers/net/wireless/ath/ath10k/wmi.c +++ b/drivers/net/wireless/ath/ath10k/wmi.c @@ -3990,7 +3990,7 @@ static void ath10k_radar_detected(struct ath10k *ar) if (ar->dfs_block_radar_events) ath10k_info(ar, "DFS Radar detected, but ignored as requested\n"); else - ieee80211_radar_detected(ar->hw); + ieee80211_radar_detected(ar->hw, NULL); } static void ath10k_radar_confirmation_work(struct work_struct *work) diff --git a/drivers/net/wireless/ath/ath11k/wmi.c b/drivers/net/wireless/ath/ath11k/wmi.c index 38f175dd1557..8839825c22fa 100644 --- a/drivers/net/wireless/ath/ath11k/wmi.c +++ b/drivers/net/wireless/ath/ath11k/wmi.c @@ -8356,7 +8356,7 @@ ath11k_wmi_pdev_dfs_radar_detected_event(struct ath11k_base *ab, struct sk_buff if (ar->dfs_block_radar_events) ath11k_info(ab, "DFS Radar detected, but ignored as requested\n"); else - ieee80211_radar_detected(ar->hw); + ieee80211_radar_detected(ar->hw, NULL); exit: rcu_read_unlock(); diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c index d6e1d1398cdb..013f46ab982a 100644 --- a/drivers/net/wireless/ath/ath12k/wmi.c +++ b/drivers/net/wireless/ath/ath12k/wmi.c @@ -6785,7 +6785,7 @@ ath12k_wmi_pdev_dfs_radar_detected_event(struct ath12k_base *ab, struct sk_buff if (ar->dfs_block_radar_events) ath12k_info(ab, "DFS Radar detected, but ignored as requested\n"); else - ieee80211_radar_detected(ath12k_ar_to_hw(ar)); + ieee80211_radar_detected(ath12k_ar_to_hw(ar), NULL); exit: rcu_read_unlock(); diff --git a/drivers/net/wireless/ath/ath9k/dfs.c b/drivers/net/wireless/ath/ath9k/dfs.c index 11349218bc21..3689e12db9f7 100644 --- a/drivers/net/wireless/ath/ath9k/dfs.c +++ b/drivers/net/wireless/ath/ath9k/dfs.c @@ -280,7 +280,7 @@ ath9k_dfs_process_radar_pulse(struct ath_softc *sc, struct pulse_event *pe) if (!pd->add_pulse(pd, pe, NULL)) return; DFS_STAT_INC(sc, radar_detected); - ieee80211_radar_detected(sc->hw); + ieee80211_radar_detected(sc->hw, NULL); } /* diff --git a/drivers/net/wireless/ath/ath9k/dfs_debug.c b/drivers/net/wireless/ath/ath9k/dfs_debug.c index 8e18e9b4ef48..426caa057396 100644 --- a/drivers/net/wireless/ath/ath9k/dfs_debug.c +++ b/drivers/net/wireless/ath/ath9k/dfs_debug.c @@ -116,7 +116,7 @@ static ssize_t write_file_simulate_radar(struct file *file, { struct ath_softc *sc = file->private_data; - ieee80211_radar_detected(sc->hw); + ieee80211_radar_detected(sc->hw, NULL); return count; } diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c index c807bd8d928d..282d9eb336bb 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c @@ -394,7 +394,7 @@ mt7615_mcu_rx_radar_detected(struct mt7615_dev *dev, struct sk_buff *skb) if (mt76_phy_dfs_state(mphy) < MT_DFS_STATE_CAC) return; - ieee80211_radar_detected(mphy->hw); + ieee80211_radar_detected(mphy->hw, NULL); dev->hw_pattern++; } diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_dfs.c b/drivers/net/wireless/mediatek/mt76/mt76x02_dfs.c index 024a5c0a5a57..7a07636d09c6 100644 --- a/drivers/net/wireless/mediatek/mt76/mt76x02_dfs.c +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_dfs.c @@ -630,7 +630,7 @@ static void mt76x02_dfs_tasklet(struct tasklet_struct *t) radar_detected = mt76x02_dfs_check_detection(dev); if (radar_detected) { /* sw detector rx radar pattern */ - ieee80211_radar_detected(dev->mt76.hw); + ieee80211_radar_detected(dev->mt76.hw, NULL); mt76x02_dfs_detector_reset(dev); return; @@ -658,7 +658,7 @@ static void mt76x02_dfs_tasklet(struct tasklet_struct *t) /* hw detector rx radar pattern */ dfs_pd->stats[i].hw_pattern++; - ieee80211_radar_detected(dev->mt76.hw); + ieee80211_radar_detected(dev->mt76.hw, NULL); mt76x02_dfs_detector_reset(dev); return; diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c index 9599adf104b1..35fdd7df0a1e 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c @@ -293,7 +293,7 @@ mt7915_mcu_rx_radar_detected(struct mt7915_dev *dev, struct sk_buff *skb) &dev->rdd2_chandef, GFP_ATOMIC); else - ieee80211_radar_detected(mphy->hw); + ieee80211_radar_detected(mphy->hw, NULL); dev->hw_pattern++; } diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c index 2c8578677800..bae7b2eeb02d 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c @@ -371,7 +371,7 @@ mt7996_mcu_rx_radar_detected(struct mt7996_dev *dev, struct sk_buff *skb) &dev->rdd2_chandef, GFP_ATOMIC); else - ieee80211_radar_detected(mphy->hw); + ieee80211_radar_detected(mphy->hw, NULL); dev->hw_pattern++; } diff --git a/drivers/net/wireless/ti/wl18xx/event.c b/drivers/net/wireless/ti/wl18xx/event.c index 34d95f458e1a..a9f090e15cbb 100644 --- a/drivers/net/wireless/ti/wl18xx/event.c +++ b/drivers/net/wireless/ti/wl18xx/event.c @@ -142,7 +142,7 @@ int wl18xx_process_mailbox_events(struct wl1271 *wl) wl18xx_radar_type_decode(mbox->radar_type)); if (!wl->radar_debug_mode) - ieee80211_radar_detected(wl->hw); + ieee80211_radar_detected(wl->hw, NULL); } if (vector & PERIODIC_SCAN_REPORT_EVENT_ID) { diff --git a/drivers/net/wireless/virtual/mac80211_hwsim.c b/drivers/net/wireless/virtual/mac80211_hwsim.c index fbf24870209d..f405acdfd4e6 100644 --- a/drivers/net/wireless/virtual/mac80211_hwsim.c +++ b/drivers/net/wireless/virtual/mac80211_hwsim.c @@ -1137,7 +1137,7 @@ static int hwsim_write_simulate_radar(void *dat, u64 val) { struct mac80211_hwsim_data *data = dat; - ieee80211_radar_detected(data->hw); + ieee80211_radar_detected(data->hw, NULL); return 0; } diff --git a/include/net/mac80211.h b/include/net/mac80211.h index ecfa65ade226..f46f61c8e3db 100644 --- a/include/net/mac80211.h +++ b/include/net/mac80211.h @@ -6724,8 +6724,11 @@ void ieee80211_cqm_beacon_loss_notify(struct ieee80211_vif *vif, gfp_t gfp); * ieee80211_radar_detected - inform that a radar was detected * * @hw: pointer as obtained from ieee80211_alloc_hw() + * @radar_channel: Channel pointer on which radar is detected. Mandatory to + * pass a valid pointer during MLO. For non-MLO %NULL can be passed */ -void ieee80211_radar_detected(struct ieee80211_hw *hw); +void ieee80211_radar_detected(struct ieee80211_hw *hw, + struct ieee80211_channel *radar_channel); /** * ieee80211_chswitch_done - Complete channel switch process diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index 4af0f9bd434d..0e6a54f03eb7 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -1329,6 +1329,11 @@ enum mac80211_scan_state { DECLARE_STATIC_KEY_FALSE(aql_disable); +struct radar_info { + struct list_head list; + struct ieee80211_channel *channel; +}; + struct ieee80211_local { /* embed the driver visible part. * don't cast (use the static inlines below), but we keep @@ -1430,6 +1435,7 @@ struct ieee80211_local { bool wowlan; struct wiphy_work radar_detected_work; + struct list_head radar_info_list; /* number of RX chains the hardware has */ u8 rx_chains; diff --git a/net/mac80211/main.c b/net/mac80211/main.c index c8cb091b5ea3..30b383f1e27d 100644 --- a/net/mac80211/main.c +++ b/net/mac80211/main.c @@ -943,6 +943,7 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len, INIT_LIST_HEAD(&local->interfaces); INIT_LIST_HEAD(&local->mon_list); + INIT_LIST_HEAD(&local->radar_info_list); __hw_addr_init(&local->mc_list); @@ -1638,6 +1639,7 @@ EXPORT_SYMBOL(ieee80211_register_hw); void ieee80211_unregister_hw(struct ieee80211_hw *hw) { struct ieee80211_local *local = hw_to_local(hw); + struct radar_info *radar_info, *temp; tasklet_kill(&local->tx_pending_tasklet); tasklet_kill(&local->tasklet); @@ -1673,6 +1675,12 @@ void ieee80211_unregister_hw(struct ieee80211_hw *hw) ieee80211_clear_tx_pending(local); rate_control_deinitialize(local); + list_for_each_entry_safe(radar_info, temp, &local->radar_info_list, + list) { + list_del(&radar_info->list); + kfree(radar_info); + } + if (skb_queue_len(&local->skb_queue) || skb_queue_len(&local->skb_queue_unreliable)) wiphy_warn(local->hw.wiphy, "skb_queue not empty\n"); diff --git a/net/mac80211/util.c b/net/mac80211/util.c index 3d34e07f441b..a39845465d2b 100644 --- a/net/mac80211/util.c +++ b/net/mac80211/util.c @@ -3488,12 +3488,12 @@ void ieee80211_dfs_cac_cancel(struct ieee80211_local *local) } } -void ieee80211_dfs_radar_detected_work(struct wiphy *wiphy, - struct wiphy_work *work) +static void +ieee80211_dfs_handle_radar_detection(struct ieee80211_local *local, + struct ieee80211_channel *radar_channel) { - struct ieee80211_local *local = - container_of(work, struct ieee80211_local, radar_detected_work); struct cfg80211_chan_def chandef = local->hw.conf.chandef; + struct cfg80211_chan_def *radar_chandef = NULL; struct ieee80211_chanctx *ctx; int num_chanctx = 0; @@ -3505,23 +3505,69 @@ void ieee80211_dfs_radar_detected_work(struct wiphy *wiphy, num_chanctx++; chandef = ctx->conf.def; + + if (radar_channel && + (chandef.chan == radar_channel)) + radar_chandef = &ctx->conf.def; } ieee80211_dfs_cac_cancel(local); - if (num_chanctx > 1) - /* XXX: multi-channel is not supported yet */ - WARN_ON(1); - else + if (num_chanctx > 1) { + if (local->hw.wiphy->flags & WIPHY_FLAG_SUPPORTS_MLO) { + if (WARN_ON(!radar_chandef)) + return; + + cfg80211_radar_event(local->hw.wiphy, radar_chandef, GFP_KERNEL); + } else { + /* XXX: multi-channel is not supported yet */ + WARN_ON(1); + } + } else { cfg80211_radar_event(local->hw.wiphy, &chandef, GFP_KERNEL); + } } -void ieee80211_radar_detected(struct ieee80211_hw *hw) +void ieee80211_dfs_radar_detected_work(struct wiphy *wiphy, + struct wiphy_work *work) +{ + struct ieee80211_local *local = + container_of(work, struct ieee80211_local, radar_detected_work); + struct radar_info *radar_info, *temp; + struct ieee80211_channel *radar_channel; + + lockdep_assert_wiphy(local->hw.wiphy); + + if (list_empty(&local->radar_info_list)) { + ieee80211_dfs_handle_radar_detection(local, NULL); + return; + } + + list_for_each_entry_safe(radar_info, temp, &local->radar_info_list, + list) { + radar_channel = radar_info->channel; + ieee80211_dfs_handle_radar_detection(local, radar_channel); + list_del(&radar_info->list); + kfree(radar_info); + } +} + +void ieee80211_radar_detected(struct ieee80211_hw *hw, + struct ieee80211_channel *radar_channel) { struct ieee80211_local *local = hw_to_local(hw); + struct radar_info *radar_info; trace_api_radar_detected(local); + radar_info = kzalloc(sizeof(*radar_info), GFP_ATOMIC); + if (!radar_info) + return; + + INIT_LIST_HEAD(&radar_info->list); + radar_info->channel = radar_channel; + list_add_tail(&radar_info->list, &local->radar_info_list); + wiphy_work_queue(hw->wiphy, &local->radar_detected_work); } EXPORT_SYMBOL(ieee80211_radar_detected);
Currently DFS works under assumption there could be only one channel context in the hardware. Hence, drivers just calls the function ieee80211_radar_detected() passing the hardware structure. However, with MLO, this obviously will not work since number of channel contexts will be more than one and hence drivers would need to pass the channel information as well on which the radar is detected. Hence, in order to support DFS with MLO, do the following changes - * Add channel pointer as an argument to the function ieee80211_radar_detected(). During MLO, drivers would have to pass on which channel radar is detected. * In order to pass on this channel information to the radar detected worker later on, introduce a linked list 'radar_info' in the structure local. * When driver calls radar detected, a node is created and added to this list and work is scheduled. The work handler takes care to process each node and take further action. * This would also help in scenarios where there is split phy 5 GHz radio, which is capable of DFS channels in both lower and upper band. In this case, simultaneous radars can be detected. Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com> --- drivers/net/wireless/ath/ath10k/debug.c | 4 +- drivers/net/wireless/ath/ath10k/mac.c | 2 +- drivers/net/wireless/ath/ath10k/wmi.c | 2 +- drivers/net/wireless/ath/ath11k/wmi.c | 2 +- drivers/net/wireless/ath/ath12k/wmi.c | 2 +- drivers/net/wireless/ath/ath9k/dfs.c | 2 +- drivers/net/wireless/ath/ath9k/dfs_debug.c | 2 +- .../net/wireless/mediatek/mt76/mt7615/mcu.c | 2 +- .../net/wireless/mediatek/mt76/mt76x02_dfs.c | 4 +- .../net/wireless/mediatek/mt76/mt7915/mcu.c | 2 +- .../net/wireless/mediatek/mt76/mt7996/mcu.c | 2 +- drivers/net/wireless/ti/wl18xx/event.c | 2 +- drivers/net/wireless/virtual/mac80211_hwsim.c | 2 +- include/net/mac80211.h | 5 +- net/mac80211/ieee80211_i.h | 6 ++ net/mac80211/main.c | 8 +++ net/mac80211/util.c | 64 ++++++++++++++++--- 17 files changed, 88 insertions(+), 25 deletions(-)