[RFC] initial channel context implementation

[RFC] initial channel context implementation

[Michal Kazior]

Hi Johannes,

I've done some work on channel contexts. It's based on your recently
posted patches (set_channel, join_mesh, etc). This is course for
multi-channel operation.

This isn't ready/complete yet. Channel selection is unsafe because the
for() shouldn't be limited to the IEEE80211_MAX_CHANNEL_CONTEXTS. It
should be limited by the maximum number of different channels for a
matching interface combination during runtime.

This however seems to be a bit tricky right now. I've looked at your
cfg80211-enforce-beacon_int_infra_match.patch but the
cfg80211_find_current_combination ignores the fact that an interface
may be up and running without a channel contexts being bound (i.e.
without an association whatsoever). For this to work we need to know
whether a given interface is really running/operational for tx.

I've thought about using netif_carrier_on/off for this purpose but
I'm not sure whether we can use it for our purpose. Other idea is to
create a new function that would be able to tell us whether an
interface is using a channel. But what would that be based on? Can we
check that easily for all interface types (monitor)?

My current idea is to make channel contexts work alongside what we
have today. Old drivers would remain to use current structures
(oper_channel, hw.conf.channel, etc), new ones or the ones wanting to
support multi-channel would switch to channel contexts.


--
Pozdrawiam / Best regards,
Michal Kazior.

[RFC 1/6] mac80211: introduce channel contexts skeleton code

[Michal Kazior]

Channel context are the foundation for
multi-channel operation.

Channel contexts are immutable and are re-created
(or re-used if other interfaces are bound to a
certain channel) on channel switching.

This is a initial implementation and more features
will come in separate patches for easier review.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 include/net/mac80211.h     |   21 +++++++++
 net/mac80211/chan.c        |  108 ++++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/ieee80211_i.h |   11 +++++
 net/mac80211/main.c        |    3 +
 4 files changed, 143 insertions(+), 0 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 1937c7d..75a613f 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -144,6 +144,22 @@ struct ieee80211_low_level_stats {
 };
 
 /**
+ * struct ieee80211_channel_context - channel context that vifs may be tuned to
+ *
+ * @channel: the channel to tune to
+ * @channel_type: the channel (HT) type
+ * @vif_list: vifs bound to channel context
+ */
+struct ieee80211_channel_context {
+	struct ieee80211_channel *channel;
+	enum nl80211_channel_type channel_type;
+
+	struct list_head vif_list;
+};
+
+#define IEEE80211_MAX_CHANNEL_CONTEXTS 8
+
+/**
  * enum ieee80211_bss_change - BSS change notification flags
  *
  * These flags are used with the bss_info_changed() callback
@@ -897,6 +913,8 @@ enum ieee80211_vif_flags {
  *	at runtime, mac80211 will never touch this field
  * @hw_queue: hardware queue for each AC
  * @cab_queue: content-after-beacon (DTIM beacon really) queue, AP mode only
+ * @channel_context: channel context vif is bound to, may be NULL
+ * @list: linked list for channel context's vif_list
  * @drv_priv: data area for driver use, will always be aligned to
  *	sizeof(void *).
  */
