Hi Sergey,
>Hi Orr,
>
>Thanks for detailed use-case clarification.
Sure thing.
Thanks for reviewing my patch.
>
>> >> In case a radar event of CAC_FINISHED or RADAR_DETECTED happens
>during
>> >> another phy is during CAC we might need to cancel that CAC.
>> >> If we got a radar in a channel that another phy is now doing CAC on
>> >> then the CAC should be canceled.
>> >> If, for example, 2 phys doing CAC on the same channels, or on
>> >> comptable channels, once on of them will finish his CAC the other
>> >> might need to cancel his CAC, since it is no longer relevant.
>> >>
>> >> To fix that the commit adds an callback and implement it in mac80211
>> >> to end CAC.
>> >> This commit also adds a call to said callback if after a radar event
>> >> we see the cac is no longer relevant
>> >
>> >> net/mac80211/cfg.c | 23 +++++++++++++++++++++++
>> >> net/wireless/rdev-ops.h | 10 ++++++++++
>> >> net/wireless/reg.c | 24 +++++++++++++++++++++++-
>> >> net/wireless/trace.h | 5 +++++
>> >> 5 files changed, 66 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index
>> >> 4ab2c49423dc..68782ba8b6e8 100644
>> >> --- a/include/net/cfg80211.h
>> >> +++ b/include/net/cfg80211.h
>> >> @@ -3537,6 +3537,9 @@ struct cfg80211_update_owe_info {
>> >> *
>> >> * @start_radar_detection: Start radar detection in the driver.
>> >> *
>> >> + * @end_cac: End running CAC, probably because a related CAC
>> >> + * was finished on another phy.
>> >> + *
>> >
>> >Maybe it makes sense to follow existing naming convention here and to
>use
>> >something like 'stop_radar_detection' ?
>>
>> I think 'stop_radar_detection' might be misleading as we don’t stop
>radar_detection,
>> we only end cac, normal radar detection will continue.
>
>Ok, good point.
>
>
>> >> * @update_ft_ies: Provide updated Fast BSS Transition information to
>the
>> >> * driver. If the SME is in the driver/firmware, this information can be
>> >> * used in building Authentication and Reassociation Request frames.
>> >> @@ -3863,6 +3866,8 @@ struct cfg80211_ops {
>> >> struct net_device *dev,
>> >> struct cfg80211_chan_def *chandef,
>> >> u32 cac_time_ms);
>> >> + void (*end_cac)(struct wiphy *wiphy,
>> >> + struct net_device *dev);
>> >
>> >...
>> >
>> >> +static void cfg80211_check_and_end_cac(struct
>> >> +cfg80211_registered_device *rdev) {
>> >> + struct wireless_dev *wdev;
>> >> + /* If we finished CAC or received radar, we should end any
>> >> + * CAC running on the same channels.
>> >> + * the check !cfg80211_chandef_dfs_usable contain 2 options:
>> >> + * either all channels are available - those the CAC_FINISHED
>> >> + * event has effected another wdev state, or there is a channel
>> >> + * in unavailable state in wdev chandef - those the RADAR_DETECTED
>> >> + * event has effected another wdev state.
>> >> + * In both cases we should end the CAC on the wdev.
>> >> + *
>> >> + */
>> >> + list_for_each_entry(wdev, &rdev->wiphy.wdev_list, list) {
>> >> + if (wdev->cac_started &&
>> >> + !cfg80211_chandef_dfs_usable(&rdev->wiphy, &wdev-
>> >>chandef))
>> >> + rdev_end_cac(rdev, wdev->netdev);
>> >> + }
>> >> +}
>> >> +
>> >
>> >IIUC, this code does not match your commit message. You are stopping
>CAC
>> >on all the virtual wireless interfaces on the same PHY, but not CACs on
>> >different PHYs. Meanwhile CAC does not need to be started on multiple
>> >virtual interfaces. For instance, in multiple BSSID configuration, hostapd
>> >performs CAC only on primary interface.
>> >
>>
>> regulatory_propagate_dfs_state will call cfg80211_check_and_end_cac
>> only on phys != current phy.
>> So for each phy != current we will call mac80211 end_cac (if needed)
>> which in turn will end the cac on all that phys’ interfaces.
>
>Ok, so regulatory_propagate_dfs_state iterates over other PHYs from the
>same regulatory region updating state of DFS channel reported by the
>original PHY. Unless I am missing something, in this case there are two
>possible ways to proceed with CAC running on other PHYs:
>
>- to stop CAC on other PHYs with the same channel/bandwidth in both
> RADAR_DETECTION and CAC_FINISHED cases
>- to stop CAC on other PHYs if their channel has just become unusable
> due to RADAR_DETECTION event reported by the original PHY
>
>So it looks like you have chosen a more conservative second option.
>But then I don't quite understand your comment for the new function
>cfg80211_check_and_end_cac: why do you say that CAC_FINISHED case
>is also covered here ?
>
CAC_FINISHED is covered here:
If there is an unavailable channel cfg80211_chandef_dfs_usable will return false - this covers RADAR_DETECTED.
If all channels are available cfg80211_chandef_dfs_usable will also return false,
as they are not usable, they are available - this covers CAC_FINISHED.
Basically cfg80211_chandef_dfs_usable checks if we need to do CAC,
it is also used in nl80211_start_radar_detection in order to make sure we actually need CAC.
Regards,
Orr
