On 3/18/24 16:23, Benjamin Berg wrote:
...
diff --git a/src/ap/beacon.c b/src/ap/beacon.c
index e50f0a0c976e..9c028b5b72d6 100644
--- a/src/ap/beacon.c
+++ b/src/ap/beacon.c
@@ -960,7 +960,7 @@ static void
hostapd_fill_probe_resp_ml_params(struct hostapd_data *hapd,
* We want to include the AP MLD ID in the response if it
was
* included in the request.
*/
- probed_mld_id = mld_id != -1 ? mld_id : hapd->conf->mld_id;
+ probed_mld_id = mld_id != -1 ? mld_id :
hostapd_get_mld_id(hapd);
We are not (yet) hitting these code paths. But, I think conceptually it
would make more sense to get the probed hapd from the MLD ID. i.e.
something like:
probed_hapd = mld_id == 0 ? hapd : hostapd_get_mbssid_hapd(hapd, mld_id);
Yes correct. But fixes regarding MLO+MBSSID is in later part of our
work. As you rightly said, this we are not hitting as of today. Also,
this patch series tries to just support 1 MLD interface (as it was doing
already before this) and at the same time just scale up certain
structures/handling so that it can be leveraged when support for
multiple MLD interfaces (co-hosted) is added later (this I'm working on
right now and will probably send out series soon :)). So at that time
this would be fixed properly.
for_each_mld_link(link, i, j, hapd->iface->interfaces,
probed_mld_id) {
Then we need to change the iteration, but I think that makes sense
anyway, see below.
... \
@@ -794,7 +798,7 @@ int hostapd_link_remove(struct hostapd_data
*hapd, u32 count);
for (_link
= \
(_ifaces)->iface[_iface_idx]-
bss[_bss_idx]; \
_link && _link->conf->mld_ap
&& \
- _link->conf->mld_id ==
_mld_id; \
+ hostapd_get_mld_id(_link) ==
_mld_id; \
_link = NULL)
I think this part is slightly wrong, as the mld_id here would be local
to the link, but it should to be local to the hapd from which we
started the iteration.
Maybe we can update the macro and simplify it a bit at the same time.
We always have an hapd instance anyway, so passing the interfaces
explicitly does not really make sense. So, we can just start with an
hapd instance, and iterate all links (including itself). Something
like:
#define for_each_mld_link(_hapd, _link, _bss_idx, _iface_idx) \
for (_iface_idx = 0; \
_iface_idx < (_hapd)->iface->interfaces)->count; \
_iface_idx++) \
for (_bss_idx = 0; \
_bss_idx < \
(_ifaces)->iface[_iface_idx]->num_bss; \
_bss_idx++) \
for (_link = \
(_hapd)->iface->interfaces-> \
iface[_iface_idx]->bss[_bss_idx]; \
_link && \
hostapd_is_ml_partner(_link, _hapd); \
_link = NULL)
Benjamin
Yeah that makes sense. Thanks for the input! But as I said above, in the
next series, actually this logic will be replaced with another one.
Hence did not change much here.
Now (after this series) since we have all affiliated links tied up to a
common MLD structure via a list member, there is no need of these many
arguments for this macro. We can just use dl_list_for_each() and iterate
over all affiliated links.
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
