diff mbox series

[V2,03/10] mac80211: add multiple bssid support

Message ID 20200706115219.663650-3-john@phrozen.org (mailing list archive)
State Changes Requested
Delegated to: Johannes Berg
Headers show
Series [V2,01/10] nl80211: add basic multiple bssid support | expand

Commit Message

John Crispin July 6, 2020, 11:52 a.m. UTC
When bringing up multi bssid APs we need to track the parent-child relation
of non-transmitting and transmitting VAPs.

The patch warns when
* a parent is deleted while it has children
* a parent is opened before all children are opened
* a child is closed when its parent is open

This patch checks the above by using a linked list to track the relations.

Signed-off-by: John Crispin <john@phrozen.org>
---
 include/net/mac80211.h | 20 +++++++++++++
 net/mac80211/cfg.c     | 68 ++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/iface.c   | 19 ++++++++++++
 net/mac80211/util.c    | 27 +++++++++++++++++
 4 files changed, 134 insertions(+)

Comments

Johannes Berg July 30, 2020, 1:03 p.m. UTC | #1
On Mon, 2020-07-06 at 13:52 +0200, John Crispin wrote:
> 
> +/**
> + * ieee80211_get_multi_bssid_mode - get a vifs multi bssid mode.
> + *
> + * This function is used to help look up the multi bssid mode which is tracked
> + * inside the wdev.
> + *
> + * @vif: &struct ieee80211_vif pointer from the add_interface callback.
> + */
> +enum nl80211_multi_bssid_mode ieee80211_get_multi_bssid_mode(struct ieee80211_vif *vif);
> +
> +/**
> + * ieee80211_get_multi_bssid_parent - get a vifs multi bssid parent.
> + *
> + * This function is used to help look up the multi bssid parent which is tracked
> + * inside the wdev.
> + *
> + * @vif: &struct ieee80211_vif pointer from the add_interface callback.
> + */
> +struct ieee80211_vif *ieee80211_get_multi_bssid_parent(struct ieee80211_vif *vif);

All this can be a lot simpler if you don't just push the data that I
just mentioned from the wdev down to the sdata, but actually down to the
vif. Then these are just something like

	vif->multi_bssid.parent

without a need to call a function. That'd probably result in
significantly smaller code too, since exporting a function takes quite a
bit of space.

(Also, if you insist that it must be in the wdev, you can use the
function that obtains the wdev from the vif, and dereference that -
still wouldn't require exporting a lot of new functions.)

> +	if (params->multi_bssid_mode &&
> +	    !ieee80211_hw_check(&local->hw, SUPPORTS_MULTI_BSSID))
> +		return -ENOTSUPP;

IMHO that needs to be a new, separate feature bit, probably even at
nl80211 level. This here was more of a client-side thing, and now you're
doing AP side. I don't think we can mix those (and iwlwifi surely would
have issues with that.)

