On 8/28/24 17:34, Johannes Berg wrote:
On Wed, 2024-08-28 at 17:17 +0530, Aditya Kumar Singh wrote:
On 8/28/24 16:03, Johannes Berg wrote:
On Thu, 2024-07-11 at 09:21 +0530, Aditya Kumar Singh wrote:
* In the worker, go over all the contexts again and for all such context
which is marked with radar detected, add it to a local linked list.
Why is the local list needed first?
As per the current design and hwsim test cases,
NL80211_RADAR_CAC_ABORTED event is expected first and then
NL80211_RADAR_DETECTED event.
Ouch, OK.
Now, in the worker, while iterating over
the contexts and upon finding radar marked contexts,
cfg80211_radar_event() can not be called immediately. First
ieee80211_dfs_cac_cancel() needs to be called which in turn will send
CAC_ABORTED event. Now calling this will delete the channel context,
hence a local copy is taken and since we could have multiple contexts
marked with radar, a linked list is used.
Why not do both in ieee80211_dfs_cac_cancel() then? You're effectively
iterating all the chanctx's there anyway, so you could just add
if (ctx->radar_reported) {
ctx->radar_reported = false;
tmp_chandef = ctx->conf.chandef;
report = true;
}
...
cac_event()
if (report)
cfg80211_radar_event(...)
or so in the loop there?
Okay sure. Let me see what I can do here.
+ radar_info = kzalloc(sizeof(*radar_info), GFP_KERNEL);
+ if (WARN_ON(!radar_info))
+ continue;
that clearly shouldn't be a WARN_ON,
If node can not be added, NL event will not be sent to user sapce and
apt action can not be taken. Will that be fine missing out it without
any warning?
Have you actually _seen_ what happens if the kernel runs out of memory?
Not that it'll actually happen here.
:) Okay will remove WARN_ON().
+ if (num_chanctx > 1) {
+ /* XXX: multi-channel is not supported yet in case of non-MLO */
+ if (WARN_ON(!(wiphy->flags & WIPHY_FLAG_SUPPORTS_MLO)))
+ trigger_event = false;
+ }
I don't see how that'd happen in the first place?
This part is keeping the same old code with the new handling with MLO.
Prior to MLO, if num_chanctx is more than 1, it would throw a warn_on so
that's why it will throw now as well if MLO is not supported and
simultaneously two channel contexts are marked as radar detected.
I don't think it can actually happen, and if keeping the warning around
makes the code this complex I'm not sure I'd want to?
Sure will remove that condition check. Thanks for the suggestions.
johannes
--
Aditya
