On Fri, 2023-01-20 at 03:43 +0530, Sriram R wrote:
> Currently we check the status of bss active flag to see if the
> AP is active.
>
Following so far :)
> But in case of a MLD AP, when some of the links
> are getting teardown
"getting torn down"?
> and some are active, mgmt Tx(like deauth)
> can be sent on some links before they are brought down as well.
Makes sense.
> In such cases, the bss active flag might not provide the exact
> status of the MLD links, which becomes false on first link deleted.
Wait, isn't that already a bug?
> Hence check if any of the links can be used for mgmt tx
> before returning error status.
>
> Also, use the link id passed from userspace when the link bss
> address matches the mgmt SA and the chan params match the request.
> This will avoid scenario where the link id from userspace
> gets reset.
"gets reset"??
>
> +static bool ieee80211_is_link_bss_active(struct ieee80211_sub_if_data *sdata,
> + int link_id)
> +{
[...]
> + sdata_lock(sdata);
> + link = sdata_dereference(sdata->link[link_id], sdata);
> + if (!link) {
> + sdata_unlock(sdata);
> + return false;
> + }
> +
> + if (sdata_dereference(link->u.ap.beacon, sdata)) {
> + sdata_unlock(sdata);
> + return true;
> + }
> +
> + sdata_unlock(sdata);
The locking here is ... decidedly odd. It feels like with all the
wdev_lock()ing going on in cfg80211_mlme_mgmt_tx() we should probably
just lock around the *entire* thing in cfg80211, including the
driver/mac80211 call?
> @@ -883,8 +920,17 @@ int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
> break;
> }
>
> - if (ether_addr_equal(conf->addr, mgmt->sa))
> + if (ether_addr_equal(conf->addr, mgmt->sa)) {
> + /* If userspace requested Tx on a specific link
> + * use the same link id if the link bss is matching
> + * the requested chan.
> + */
> + if (sdata->vif.valid_links &&
> + params->link_id >= 0 && params->link_id == i &&
> + params->chan == chanctx_conf->def.chan)
> + link_id = i;
> break;
> + }
Not sure I get it, if it's bad (link ID doesn't match BSS) then
shouldn't we just reject it?
johannes
