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