>  static int ieee80211_del_iface(struct wiphy *wiphy, struct wireless_dev *wdev)
>  {
> +	struct ieee80211_sub_if_data *sdata;
> +	struct wireless_dev *child, *tmp;
> +
> +	sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
> +	switch (sdata->wdev.multi_bssid_mode) {
> +	case NL80211_MULTIPLE_BSSID_TRANSMITTED:
> +		if (list_empty(&sdata->wdev.multi_bssid_list))
> +			break;
> +		sdata_info(sdata, "deleting while non-transmitting children still exist\n");

Is that even worth a message? I mean, you could just destroy the
children too, and document it in the API that way?

> +		list_for_each_entry_safe(child, tmp, &sdata->wdev.multi_bssid_list,
> +					 multi_bssid_list) {
> +			list_del(&child->multi_bssid_list);
> +			child->multi_bssid_parent = NULL;
> +		}

It also seems you shouldn't just NULL out the pointer but dev_close()
them so they stop operating?

>  	case NL80211_IFTYPE_AP:
>  		sdata->bss = &sdata->u.ap;
> +		if (wdev->multi_bssid_mode == NL80211_MULTIPLE_BSSID_TRANSMITTED) {
> +			struct wireless_dev *child;
> +			int children_down = 0;
> +
> +			/* check if all children are already up */
> +			list_for_each_entry(child, &wdev->multi_bssid_list,
> +					    multi_bssid_list)
> +				if (!wdev_running(child))
> +					children_down = 1;
> +			if (children_down)
> +				sdata_info(sdata, "non-transmitting children are not up yet\n");

reject it?

> @@ -800,6 +812,7 @@ static int ieee80211_open(struct net_device *dev)
>  static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
>  			      bool going_down)
>  {
> +	struct wireless_dev *wdev = ieee80211_vif_to_wdev(&sdata->vif);
>  	struct ieee80211_local *local = sdata->local;
>  	unsigned long flags;
>  	struct sk_buff *skb, *tmp;
> @@ -810,6 +823,12 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
>  	bool cancel_scan;
>  	struct cfg80211_nan_func *func;
>  
> +	if (sdata->vif.type == NL80211_IFTYPE_AP &&
> +	    wdev->multi_bssid_mode == NL80211_MULTIPLE_BSSID_NON_TRANSMITTED)
> +		/* make sure the parent is already down */
> +		if (wdev->multi_bssid_parent && wdev_running(wdev->multi_bssid_parent))
> +			sdata_info(sdata, "transmitting parent is still up\n");

Reject it? Or dev_close() the parent?

johannes
diff mbox series

Patch

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 11d5610d2ad5..3617a2742c4d 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -6558,4 +6558,24 @@  u32 ieee80211_calc_tx_airtime(struct ieee80211_hw *hw,
  */
 bool ieee80211_set_hw_80211_encap(struct ieee80211_vif *vif, bool enable);
 
+/**
+ * ieee80211_get_multi_bssid_mode - get a vifs multi bssid mode.
+ *
+ * This function is used to help look up the multi bssid mode which is tracked
+ * inside the wdev.
+ *
+ * @vif: &struct ieee80211_vif pointer from the add_interface callback.
+ */
+enum nl80211_multi_bssid_mode ieee80211_get_multi_bssid_mode(struct ieee80211_vif *vif);
+
+/**
+ * ieee80211_get_multi_bssid_parent - get a vifs multi bssid parent.
+ *
+ * This function is used to help look up the multi bssid parent which is tracked
+ * inside the wdev.
+ *
+ * @vif: &struct ieee80211_vif pointer from the add_interface callback.
+ */
+struct ieee80211_vif *ieee80211_get_multi_bssid_parent(struct ieee80211_vif *vif);
+
 #endif /* MAC80211_H */
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 9b360544ad6f..d315120799c0 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -111,6 +111,42 @@  static int ieee80211_set_mon_options(struct ieee80211_sub_if_data *sdata,
 	return 0;
 }
 
+static int ieee80211_set_multi_bssid_options(struct ieee80211_sub_if_data *sdata,
+					     struct vif_params *params)
+{
+	struct ieee80211_local *local = sdata->local;
+	struct wiphy *wiphy = local->hw.wiphy;
+	struct net_device *parent;
+
+	if (params->multi_bssid_mode &&
+	    !ieee80211_hw_check(&local->hw, SUPPORTS_MULTI_BSSID))
+		return -ENOTSUPP;
+
+	switch (params->multi_bssid_mode) {
+	case NL80211_MULTIPLE_BSSID_NON_TRANSMITTED:
+		parent = __dev_get_by_index(wiphy_net(wiphy),
+					    params->multi_bssid_parent);
+		if (!parent || !parent->ieee80211_ptr ||
+		    parent->ieee80211_ptr->multi_bssid_mode !=
+					NL80211_MULTIPLE_BSSID_TRANSMITTED)
+			return -EINVAL;
+		sdata->wdev.multi_bssid_parent = parent->ieee80211_ptr;
+		list_add(&sdata->wdev.multi_bssid_list,
+			 &parent->ieee80211_ptr->multi_bssid_list);
+		break;
+
+	case NL80211_MULTIPLE_BSSID_TRANSMITTED:
+		INIT_LIST_HEAD(&sdata->wdev.multi_bssid_list);
+		break;
+
+	default:
+		break;
+	}
+	sdata->wdev.multi_bssid_mode = params->multi_bssid_mode;
+
+	return 0;
+}
+
 static struct wireless_dev *ieee80211_add_iface(struct wiphy *wiphy,
 						const char *name,
 						unsigned char name_assign_type,
@@ -136,11 +172,43 @@  static struct wireless_dev *ieee80211_add_iface(struct wiphy *wiphy,
 		}
 	}
 
+	if (type == NL80211_IFTYPE_AP) {
+		err = ieee80211_set_multi_bssid_options(sdata, params);
+		if (err) {
+			ieee80211_if_remove(sdata);
+			return NULL;
+		}
+	}
+
 	return wdev;
 }
 
 static int ieee80211_del_iface(struct wiphy *wiphy, struct wireless_dev *wdev)
 {
+	struct ieee80211_sub_if_data *sdata;
+	struct wireless_dev *child, *tmp;
+
+	sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
+	switch (sdata->wdev.multi_bssid_mode) {
+	case NL80211_MULTIPLE_BSSID_TRANSMITTED:
+		if (list_empty(&sdata->wdev.multi_bssid_list))
+			break;
+		sdata_info(sdata, "deleting while non-transmitting children still exist\n");
+		list_for_each_entry_safe(child, tmp, &sdata->wdev.multi_bssid_list,
+					 multi_bssid_list) {
+			list_del(&child->multi_bssid_list);
+			child->multi_bssid_parent = NULL;
+		}
+		break;
+
+	case NL80211_MULTIPLE_BSSID_NON_TRANSMITTED:
+		list_del(&sdata->wdev.multi_bssid_list);
+		break;
+
+	default:
+		break;
+	}
+
 	ieee80211_if_remove(IEEE80211_WDEV_TO_SUB_IF(wdev));
 
 	return 0;
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index f900c84fb40f..777b530e38d0 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -535,6 +535,18 @@  int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
 		}
 	case NL80211_IFTYPE_AP:
 		sdata->bss = &sdata->u.ap;
+		if (wdev->multi_bssid_mode == NL80211_MULTIPLE_BSSID_TRANSMITTED) {
+			struct wireless_dev *child;
+			int children_down = 0;
+
+			/* check if all children are already up */
+			list_for_each_entry(child, &wdev->multi_bssid_list,
+					    multi_bssid_list)
+				if (!wdev_running(child))
+					children_down = 1;
+			if (children_down)
+				sdata_info(sdata, "non-transmitting children are not up yet\n");
+		}
 		break;
 	case NL80211_IFTYPE_MESH_POINT:
 	case NL80211_IFTYPE_STATION:
@@ -800,6 +812,7 @@  static int ieee80211_open(struct net_device *dev)
 static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 			      bool going_down)
 {
+	struct wireless_dev *wdev = ieee80211_vif_to_wdev(&sdata->vif);
 	struct ieee80211_local *local = sdata->local;
 	unsigned long flags;
 	struct sk_buff *skb, *tmp;
@@ -810,6 +823,12 @@  static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 	bool cancel_scan;
 	struct cfg80211_nan_func *func;
 
+	if (sdata->vif.type == NL80211_IFTYPE_AP &&
+	    wdev->multi_bssid_mode == NL80211_MULTIPLE_BSSID_NON_TRANSMITTED)
+		/* make sure the parent is already down */
+		if (wdev->multi_bssid_parent && wdev_running(wdev->multi_bssid_parent))
+			sdata_info(sdata, "transmitting parent is still up\n");
+
 	clear_bit(SDATA_STATE_RUNNING, &sdata->state);
 
 	cancel_scan = rcu_access_pointer(local->scan_sdata) == sdata;
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 21c94094a699..3e4308c8f690 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -847,6 +847,33 @@  struct wireless_dev *ieee80211_vif_to_wdev(struct ieee80211_vif *vif)
 }
 EXPORT_SYMBOL_GPL(ieee80211_vif_to_wdev);
 
+enum nl80211_multi_bssid_mode ieee80211_get_multi_bssid_mode(struct ieee80211_vif *vif)
+{
+	struct ieee80211_sub_if_data *sdata;
+
+	if (!vif)
+		return 0;
+
+	sdata = vif_to_sdata(vif);
+
+	return sdata->wdev.multi_bssid_mode;
+}
+EXPORT_SYMBOL_GPL(ieee80211_get_multi_bssid_mode);
+
+struct ieee80211_vif *ieee80211_get_multi_bssid_parent(struct ieee80211_vif *vif)
+{
+	struct ieee80211_sub_if_data *sdata;
+
+	if (!vif)
+		return NULL;
+
+	sdata = vif_to_sdata(vif);
+	if (sdata->wdev.multi_bssid_parent)
+		return wdev_to_ieee80211_vif(sdata->wdev.multi_bssid_parent);
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(ieee80211_get_multi_bssid_parent);
+
 /*
  * Nothing should have been stuffed into the workqueue during
  * the suspend->resume cycle. Since we can't check each caller