@@ -909,6 +927,9 @@ struct ieee80211_vif {
 	u8 cab_queue;
 	u8 hw_queue[IEEE80211_NUM_ACS];
 
+	struct ieee80211_channel_context *channel_context;
+	struct list_head list;
+
 	u32 driver_flags;
 
 	/* must be last */
diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index c76cf72..e9b0f3e 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -135,3 +135,111 @@ bool ieee80211_set_channel_type(struct ieee80211_local *local,
 
 	return result;
 }
+
+static struct ieee80211_channel_context *
+ieee80211_find_channel_context(struct ieee80211_local *local,
+			       struct ieee80211_channel *channel,
+			       enum nl80211_channel_type channel_type)
+{
+	struct ieee80211_channel_context *ctx;
+	int i;
+
+	if (WARN_ON(!channel))
+		return NULL;
+
+	for (i = 0; i < IEEE80211_MAX_CHANNEL_CONTEXTS; i++) {
+		ctx = &local->channel_contexts[i];
+		if (ctx->channel != channel)
+			continue;
+		if (ctx->channel_type != channel_type)
+			continue;
+
+		return ctx;
+	}
+
+	return NULL;
+}
+
+static struct ieee80211_channel_context *
+ieee80211_new_channel_context(struct ieee80211_local *local,
+			      struct ieee80211_channel *channel,
+			      enum nl80211_channel_type channel_type)
+{
+	struct ieee80211_channel_context *ctx = NULL;
+	int i;
+
+	for (i = 0; i < IEEE80211_MAX_CHANNEL_CONTEXTS; i++)
+		if (!local->channel_contexts[i].channel) {
+			ctx = &local->channel_contexts[i];
+			break;
+		}
+
+	if (WARN_ON(!ctx))
+		return NULL;
+
+	ctx->channel = channel;
+	ctx->channel_type = channel_type;
+	INIT_LIST_HEAD(&ctx->vif_list);
+
+	return ctx;
+}
+
+static void
+ieee80211_free_channel_context(struct ieee80211_local *local,
+			       struct ieee80211_channel_context *ctx)
+{
+	if (WARN_ON(!list_empty(&ctx->vif_list)))
+		return;
+
+	ctx->channel = NULL;
+}
+
+static void
+ieee80211_assign_vif_channel_context(struct ieee80211_sub_if_data *sdata,
+				     struct ieee80211_channel_context *ctx)
+{
+	list_add(&sdata->vif.list, &ctx->vif_list);
+	sdata->vif.channel_context = ctx;
+}
+
+static void
+ieee80211_unassign_vif_channel_context(struct ieee80211_sub_if_data *sdata,
+				       struct ieee80211_channel_context *ctx)
+{
+	sdata->vif.channel_context = NULL;
+	list_del(&sdata->vif.list);
+}
+
+bool
+ieee80211_vif_use_channel(struct ieee80211_sub_if_data *sdata,
+			  struct ieee80211_channel *channel,
+			  enum nl80211_channel_type channel_type)
+{
+	struct ieee80211_local *local = sdata->local;
+	struct ieee80211_channel_context *ctx;
+
+	ieee80211_vif_release_channel(sdata);
+
+	ctx = ieee80211_find_channel_context(local, channel, channel_type);
+	if (!ctx)
+		ctx = ieee80211_new_channel_context(local, channel, channel_type);
+	if (!ctx)
+		return false;
+
+	ieee80211_assign_vif_channel_context(sdata, ctx);
+	return true;
+}
+
+void
+ieee80211_vif_release_channel(struct ieee80211_sub_if_data *sdata)
+{
+	struct ieee80211_local *local = sdata->local;
+	struct ieee80211_channel_context *ctx = sdata->vif.channel_context;
+
+	if (!ctx)
+		return;
+
+	ieee80211_unassign_vif_channel_context(sdata, ctx);
+	if (list_empty(&ctx->vif_list))
+		ieee80211_free_channel_context(local, ctx);
+}
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ae046b5..d8a266e 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1000,6 +1000,10 @@ struct ieee80211_local {
 	struct ieee80211_channel *tmp_channel;
 	enum nl80211_channel_type tmp_channel_type;
 
+	/* channel contexts */
+	struct ieee80211_channel_context
+			channel_contexts[IEEE80211_MAX_CHANNEL_CONTEXTS];
+
 	/* SNMP counters */
 	/* dot11CountersTable */
 	u32 dot11TransmittedFragmentCount;
@@ -1528,6 +1532,13 @@ bool ieee80211_set_channel_type(struct ieee80211_local *local,
 enum nl80211_channel_type
 ieee80211_ht_oper_to_channel_type(struct ieee80211_ht_operation *ht_oper);
 
+bool
+ieee80211_vif_use_channel(struct ieee80211_sub_if_data *sdata,
+			  struct ieee80211_channel *channel,
+			  enum nl80211_channel_type channel_type);
+void
+ieee80211_vif_release_channel(struct ieee80211_sub_if_data *sdata);
+
 #ifdef CONFIG_MAC80211_NOINLINE
 #define debug_noinline noinline
 #else
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index f5548e9..7cbc005 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -772,6 +772,9 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 		if (c->num_different_channels > 1)
 			return -EINVAL;
 
+		if (c->num_different_channels > IEEE80211_MAX_CHANNEL_CONTEXTS)
+			return -EINVAL;
+
 		for (j = 0; j < c->n_limits; j++)
 			if ((c->limits[j].types & BIT(NL80211_IFTYPE_ADHOC)) &&
 			    c->limits[j].max > 1)
-- 
1.7.0.4

[RFC 2/6] mac80211: introduce new ieee80211_ops

[Michal Kazior]

Introduces channel context callbacks. Channel on a
context channel is immutable. Channel type will be
changable later though, thus change_channel_type is
adveristed.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 include/net/mac80211.h |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 75a613f..1e5722b 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2262,6 +2262,18 @@ enum ieee80211_rate_control_changed {
  * @get_et_strings:  Ethtool API to get a set of strings to describe stats
  *	and perhaps other supported types of ethtool data-sets.
  *
+ * @add_channel_context: Notifies device driver about new channel
+ *	context creation.
+ * @remove_channel_context: Notifies device driver about channel context
+ *	destruction.
+ * @change_channel_type: Notifies device driver about channel context
+ *	channel_type change which may happen when combining different vifs on
+ *	a same channel with different HTs.
+ * @assign_vif_channel_context: Notifies device driver about channel
+ *	context being bound to vif. Possible use is for hw queue
+ *	remapping.
+ * @unassign_vif_channel_context: Notifies device driver about channel
+ *	context being unbound from vif.
  */
 struct ieee80211_ops {
 	void (*tx)(struct ieee80211_hw *hw, struct sk_buff *skb);
@@ -2401,6 +2413,19 @@ struct ieee80211_ops {
 	void	(*get_et_strings)(struct ieee80211_hw *hw,
 				  struct ieee80211_vif *vif,
 				  u32 sset, u8 *data);
+
+	void (*add_channel_context)(struct ieee80211_hw *hw,
+				    struct ieee80211_channel_context *ctx);
+	void (*remove_channel_context)(struct ieee80211_hw *hw,
+				       struct ieee80211_channel_context *ctx);
+	void (*change_channel_type)(struct ieee80211_hw *hw,
+				    struct ieee80211_channel_context *ctx);
+	void (*assign_vif_channel_context)(struct ieee80211_hw *hw,
+					   struct ieee80211_vif *vif,
+					   struct ieee80211_channel_context *ctx);
+	void (*unassign_vif_channel_context)(struct ieee80211_hw *hw,
+					     struct ieee80211_vif *vif,
+					     struct ieee80211_channel_context *ctx);
 };
 
 /**
-- 
1.7.0.4

[RFC 3/6] mac80211: add drv_* wrappers for channel contexts

[Michal Kazior]

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/mac80211/driver-ops.h |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 6d33a0c..5bc1efc 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -845,4 +845,49 @@ drv_allow_buffered_frames(struct ieee80211_local *local,
 						  more_data);
 	trace_drv_return_void(local);
 }
+
+static inline void
+drv_add_channel_context(struct ieee80211_local *local,
+		        struct ieee80211_channel_context *ctx)
+{
+	if (local->ops->add_channel_context)
+		local->ops->add_channel_context(&local->hw, ctx);
+}
+
+static inline void
+drv_remove_channel_context(struct ieee80211_local *local,
+			   struct ieee80211_channel_context *ctx)
+{
+	if (local->ops->remove_channel_context)
+		local->ops->remove_channel_context(&local->hw, ctx);
+}
+
+static inline void
+drv_change_channel_type(struct ieee80211_local *local,
+			struct ieee80211_channel_context *ctx)
+{
+	if (local->ops->change_channel_type)
+		local->ops->change_channel_type(&local->hw, ctx);
+}
+
+static inline void
+drv_assign_vif_channel_context(struct ieee80211_local *local,
+			       struct ieee80211_sub_if_data *sdata,
+			       struct ieee80211_channel_context *ctx)
+{
+	if (local->ops->assign_vif_channel_context)
+		local->ops->assign_vif_channel_context(&local->hw,
+						       &sdata->vif, ctx);
+}
+
+static inline void
+drv_unassign_vif_channel_context(struct ieee80211_local *local,
+				 struct ieee80211_sub_if_data *sdata,
+				 struct ieee80211_channel_context *ctx)
+{
+	if (local->ops->unassign_vif_channel_context)
+		local->ops->unassign_vif_channel_context(&local->hw,
+							 &sdata->vif, ctx);
+}
+
 #endif /* __MAC80211_DRIVER_OPS */
-- 
1.7.0.4

[RFC 4/6] mac80211: use channel context notifications

[Michal Kazior]

Channel context pointer will be accessiable on
both assign and unassign events.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/mac80211/chan.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index e9b0f3e..32b7088 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -5,6 +5,7 @@
 #include <linux/nl80211.h>
 #include <net/cfg80211.h>
 #include "ieee80211_i.h"
+#include "driver-ops.h"
 
 static enum ieee80211_chan_mode
 __ieee80211_get_channel_mode(struct ieee80211_local *local,
@@ -181,6 +182,8 @@ ieee80211_new_channel_context(struct ieee80211_local *local,
 	ctx->channel_type = channel_type;
 	INIT_LIST_HEAD(&ctx->vif_list);
 
+	drv_add_channel_context(local, ctx);
+
 	return ctx;
 }
 
@@ -191,6 +194,8 @@ ieee80211_free_channel_context(struct ieee80211_local *local,
 	if (WARN_ON(!list_empty(&ctx->vif_list)))
 		return;
 
+	drv_remove_channel_context(local, ctx);
+
 	ctx->channel = NULL;
 }
 
@@ -198,14 +203,22 @@ static void
 ieee80211_assign_vif_channel_context(struct ieee80211_sub_if_data *sdata,
 				     struct ieee80211_channel_context *ctx)
 {
+	struct ieee80211_local *local = sdata->local;
+
 	list_add(&sdata->vif.list, &ctx->vif_list);
 	sdata->vif.channel_context = ctx;
+
+	drv_assign_vif_channel_context(local, sdata, ctx);
 }
 
 static void
 ieee80211_unassign_vif_channel_context(struct ieee80211_sub_if_data *sdata,
 				       struct ieee80211_channel_context *ctx)
 {
+	struct ieee80211_local *local = sdata->local;
+
+	drv_unassign_vif_channel_context(local, sdata, ctx);
+
 	sdata->vif.channel_context = NULL;
 	list_del(&sdata->vif.list);
 }
-- 
1.7.0.4

[RFC 5/6] mac80211: refactor set_channel_type

[Michal Kazior]

Split functionality for further reuse.

Will prevent code duplication when channel context
channel_type merging is introduced.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/mac80211/chan.c |   57 ++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 32b7088..030d44e 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -65,16 +65,14 @@ ieee80211_get_channel_mode(struct ieee80211_local *local,
 	return mode;
 }
 
-bool ieee80211_set_channel_type(struct ieee80211_local *local,
-				struct ieee80211_sub_if_data *sdata,
-				enum nl80211_channel_type chantype)
+static enum nl80211_channel_type
+ieee80211_get_superchan(struct ieee80211_local *local,
+			struct ieee80211_sub_if_data *sdata)
 {
-	struct ieee80211_sub_if_data *tmp;
 	enum nl80211_channel_type superchan = NL80211_CHAN_NO_HT;
-	bool result;
+	struct ieee80211_sub_if_data *tmp;
 
 	mutex_lock(&local->iflist_mtx);
-
 	list_for_each_entry(tmp, &local->interfaces, list) {
 		if (tmp == sdata)
 			continue;
@@ -100,41 +98,62 @@ bool ieee80211_set_channel_type(struct ieee80211_local *local,
 			break;
 		}
 	}
+	mutex_unlock(&local->iflist_mtx);
+
+	return superchan;
+}
 
-	switch (superchan) {
+static bool
+ieee80211_channel_types_are_compatible(enum nl80211_channel_type chantype1,
+				       enum nl80211_channel_type chantype2,
+				       enum nl80211_channel_type *compat)
+{
+	switch (chantype1) {
 	case NL80211_CHAN_NO_HT:
 	case NL80211_CHAN_HT20:
 		/*
 		 * allow any change that doesn't go to no-HT
 		 * (if it already is no-HT no change is needed)
 		 */
-		if (chantype == NL80211_CHAN_NO_HT)
+		if (chantype2 == NL80211_CHAN_NO_HT)
 			break;
-		superchan = chantype;
+		*compat = chantype2;
 		break;
 	case NL80211_CHAN_HT40PLUS:
 	case NL80211_CHAN_HT40MINUS:
+		*compat = chantype1;
 		/* allow smaller bandwidth and same */
-		if (chantype == NL80211_CHAN_NO_HT)
+		if (chantype2 == NL80211_CHAN_NO_HT)
 			break;
-		if (chantype == NL80211_CHAN_HT20)
+		if (chantype2 == NL80211_CHAN_HT20)
 			break;
-		if (superchan == chantype)
+		if (chantype2 == chantype1)
 			break;
-		result = false;
-		goto out;
+		return false;
 	}
 
-	local->_oper_channel_type = superchan;
+	return true;
+}
+
+bool ieee80211_set_channel_type(struct ieee80211_local *local,
+				struct ieee80211_sub_if_data *sdata,
+				enum nl80211_channel_type chantype)
+{
+	enum nl80211_channel_type superchan;
+	enum nl80211_channel_type compatchan = NL80211_CHAN_NO_HT;
+
+	superchan = ieee80211_get_superchan(local, sdata);
+	if (!ieee80211_channel_types_are_compatible(superchan, chantype,
+						    &compatchan))
+		return false;
+
+	local->_oper_channel_type = compatchan;
 
 	if (sdata)
 		sdata->vif.bss_conf.channel_type = chantype;
 
-	result = true;
- out:
-	mutex_unlock(&local->iflist_mtx);
+	return true;
 
-	return result;
 }
 
 static struct ieee80211_channel_context *
-- 
1.7.0.4

[RFC 6/6] mac80211: reuse channels for channel context

[Michal Kazior]

Reuse channels with compatible channel types. Some
channel types are compatible and can be used
concurrently.

Change-Id: I30665b42ea141ed63bba94be6fd2755c7e16589d
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/mac80211/chan.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 030d44e..75549bc 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -162,6 +162,7 @@ ieee80211_find_channel_context(struct ieee80211_local *local,
 			       enum nl80211_channel_type channel_type)
 {
 	struct ieee80211_channel_context *ctx;
+	enum nl80211_channel_type compat_type;
 	int i;
 
 	if (WARN_ON(!channel))
@@ -169,10 +170,20 @@ ieee80211_find_channel_context(struct ieee80211_local *local,
 
 	for (i = 0; i < IEEE80211_MAX_CHANNEL_CONTEXTS; i++) {
 		ctx = &local->channel_contexts[i];
+		compat_type = ctx->channel_type;
+
 		if (ctx->channel != channel)
 			continue;
 		if (ctx->channel_type != channel_type)
 			continue;
+		if (!ieee80211_channel_types_are_compatible(ctx->channel_type,
+							    channel_type,
+							    &compat_type))
+			continue;
+		if (ctx->channel_type != compat_type) {
+			ctx->channel_type = compat_type;
+			drv_change_channel_type(local, ctx);
+		}
 
 		return ctx;
 	}
-- 
1.7.0.4

Re: [RFC 1/6] mac80211: introduce channel contexts skeleton code

[Johannes Berg]

On Fri, 2012-05-18 at 14:03 +0200, Michal Kazior wrote:

>  /**
> + * struct ieee80211_channel_context - channel context that vifs may be tuned to
> + *
> + * @channel: the channel to tune to
> + * @channel_type: the channel (HT) type
> + * @vif_list: vifs bound to channel context
> + */
> +struct ieee80211_channel_context {
> +	struct ieee80211_channel *channel;
> +	enum nl80211_channel_type channel_type;
> +
> +	struct list_head vif_list;
> +};

A few trivial comments on this particular struct: I think we should have
an internal and an external struct, so that the list for example isn't
accessible by drivers. OTOH, maybe it should be accessible?

The other request I have would be for private space in here for the
driver, like inside vif or hw structs.

Anyway, I'll look over this in more detail, but might not get to it
before the week after next.

johannes

Re: [RFC 5/6] mac80211: refactor set_channel_type

[Eliad Peller]

hi Michal,

On Fri, May 18, 2012 at 3:03 PM, Michal Kazior <michal.kazior@tieto.com> wrote:
> Split functionality for further reuse.
>
> Will prevent code duplication when channel context
> channel_type merging is introduced.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
[...]

> -       switch (superchan) {
> +static bool
> +ieee80211_channel_types_are_compatible(enum nl80211_channel_type chantype1,
> +                                      enum nl80211_channel_type chantype2,
> +                                      enum nl80211_channel_type *compat)
> +{

you split superchan (in + out) into chantype1 (in) + compat (out)
here, but it looks a bit broken -

> +       switch (chantype1) {
>        case NL80211_CHAN_NO_HT:
>        case NL80211_CHAN_HT20:
>                /*
>                 * allow any change that doesn't go to no-HT
>                 * (if it already is no-HT no change is needed)
>                 */
> -               if (chantype == NL80211_CHAN_NO_HT)
> +               if (chantype2 == NL80211_CHAN_NO_HT)
>                        break;

you don't set compat here...

> +
> +bool ieee80211_set_channel_type(struct ieee80211_local *local,
> +                               struct ieee80211_sub_if_data *sdata,
> +                               enum nl80211_channel_type chantype)
> +{
> +       enum nl80211_channel_type superchan;
> +       enum nl80211_channel_type compatchan = NL80211_CHAN_NO_HT;
> +
> +       superchan = ieee80211_get_superchan(local, sdata);
> +       if (!ieee80211_channel_types_are_compatible(superchan, chantype,
> +                                                   &compatchan))
> +               return false;
> +
> +       local->_oper_channel_type = compatchan;
>
so when superchan=NL80211_CHAN_HT20, and chantype=NL80211_CHAN_NO_HT,
you'll end up with compatchan=NL80211_CHAN_NO_HT which is wrong.

Eliad.

Re: [RFC 1/6] mac80211: introduce channel contexts skeleton code

[Johannes Berg]

On Fri, 2012-05-18 at 14:03 +0200, Michal Kazior wrote:
> Channel context are the foundation for
> multi-channel operation.
> 
> Channel contexts are immutable and are re-created
> (or re-used if other interfaces are bound to a
> certain channel) on channel switching.

That makes sense, although I think we may need to have context channel
changing APIs too, later, for things like CSA. We'll also need channel
type changing, see below.

>  /**
> + * struct ieee80211_channel_context - channel context that vifs may be tuned to
> + *
> + * @channel: the channel to tune to
> + * @channel_type: the channel (HT) type
> + * @vif_list: vifs bound to channel context
> + */
> +struct ieee80211_channel_context {
> +	struct ieee80211_channel *channel;
> +	enum nl80211_channel_type channel_type;
> +
> +	struct list_head vif_list;
> +};

I think I'd prefer this struct to be split into mac80211 internal and
driver-visible pieces, like ieee80211_sub_if_data / ieee80211_vif or
ieee80211_local / ieee80211_hw.

> @@ -909,6 +927,9 @@ struct ieee80211_vif {
>  	u8 cab_queue;
>  	u8 hw_queue[IEEE80211_NUM_ACS];
>  
> +	struct ieee80211_channel_context *channel_context;

That makes sense,

> +	struct list_head list;

I'm not sure this does? I have a feeling it should be inside the
internal sub_if_data struct in mac80211 so we can control access to it
and the driver doesn't modify things etc. We would have iteration
functions that also guarantee the right locking.

> +static struct ieee80211_channel_context *
> +ieee80211_find_channel_context(struct ieee80211_local *local,
> +			       struct ieee80211_channel *channel,
> +			       enum nl80211_channel_type channel_type)
> +{
> +	struct ieee80211_channel_context *ctx;
> +	int i;
> +
> +	if (WARN_ON(!channel))
> +		return NULL;
> +
> +	for (i = 0; i < IEEE80211_MAX_CHANNEL_CONTEXTS; i++) {
> +		ctx = &local->channel_contexts[i];

Given my earlier request of having driver-private data inside the
channel context struct, we won't be able to have an array, so we should
have a doubly-linked list instead.

> +		if (ctx->channel != channel)
> +			continue;
> +		if (ctx->channel_type != channel_type)
> +			continue;

I think we need to take compatibility into account here.

If the new channel type is HT_20 and the old one was NO_HT, then we
should reconfigure this context to HT_20 instead of using a new one.
>From a software POV that doesn't matter, but where it maps to hardware
contexts we only have a limited number of them and using just one for
compatible ones is better.

johannes

Re: [RFC 2/6] mac80211: introduce new ieee80211_ops

[Johannes Berg]

On Fri, 2012-05-18 at 14:03 +0200, Michal Kazior wrote:

> + * @change_channel_type: Notifies device driver about channel context
> + *	channel_type change which may happen when combining different vifs on
> + *	a same channel with different HTs.

Oh, so you did take this into account, but not in the first patch yet I
guess then.

These totally make sense to me. Not sure I see a need to split patch 2
and 3 though :-)

johannes

Re: [RFC 3/6] mac80211: add drv_* wrappers for channel contexts

[Johannes Berg]

On Fri, 2012-05-18 at 14:03 +0200, Michal Kazior wrote:

> +static inline void
> +drv_add_channel_context(struct ieee80211_local *local,
> +		        struct ieee80211_channel_context *ctx)
> +{
> +	if (local->ops->add_channel_context)
> +		local->ops->add_channel_context(&local->hw, ctx);
> +}

tracing would be good, but otherwise this is obviously good

johannes

Re: [RFC 1/6] mac80211: introduce channel contexts skeleton code

[Johannes Berg]

On Fri, 2012-05-18 at 14:03 +0200, Michal Kazior wrote:

> +void
> +ieee80211_vif_release_channel(struct ieee80211_sub_if_data *sdata)
> +{

Ok one more comment on all these functions -- I'd like to add some
lockdep assertion here or so to make sure we're always protected
correctly. I haven't thought about which lock it needs to be though --
maybe a new one for the channel context stuff?

johannes

Re: [RFC 6/6] mac80211: reuse channels for channel context

[Johannes Berg]

On Fri, 2012-05-18 at 14:03 +0200, Michal Kazior wrote:
> Reuse channels with compatible channel types. Some
> channel types are compatible and can be used
> concurrently.

Yeah, ok, I wasn't paying attention to the last patch when looking at
the first! :-)

johannes