On Mon, 2021-04-26 at 12:05 -0700, Aloka Dixit wrote:
>
> + * @IEEE80211_HW_SUPPORTS_MBSSID_AP: Hardware supports multiple BSSID
> + * advertisements in AP mode.
> + * @IEEE80211_HW_SUPPORTS_EMA_AP: Hardware supports enhanced multiple BSSID
> + * advertisements in AP mode.
Is there a good reason for these - the driver can just set the extended
nl80211 flags instead and we can check, or nl80211 already checked?
>
> +static int ieee80211_set_mbssid_options(struct ieee80211_sub_if_data *sdata,
> + struct cfg80211_mbssid_config params)
> +{
> + struct ieee80211_local *local = sdata->local;
> + struct wiphy *wiphy = local->hw.wiphy;
> + struct net_device *parent;
> + struct ieee80211_sub_if_data *psdata;
> +
> + if (!params.count || sdata->vif.type != NL80211_IFTYPE_AP ||
> + !ieee80211_hw_check(&local->hw, SUPPORTS_MBSSID_AP))
> + return -EINVAL;
> +
> + sdata->vif.mbssid.parent = NULL;
> + sdata->vif.mbssid.flags = IEEE80211_VIF_MBSSID_TX;
> + if (params.parent) {
> + parent = __dev_get_by_index(wiphy_net(wiphy), params.parent);
> + if (!parent || !parent->ieee80211_ptr)
> + return -EINVAL;
Hm wait, didn't I see the same code in cfg80211 - maybe cfg80211 should
just pass the pointer?
>
> static int ieee80211_del_iface(struct wiphy *wiphy, struct wireless_dev *wdev)
> {
> + struct ieee80211_sub_if_data *sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
> + struct ieee80211_local *local;
> + struct ieee80211_vif *vif;
> +
> + if (!sdata)
> + return 0;
> +
> + local = sdata->local;
> + vif = &sdata->vif;
> + if (vif->type == NL80211_IFTYPE_AP &&
> + ieee80211_hw_check(&local->hw, SUPPORTS_MBSSID_AP)) {
> + if (vif->mbssid.flags & IEEE80211_VIF_MBSSID_TX) {
> + struct ieee80211_sub_if_data *child, *tmpsdata;
> +
> + wiphy_unlock(local->hw.wiphy);
> + mutex_lock(&local->iflist_mtx);
I really don't think you can drop the locking like that in the middle of
something. That's almost always a recipe for disaster.
> @@ -375,6 +375,18 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata, bool going_do
> struct cfg80211_chan_def chandef;
> bool cancel_scan;
> struct cfg80211_nan_func *func;
> + struct ieee80211_sub_if_data *parent;
> +
> + if (sdata->vif.type == NL80211_IFTYPE_AP &&
> + ieee80211_hw_check(&local->hw, SUPPORTS_MBSSID_AP) &&
> + sdata->vif.mbssid.flags & IEEE80211_VIF_MBSSID_NON_TX) {
> + parent = vif_to_sdata(sdata->vif.mbssid.parent);
> + if (parent && ieee80211_sdata_running(parent)) {
> + wiphy_unlock(local->hw.wiphy);
> + dev_close(parent->wdev.netdev);
> + wiphy_lock(local->hw.wiphy);
Same here.
johannes
