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?
> > > + 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.
> > > + 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?
johannes
