[RFC 0/2] mac80211 queue redesign

[RFC 0/2] mac80211 queue redesign

[Johannes Berg]

Here's the queue redesign we've been talking about.

The monitor patch is needed because when a driver actually
uses the new queue feature there isn't any queue to TX
frames on for purely virtual interfaces. This only affects
monitor (AP_VLAN are tied to AP) and only when injecting
frames that don't have a TA matching another interface (as
we bind the frames to that interface then). That, however
is probably the biggest use case (now that monitor-for-AP
is dying). As a consequence, drivers wanting to support
"pure" injection and using the new queue features need to
support the pure monitor interface feature as well. Also,
the pure monitor feature will help implement monitor mode
on devices where that is mutually exclusive with other
modes (like iwlwifi.)

The monitor feature I've tested in hwsim, it works. The
queue feature isn't tested at all, I haven't even done
any kind of regression tests for drivers not using it.

Thanks goes out to everybody I discussed this with (on
IRC or elsewhere), and also Michal for reviewing a first
version of this patch :-)

johannes

[RFC 1/2] mac80211: add explicit monitor interface if needed

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

The queue mapping redesign that I'm planning to do
will break pure injection unless we handle monitor
interfaces explicitly. One possible option would
be to have the driver tell mac80211 about monitor
mode queues etc., but that would duplicate the API
since we already need to have queue assignments
handled per virtual interface.

So in order to solve this, have a virtual monitor
interface that is added whenever all active vifs
are monitors. We could also use the state of one
of the monitor interfaces, but managing that would
be complicated, so allocate separate state.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 drivers/net/wireless/mac80211_hwsim.c |    3 +
 include/net/mac80211.h                |    6 ++-
 net/mac80211/driver-ops.h             |    3 -
 net/mac80211/ieee80211_i.h            |    3 +
 net/mac80211/iface.c                  |   65 ++++++++++++++++++++++++++++++++++
 net/mac80211/pm.c                     |    4 ++
 net/mac80211/tx.c                     |    7 ++-
 net/mac80211/util.c                   |   10 +++++
 8 files changed, 95 insertions(+), 6 deletions(-)

--- a/net/mac80211/ieee80211_i.h	2012-03-28 10:29:13.000000000 +0200
+++ b/net/mac80211/ieee80211_i.h	2012-03-28 10:39:22.000000000 +0200
@@ -1077,6 +1077,9 @@ struct ieee80211_local {
 	struct net_device napi_dev;
 
 	struct napi_struct napi;
+
+	/* virtual monitor interface */
+	struct ieee80211_sub_if_data __rcu *monitor_sdata;
 };
 
 static inline struct ieee80211_sub_if_data *
--- a/net/mac80211/pm.c	2012-03-28 10:29:13.000000000 +0200
+++ b/net/mac80211/pm.c	2012-03-28 10:29:13.000000000 +0200
@@ -127,6 +127,10 @@ int __ieee80211_suspend(struct ieee80211
 		drv_remove_interface(local, sdata);
 	}
 
+	sdata = rtnl_dereference(local->monitor_sdata);
+	if (sdata)
+		drv_remove_interface(local, sdata);
+
 	/* stop hardware - this must stop RX */
 	if (local->open_count)
 		ieee80211_stop_device(local);
--- a/include/net/mac80211.h	2012-03-28 10:29:13.000000000 +0200
+++ b/include/net/mac80211.h	2012-03-28 10:39:22.000000000 +0200
@@ -1173,6 +1173,10 @@ enum sta_notify_cmd {
  * @IEEE80211_HW_SCAN_WHILE_IDLE: The device can do hw scan while
  *	being idle (i.e. mac80211 doesn't have to go idle-off during the
  *	the scan).
+ *
+ * @IEEE80211_HW_WANT_MONITOR_VIF: The driver would like to be informed of
+ *	a virtual monitor interface when monitor interfaces are the only
+ *	active interfaces.
  */
 enum ieee80211_hw_flags {
 	IEEE80211_HW_HAS_RATE_CONTROL			= 1<<0,
@@ -1189,7 +1193,7 @@ enum ieee80211_hw_flags {
 	IEEE80211_HW_PS_NULLFUNC_STACK			= 1<<11,
 	IEEE80211_HW_SUPPORTS_DYNAMIC_PS		= 1<<12,
 	IEEE80211_HW_MFP_CAPABLE			= 1<<13,
-	/* reuse bit 14 */
+	IEEE80211_HW_WANT_MONITOR_VIF			= 1<<14,
 	IEEE80211_HW_SUPPORTS_STATIC_SMPS		= 1<<15,
 	IEEE80211_HW_SUPPORTS_DYNAMIC_SMPS		= 1<<16,
 	IEEE80211_HW_SUPPORTS_UAPSD			= 1<<17,
--- a/net/mac80211/iface.c	2012-03-28 10:29:13.000000000 +0200
+++ b/net/mac80211/iface.c	2012-03-28 10:39:38.000000000 +0200
@@ -169,6 +169,59 @@ void ieee80211_adjust_monitor_flags(stru
 #undef ADJUST
 }
 
+static int ieee80211_add_virtual_monitor(struct ieee80211_local *local)
+{
+	struct ieee80211_sub_if_data *sdata;
+	int ret;
+
+	if (!(local->hw.flags & IEEE80211_HW_WANT_MONITOR_VIF))
+		return 0;
+
+	if (local->monitor_sdata)
+		return 0;
+
+	sdata = kzalloc(sizeof(*sdata) + local->hw.vif_data_size, GFP_KERNEL);
+	if (!sdata)
+		return -ENOMEM;
+
+	/* set up data */
+	sdata->local = local;
+	sdata->vif.type = NL80211_IFTYPE_MONITOR;
+	snprintf(sdata->name, IFNAMSIZ, "%s-monitor",
+		 wiphy_name(local->hw.wiphy));
+
+	ret = drv_add_interface(local, sdata);
+	if (WARN_ON(ret)) {
+		/* ok .. stupid driver, it asked for this! */
+		kfree(sdata);
+		return ret;
+	}
+
+	rcu_assign_pointer(local->monitor_sdata, sdata);
+
+	return 0;
+}
+
+static void ieee80211_del_virtual_monitor(struct ieee80211_local *local)
+{
+	struct ieee80211_sub_if_data *sdata;
+
+	if (!(local->hw.flags & IEEE80211_HW_WANT_MONITOR_VIF))
+		return;
+
+	sdata = rtnl_dereference(local->monitor_sdata);
+
+	if (WARN_ON(!sdata))
+		return;
+
+	rcu_assign_pointer(local->monitor_sdata, NULL);
+	synchronize_net();
+
+	drv_remove_interface(local, sdata);
+
+	kfree(sdata);
+}
+
 /*
  * NOTE: Be very careful when changing this function, it must NOT return
  * an error on interface type changes that have been pre-checked, so most
@@ -266,6 +319,12 @@ static int ieee80211_do_open(struct net_
 			break;
 		}
 
+		if (local->monitors == 0 && local->open_count == 0) {
+			res = ieee80211_add_virtual_monitor(local);
+			if (res)
+				return res;
+		}
+
 		/* must be before the call to ieee80211_configure_filter */
 		local->monitors++;
 		if (local->monitors == 1) {
@@ -280,6 +339,8 @@ static int ieee80211_do_open(struct net_
 		break;
 	default:
 		if (coming_up) {
+			ieee80211_del_virtual_monitor(local);
+
 			res = drv_add_interface(local, sdata);
 			if (res)
 				goto err_stop;
@@ -511,6 +572,7 @@ static void ieee80211_do_stop(struct iee
 		if (local->monitors == 0) {
 			local->hw.conf.flags &= ~IEEE80211_CONF_MONITOR;
 			hw_reconf_flags |= IEEE80211_CONF_CHANGE_MONITOR;
+			ieee80211_del_virtual_monitor(local);
 		}
 
 		ieee80211_adjust_monitor_flags(sdata, -1);
@@ -584,6 +646,9 @@ static void ieee80211_do_stop(struct iee
 		}
 	}
 	spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
+
+	if (local->monitors == local->open_count && local->monitors > 0)
+		ieee80211_add_virtual_monitor(local);
 }
 
 static int ieee80211_stop(struct net_device *dev)
--- a/net/mac80211/util.c	2012-03-28 10:29:13.000000000 +0200
+++ b/net/mac80211/util.c	2012-03-28 10:39:22.000000000 +0200
@@ -1213,6 +1213,16 @@ int ieee80211_reconfig(struct ieee80211_
 				   IEEE80211_TPT_LEDTRIG_FL_RADIO, 0);
 
 	/* add interfaces */
+	sdata = rtnl_dereference(local->monitor_sdata);
+	if (sdata) {
+		res = drv_add_interface(local, sdata);
+		if (WARN_ON(res)) {
+			rcu_assign_pointer(local->monitor_sdata, NULL);
+			synchronize_net();
+			kfree(sdata);
+		}
+	}
+
 	list_for_each_entry(sdata, &local->interfaces, list) {
 		if (sdata->vif.type != NL80211_IFTYPE_AP_VLAN &&
 		    sdata->vif.type != NL80211_IFTYPE_MONITOR &&
--- a/net/mac80211/tx.c	2012-03-28 10:29:13.000000000 +0200
+++ b/net/mac80211/tx.c	2012-03-28 10:39:22.000000000 +0200
@@ -1284,8 +1284,11 @@ static bool __ieee80211_tx(struct ieee80
 
 	switch (sdata->vif.type) {
 	case NL80211_IFTYPE_MONITOR:
-		sdata = NULL;
-		vif = NULL;
+		sdata = rcu_dereference(local->monitor_sdata);
+		if (sdata)
+			vif = &sdata->vif;
+		else
+			vif = NULL;
 		break;
 	case NL80211_IFTYPE_AP_VLAN:
 		sdata = container_of(sdata->bss,
--- a/drivers/net/wireless/mac80211_hwsim.c	2012-03-28 10:29:13.000000000 +0200
+++ b/drivers/net/wireless/mac80211_hwsim.c	2012-03-28 10:29:13.000000000 +0200
@@ -1789,7 +1789,8 @@ static int __init init_mac80211_hwsim(vo
 			    IEEE80211_HW_SIGNAL_DBM |
 			    IEEE80211_HW_SUPPORTS_STATIC_SMPS |
 			    IEEE80211_HW_SUPPORTS_DYNAMIC_SMPS |
-			    IEEE80211_HW_AMPDU_AGGREGATION;
+			    IEEE80211_HW_AMPDU_AGGREGATION |
+			    IEEE80211_HW_WANT_MONITOR_VIF;
 
 		hw->wiphy->flags |= WIPHY_FLAG_SUPPORTS_TDLS |
 				    WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL;
--- a/net/mac80211/driver-ops.h	2012-03-28 10:29:13.000000000 +0200
+++ b/net/mac80211/driver-ops.h	2012-03-28 10:29:13.000000000 +0200
@@ -98,8 +98,7 @@ static inline int drv_add_interface(stru
 
 	might_sleep();
 
-	if (WARN_ON(sdata->vif.type == NL80211_IFTYPE_AP_VLAN ||
-		    sdata->vif.type == NL80211_IFTYPE_MONITOR))
+	if (WARN_ON(sdata->vif.type == NL80211_IFTYPE_AP_VLAN))
 		return -EINVAL;
 
 	trace_drv_add_interface(local, sdata);

Re: [RFC 1/2] mac80211: add explicit monitor interface if needed

[Eliad Peller]

On Wed, Mar 28, 2012 at 11:13 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> The queue mapping redesign that I'm planning to do
> will break pure injection unless we handle monitor
> interfaces explicitly. One possible option would
> be to have the driver tell mac80211 about monitor
> mode queues etc., but that would duplicate the API
> since we already need to have queue assignments
> handled per virtual interface.
>
> So in order to solve this, have a virtual monitor
> interface that is added whenever all active vifs
> are monitors. We could also use the state of one
> of the monitor interfaces, but managing that would
> be complicated, so allocate separate state.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
[...]

> +static void ieee80211_del_virtual_monitor(struct ieee80211_local *local)
> +{
> +       struct ieee80211_sub_if_data *sdata;
> +
> +       if (!(local->hw.flags & IEEE80211_HW_WANT_MONITOR_VIF))
> +               return;
> +
> +       sdata = rtnl_dereference(local->monitor_sdata);
> +
> +       if (WARN_ON(!sdata))
> +               return;
> +
...

> @@ -280,6 +339,8 @@ static int ieee80211_do_open(struct net_
>                break;
>        default:
>                if (coming_up) {
> +                       ieee80211_del_virtual_monitor(local);
> +

i think this will always trigger the warning?

> @@ -98,8 +98,7 @@ static inline int drv_add_interface(stru
>
>        might_sleep();
>
> -       if (WARN_ON(sdata->vif.type == NL80211_IFTYPE_AP_VLAN ||
> -                   sdata->vif.type == NL80211_IFTYPE_MONITOR))
> +       if (WARN_ON(sdata->vif.type == NL80211_IFTYPE_AP_VLAN))
>                return -EINVAL;
>
maybe check for IEEE80211_HW_WANT_MONITOR_VIF instead?

Eliad.

Re: [RFC 1/2] mac80211: add explicit monitor interface if needed

[Johannes Berg]

On Thu, 2012-03-29 at 22:09 +0200, Eliad Peller wrote:

> > +       if (WARN_ON(!sdata))
> > +               return;
> > +
> ...
> 
> > @@ -280,6 +339,8 @@ static int ieee80211_do_open(struct net_
> >                break;
> >        default:
> >                if (coming_up) {
> > +                       ieee80211_del_virtual_monitor(local);
> > +
> 
> i think this will always trigger the warning?

Yeah, somehow I didn't see this in my first round of testing. I
distinctly remember thinking "I need to remove the warning then" but
clearly never did :-)

> > -       if (WARN_ON(sdata->vif.type == NL80211_IFTYPE_AP_VLAN ||
> > -                   sdata->vif.type == NL80211_IFTYPE_MONITOR))
> > +       if (WARN_ON(sdata->vif.type == NL80211_IFTYPE_AP_VLAN))
> >                return -EINVAL;
> >
> maybe check for IEEE80211_HW_WANT_MONITOR_VIF instead?

Good point, should do that.

johannes

[RFC 2/2] mac80211: add improved HW queue control

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>



Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/mac80211.h     |   81 ++++++++++++++++++++++++++++++++++++++++++---
 net/mac80211/agg-tx.c      |   11 +++---
 net/mac80211/cfg.c         |    6 +++
 net/mac80211/ieee80211_i.h |    1 
 net/mac80211/iface.c       |   54 ++++++++++++++++++++++++++++++
 net/mac80211/main.c        |    5 ++
 net/mac80211/tx.c          |   38 ++++++++++++++++-----
 net/mac80211/util.c        |   48 +++++++++++++++++++++-----
 8 files changed, 216 insertions(+), 28 deletions(-)

--- a/include/net/mac80211.h	2012-03-28 10:39:22.000000000 +0200
+++ b/include/net/mac80211.h	2012-03-28 10:40:04.000000000 +0200
@@ -93,9 +93,11 @@
  * @IEEE80211_MAX_QUEUES: Maximum number of regular device queues.
  */
 enum ieee80211_max_queues {
-	IEEE80211_MAX_QUEUES =		4,
+	IEEE80211_MAX_QUEUES =		16,
 };
 
+#define IEEE80211_INVAL_HW_QUEUE	0xff
+
 /**
  * enum ieee80211_ac_numbers - AC numbers as used in mac80211
  * @IEEE80211_AC_VO: voice
@@ -520,7 +522,7 @@ struct ieee80211_tx_rate {
  *
  * @flags: transmit info flags, defined above
  * @band: the band to transmit on (use for checking for races)
- * @reserved: reserved for future use
+ * @hw_queue: HW queue to put the frame on, skb_get_queue_mapping() gives the AC
  * @ack_frame_id: internal frame ID for TX status, used internally
  * @control: union for control data
  * @status: union for status data
@@ -536,7 +538,7 @@ struct ieee80211_tx_info {
 	u32 flags;
 	u8 band;
 
-	u8 reserved;
+	u8 hw_queue;
 
 	u16 ack_frame_id;
 
@@ -887,6 +889,8 @@ enum ieee80211_vif_flags {
  *	these need to be set (or cleared) when the interface is added
  *	or, if supported by the driver, the interface type is changed
  *	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
  * @drv_priv: data area for driver use, will always be aligned to
  *	sizeof(void *).
  */
@@ -895,7 +899,12 @@ struct ieee80211_vif {
 	struct ieee80211_bss_conf bss_conf;
 	u8 addr[ETH_ALEN];
 	bool p2p;
+
+	u8 cab_queue;
+	u8 hw_queue[IEEE80211_NUM_ACS];
+
 	u32 driver_flags;
+
 	/* must be last */
 	u8 drv_priv[0] __attribute__((__aligned__(sizeof(void *))));
 };
@@ -1177,6 +1186,11 @@ enum sta_notify_cmd {
  * @IEEE80211_HW_WANT_MONITOR_VIF: The driver would like to be informed of
  *	a virtual monitor interface when monitor interfaces are the only
  *	active interfaces.
+ *
+ * @IEEE80211_HW_QUEUE_CONTROL: The driver wants to control per-interface
+ *	queue mapping in order to use different queues (not just one per AC)
+ *	for different virtual interfaces. See the doc section on HW queue
+ *	control for more details.
  */
 enum ieee80211_hw_flags {
 	IEEE80211_HW_HAS_RATE_CONTROL			= 1<<0,
@@ -1199,7 +1213,7 @@ enum ieee80211_hw_flags {
 	IEEE80211_HW_SUPPORTS_UAPSD			= 1<<17,
 	IEEE80211_HW_REPORTS_TX_ACK_STATUS		= 1<<18,
 	IEEE80211_HW_CONNECTION_MONITOR			= 1<<19,
-	/* reuse bit 20 */
+	IEEE80211_HW_QUEUE_CONTROL			= 1<<20,
 	IEEE80211_HW_SUPPORTS_PER_STA_GTK		= 1<<21,
 	IEEE80211_HW_AP_LINK_PS				= 1<<22,
 	IEEE80211_HW_TX_AMPDU_SETUP_IN_HW		= 1<<23,
@@ -1269,6 +1283,9 @@ enum ieee80211_hw_flags {
  * @max_tx_aggregation_subframes: maximum number of subframes in an
  *	aggregate an HT driver will transmit, used by the peer as a
  *	hint to size its reorder buffer.
+ *
+ * @offchannel_tx_hw_queue: HW queue ID to use for offchannel TX
+ *	(if %IEEE80211_HW_QUEUE_CONTROL is set)
  */
 struct ieee80211_hw {
 	struct ieee80211_conf conf;
@@ -1289,6 +1306,7 @@ struct ieee80211_hw {
 	u8 max_rate_tries;
 	u8 max_rx_aggregation_subframes;
 	u8 max_tx_aggregation_subframes;
+	u8 offchannel_tx_hw_queue;
 };
 
 /**
@@ -1697,6 +1715,61 @@ void ieee80211_free_txskb(struct ieee802
  */
 
 /**
+ * DOC: HW queue control
+ *
+ * Before HW queue control was introduced, mac80211 only had a single static
+ * assignment of per-interface AC software queues to hardware queues. This
+ * was problematic for a few reasons:
+ * 1) off-channel transmissions might get stuck behind other frames
+ * 2) multiple virtual interfaces couldn't be handled correctly
+ * 3) after-DTIM frames could get stuck behind other frames
+ *
+ * To solve this, hardware typically uses multiple different queues for all
+ * the different usages, and this needs to be propagated into mac80211 so it
+ * won't have the same problem with the software queues.
+ *
+ * Therefore, mac80211 now offers the %IEEE80211_HW_QUEUE_CONTROL capability
+ * flag that tells it that the driver implements its own queue control. To do
+ * so, the driver will set up the various queues in each &struct ieee80211_vif
+ * and the offchannel queue in &struct ieee80211_hw. In response, mac80211 will
+ * use those queue IDs in the hw_queue field of &struct ieee80211_tx_info and
+ * if necessary will queue the frame on the right software queue that mirrors
+ * the hardware queue.
+ * Additionally, the driver has to then use these HW queue IDs for the queue
+ * management functions (ieee80211_stop_queue() et al.)
+ *
+ * The driver is free to set up the queue mappings as needed, multiple virtual
+ * interfaces may map to the same hardware queues if needed. The setup has to
+ * happen during add_interface or change_interface callbacks. For example, a
+ * driver supporting station+station and station+AP modes might decide to have
+ * 10 hardware queues to handle different scenarios:
+ *
+ * 4 AC HW queues for 1st vif: 0, 1, 2, 3
+ * 4 AC HW queues for 2nd vif: 4, 5, 6, 7
+ * after-DTIM queue for AP:   8
+ * off-channel queue:         9
+ *
+ * It would then set up the hardware like this:
+ *   hw.offchannel_tx_hw_queue = 9
+ *
+ * and the first virtual interface that is added as follows:
+ *   vif.hw_queue[IEEE80211_AC_VO] = 0
+ *   vif.hw_queue[IEEE80211_AC_VI] = 1
+ *   vif.hw_queue[IEEE80211_AC_BE] = 2
+ *   vif.hw_queue[IEEE80211_AC_BK] = 3
+ *   vif.cab_queue = 8 // if AP mode, otherwise %IEEE80211_INVAL_HW_QUEUE
+ * and the second virtual interface with 4-7.
+ *
+ * If queue 6 gets full, for example, mac80211 would only stop the second
+ * virtual interface's BE queue since virtual interface queues are per AC.
+ *
+ * Note that the vif.cab_queue value should be set to %IEEE80211_INVAL_HW_QUEUE
+ * whenever the queue is not used (i.e. the interface is not in AP mode) if the
+ * queue could potentially be shared since mac80211 will look at cab_queue when
+ * a queue is stopped/woken even if the interface is not in AP mode.
+ */
+
+/**
  * enum ieee80211_filter_flags - hardware filter flags
  *
  * These flags determine what the filter in hardware should be
--- a/net/mac80211/iface.c	2012-03-28 10:39:38.000000000 +0200
+++ b/net/mac80211/iface.c	2012-03-28 10:48:48.000000000 +0200
@@ -149,6 +149,26 @@ static int ieee80211_check_concurrent_if
 	return 0;
 }
 
+static int ieee80211_check_queues(struct ieee80211_sub_if_data *sdata)
+{
+	int i;
+
+	for (i = 0; i < IEEE80211_NUM_ACS; i++)
+		if (WARN_ON_ONCE(sdata->vif.hw_queue[i] ==
+				 IEEE80211_INVAL_HW_QUEUE))
+			return -EINVAL;
+
+	if (WARN_ON_ONCE(sdata->vif.type == NL80211_IFTYPE_AP &&
+			 sdata->vif.cab_queue == IEEE80211_INVAL_HW_QUEUE))
+		return -EINVAL;
+
+	if (WARN_ON_ONCE(sdata->vif.type != NL80211_IFTYPE_AP &&
+			 sdata->vif.cab_queue != IEEE80211_INVAL_HW_QUEUE))
+		return -EINVAL;
+
+	return 0;
+}
+
 void ieee80211_adjust_monitor_flags(struct ieee80211_sub_if_data *sdata,
 				    const int offset)
 {
@@ -169,6 +189,20 @@ void ieee80211_adjust_monitor_flags(stru
 #undef ADJUST
 }
 
+static void ieee80211_set_default_queues(struct ieee80211_sub_if_data *sdata)
+{
+	struct ieee80211_local *local = sdata->local;
+	int i;
+
+	for (i = 0; i < IEEE80211_NUM_ACS; i++) {
+		if (local->hw.flags & IEEE80211_HW_QUEUE_CONTROL)
+			sdata->vif.hw_queue[i] = IEEE80211_INVAL_HW_QUEUE;
+		else
+			sdata->vif.hw_queue[i] = i;
+	}
+	sdata->vif.cab_queue = IEEE80211_INVAL_HW_QUEUE;
+}
+
 static int ieee80211_add_virtual_monitor(struct ieee80211_local *local)
 {
 	struct ieee80211_sub_if_data *sdata;
@@ -190,6 +224,8 @@ static int ieee80211_add_virtual_monitor
 	snprintf(sdata->name, IFNAMSIZ, "%s-monitor",
 		 wiphy_name(local->hw.wiphy));
 
+	ieee80211_set_default_queues(sdata);
+
 	ret = drv_add_interface(local, sdata);
 	if (WARN_ON(ret)) {
 		/* ok .. stupid driver, it asked for this! */
@@ -197,6 +233,12 @@ static int ieee80211_add_virtual_monitor
 		return ret;
 	}
 
+	ret = ieee80211_check_queues(sdata);
+	if (ret) {
+		kfree(sdata);
+		return ret;
+	}
+
 	rcu_assign_pointer(local->monitor_sdata, sdata);
 
 	return 0;
@@ -344,6 +386,9 @@ static int ieee80211_do_open(struct net_
 			res = drv_add_interface(local, sdata);
 			if (res)
 				goto err_stop;
+			res = ieee80211_check_queues(sdata);
+			if (res)
+				goto err_stop;
 		}
 
 		if (sdata->vif.type == NL80211_IFTYPE_AP) {
@@ -1040,6 +1085,13 @@ static int ieee80211_runtime_change_ifty
 	if (ret)
 		type = sdata->vif.type;
 
+	/*
+	 * Ignore return value here, there's not much we can do since
+	 * the driver changed the interface type internally already.
+	 * The warnings will hopefully make driver authors fix it :-)
+	 */
+	ieee80211_check_queues(sdata);
+
 	ieee80211_setup_sdata(sdata, type);
 
 	err = ieee80211_do_open(sdata->dev, false);
@@ -1266,6 +1318,8 @@ int ieee80211_if_add(struct ieee80211_lo
 			       sizeof(sdata->rc_rateidx_mcs_mask[i]));
 	}
 
+	ieee80211_set_default_queues(sdata);
+
 	/* setup type-dependent data */
 	ieee80211_setup_sdata(sdata, type);
 
--- a/net/mac80211/util.c	2012-03-28 10:39:22.000000000 +0200
+++ b/net/mac80211/util.c	2012-03-28 10:48:27.000000000 +0200
@@ -265,11 +265,36 @@ __le16 ieee80211_ctstoself_duration(stru
 }
 EXPORT_SYMBOL(ieee80211_ctstoself_duration);
 
+void ieee80211_propagate_queue_wake(struct ieee80211_local *local, int queue)
+{
+	struct ieee80211_sub_if_data *sdata;
+
+	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
+		int ac;
+
+		if (test_bit(SDATA_STATE_OFFCHANNEL, &sdata->state))
+			continue;
+
+		if (sdata->vif.cab_queue != IEEE80211_INVAL_HW_QUEUE &&
+		    local->queue_stop_reasons[sdata->vif.cab_queue] != 0)
+			continue;
+
+		for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
+			int ac_queue = sdata->vif.hw_queue[ac];
+
+			if (ac_queue == queue ||
+			    (sdata->vif.cab_queue == queue &&
+			     local->queue_stop_reasons[ac_queue] == 0 &&
+			     skb_queue_empty(&local->pending[ac_queue])))
+				netif_wake_subqueue(sdata->dev, ac);
+		}
+	}
+}
+
 static void __ieee80211_wake_queue(struct ieee80211_hw *hw, int queue,
 				   enum queue_stop_reason reason)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
-	struct ieee80211_sub_if_data *sdata;
 
 	trace_wake_queue(local, queue, reason);
 
@@ -287,11 +312,7 @@ static void __ieee80211_wake_queue(struc
 
 	if (skb_queue_empty(&local->pending[queue])) {
 		rcu_read_lock();
-		list_for_each_entry_rcu(sdata, &local->interfaces, list) {
-			if (test_bit(SDATA_STATE_OFFCHANNEL, &sdata->state))
-				continue;
-			netif_wake_subqueue(sdata->dev, queue);
-		}
+		ieee80211_propagate_queue_wake(local, queue);
 		rcu_read_unlock();
 	} else
 		tasklet_schedule(&local->tx_pending_tasklet);
@@ -332,8 +353,15 @@ static void __ieee80211_stop_queue(struc
 	__set_bit(reason, &local->queue_stop_reasons[queue]);
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(sdata, &local->interfaces, list)
-		netif_stop_subqueue(sdata->dev, queue);
+	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
+		int ac;
+
+		for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
+			if (sdata->vif.hw_queue[ac] == queue ||
+			    sdata->vif.cab_queue == queue)
+				netif_stop_subqueue(sdata->dev, ac);
+		}
+	}
 	rcu_read_unlock();
 }
 
@@ -360,8 +388,8 @@ void ieee80211_add_pending_skb(struct ie
 {
 	struct ieee80211_hw *hw = &local->hw;
 	unsigned long flags;
-	int queue = skb_get_queue_mapping(skb);
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+	int queue = info->hw_queue;
 
 	if (WARN_ON(!info->control.vif)) {
 		kfree_skb(skb);
@@ -393,7 +421,7 @@ void ieee80211_add_pending_skbs_fn(struc
 			continue;
 		}
 
-		queue = skb_get_queue_mapping(skb);
+		queue = info->hw_queue;
 
 		__ieee80211_stop_queue(hw, queue,
 				IEEE80211_QUEUE_STOP_REASON_SKB_ADD);
--- a/net/mac80211/tx.c	2012-03-28 10:39:22.000000000 +0200
+++ b/net/mac80211/tx.c	2012-03-28 10:50:14.000000000 +0200
@@ -400,6 +400,8 @@ ieee80211_tx_h_multicast_ps_buf(struct i
 		return TX_CONTINUE;
 
 	info->flags |= IEEE80211_TX_CTL_SEND_AFTER_DTIM;
+	if (tx->local->hw.flags & IEEE80211_HW_QUEUE_CONTROL)
+		info->hw_queue = tx->sdata->vif.cab_queue;
 
 	/* device releases frame after DTIM beacon */
 	if (!(tx->local->hw.flags & IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING))
@@ -1215,11 +1217,19 @@ static bool ieee80211_tx_frags(struct ie
 			       bool txpending)
 {
 	struct sk_buff *skb, *tmp;
-	struct ieee80211_tx_info *info;
 	unsigned long flags;
 
 	skb_queue_walk_safe(skbs, skb, tmp) {
-		int q = skb_get_queue_mapping(skb);
+		struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+		int q = info->hw_queue;
+
+#ifdef CONFIG_MAC80211_VERBOSE_DEBUG
+		if (WARN_ON_ONCE(q >= local->hw.queues)) {
+			__skb_unlink(skb, skbs);
+			dev_kfree_skb(skb);
+			continue;
+		}
+#endif
 
 		spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
 		if (local->queue_stop_reasons[q] ||
@@ -1241,7 +1251,6 @@ static bool ieee80211_tx_frags(struct ie
 		}
 		spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
 
-		info = IEEE80211_SKB_CB(skb);
 		info->control.vif = vif;
 		info->control.sta = sta;
 
@@ -1285,9 +1294,14 @@ static bool __ieee80211_tx(struct ieee80
 	switch (sdata->vif.type) {
 	case NL80211_IFTYPE_MONITOR:
 		sdata = rcu_dereference(local->monitor_sdata);
-		if (sdata)
+		if (sdata) {
 			vif = &sdata->vif;
-		else
+			info->hw_queue =
+				vif->hw_queue[skb_get_queue_mapping(skb)];
+		} else if (local->hw.flags & IEEE80211_HW_QUEUE_CONTROL) {
+			dev_kfree_skb(skb);
+			return true;
+		} else
 			vif = NULL;
 		break;
 	case NL80211_IFTYPE_AP_VLAN:
@@ -1403,6 +1417,12 @@ static bool ieee80211_tx(struct ieee8021
 	tx.channel = local->hw.conf.channel;
 	info->band = tx.channel->band;
 
+	/* set up hw_queue value early */
+	if (!(info->flags & IEEE80211_TX_CTL_TX_OFFCHAN) ||
+	    !(local->hw.flags & IEEE80211_HW_QUEUE_CONTROL))
+		info->hw_queue =
+			sdata->vif.hw_queue[skb_get_queue_mapping(skb)];
+
 	if (!invoke_tx_handlers(&tx))
 		result = __ieee80211_tx(local, &tx.skbs, led_len,
 					tx.sta, txpending);
@@ -2173,7 +2193,6 @@ static bool ieee80211_tx_pending_skb(str
 void ieee80211_tx_pending(unsigned long data)
 {
 	struct ieee80211_local *local = (struct ieee80211_local *)data;
-	struct ieee80211_sub_if_data *sdata;
 	unsigned long flags;
 	int i;
 	bool txok;
@@ -2210,8 +2229,7 @@ void ieee80211_tx_pending(unsigned long
 		}
 
 		if (skb_queue_empty(&local->pending[i]))
-			list_for_each_entry_rcu(sdata, &local->interfaces, list)
-				netif_wake_subqueue(sdata->dev, i);
+			ieee80211_propagate_queue_wake(local, i);
 	}
 	spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
 
@@ -2713,11 +2731,13 @@ EXPORT_SYMBOL(ieee80211_get_buffered_bc)
 void ieee80211_tx_skb_tid(struct ieee80211_sub_if_data *sdata,
 			  struct sk_buff *skb, int tid)
 {
+	int ac = ieee802_1d_to_ac[tid];
+
 	skb_set_mac_header(skb, 0);
 	skb_set_network_header(skb, 0);
 	skb_set_transport_header(skb, 0);
 
-	skb_set_queue_mapping(skb, ieee802_1d_to_ac[tid]);
+	skb_set_queue_mapping(skb, ac);
 	skb->priority = tid;
 
 	/*
--- a/net/mac80211/agg-tx.c	2012-03-28 10:39:22.000000000 +0200
+++ b/net/mac80211/agg-tx.c	2012-03-28 10:48:27.000000000 +0200
@@ -314,10 +314,11 @@ ieee80211_wake_queue_agg(struct ieee8021
  * requires a call to ieee80211_agg_splice_finish later
  */
 static void __acquires(agg_queue)
-ieee80211_agg_splice_packets(struct ieee80211_local *local,
+ieee80211_agg_splice_packets(struct ieee80211_sub_if_data *sdata,
 			     struct tid_ampdu_tx *tid_tx, u16 tid)
 {
-	int queue = ieee80211_ac_from_tid(tid);
+	struct ieee80211_local *local = sdata->local;
+	int queue = sdata->vif.hw_queue[ieee80211_ac_from_tid(tid)];
 	unsigned long flags;
 
 	ieee80211_stop_queue_agg(local, tid);
@@ -376,7 +377,7 @@ void ieee80211_tx_ba_session_handle_star
 					" tid %d\n", tid);
 #endif
 		spin_lock_bh(&sta->lock);
-		ieee80211_agg_splice_packets(local, tid_tx, tid);
+		ieee80211_agg_splice_packets(sdata, tid_tx, tid);
 		ieee80211_assign_tid_tx(sta, tid, NULL);
 		ieee80211_agg_splice_finish(local, tid);
 		spin_unlock_bh(&sta->lock);
@@ -586,7 +587,7 @@ static void ieee80211_agg_tx_operational
 	 */
 	spin_lock_bh(&sta->lock);
 
-	ieee80211_agg_splice_packets(local, tid_tx, tid);
+	ieee80211_agg_splice_packets(sta->sdata, tid_tx, tid);
 	/*
 	 * Now mark as operational. This will be visible
 	 * in the TX path, and lets it go lock-free in
@@ -778,7 +779,7 @@ void ieee80211_stop_tx_ba_cb(struct ieee
 	 * more.
 	 */
 
-	ieee80211_agg_splice_packets(local, tid_tx, tid);
+	ieee80211_agg_splice_packets(sta->sdata, tid_tx, tid);
 
 	/* future packets must not find the tid_tx struct any more */
 	ieee80211_assign_tid_tx(sta, tid, NULL);
--- a/net/mac80211/cfg.c	2012-03-28 10:39:22.000000000 +0200
+++ b/net/mac80211/cfg.c	2012-03-28 10:48:27.000000000 +0200
@@ -2098,6 +2098,10 @@ static int ieee80211_mgmt_tx(struct wiph
 
 	IEEE80211_SKB_CB(skb)->flags = flags;
 
+	if (flags & IEEE80211_TX_CTL_TX_OFFCHAN)
+		IEEE80211_SKB_CB(skb)->hw_queue =
+			local->hw.offchannel_tx_hw_queue;
+
 	skb->dev = sdata->dev;
 
 	*cookie = (unsigned long) skb;
@@ -2139,6 +2143,8 @@ static int ieee80211_mgmt_tx(struct wiph
 		/* modify cookie to prevent API mismatches */
 		*cookie ^= 2;
 		IEEE80211_SKB_CB(skb)->flags |= IEEE80211_TX_CTL_TX_OFFCHAN;
+		IEEE80211_SKB_CB(skb)->hw_queue =
+			local->hw.offchannel_tx_hw_queue;
 		local->hw_roc_skb = skb;
 		local->hw_roc_skb_for_status = skb;
 		mutex_unlock(&local->mtx);
--- a/net/mac80211/ieee80211_i.h	2012-03-28 10:39:22.000000000 +0200
+++ b/net/mac80211/ieee80211_i.h	2012-03-28 10:40:04.000000000 +0200
@@ -1426,6 +1426,7 @@ void ieee80211_wake_queue_by_reason(stru
 				    enum queue_stop_reason reason);
 void ieee80211_stop_queue_by_reason(struct ieee80211_hw *hw, int queue,
 				    enum queue_stop_reason reason);
+void ieee80211_propagate_queue_wake(struct ieee80211_local *local, int queue);
 void ieee80211_add_pending_skb(struct ieee80211_local *local,
 			       struct sk_buff *skb);
 void ieee80211_add_pending_skbs_fn(struct ieee80211_local *local,
--- a/net/mac80211/main.c	2012-03-28 10:29:08.000000000 +0200
+++ b/net/mac80211/main.c	2012-03-28 10:48:27.000000000 +0200
@@ -594,6 +594,7 @@ struct ieee80211_hw *ieee80211_alloc_hw(
 	local->hw.max_report_rates = 0;
 	local->hw.max_rx_aggregation_subframes = IEEE80211_MAX_AMPDU_BUF;
 	local->hw.max_tx_aggregation_subframes = IEEE80211_MAX_AMPDU_BUF;
+	local->hw.offchannel_tx_hw_queue = IEEE80211_INVAL_HW_QUEUE;
 	local->hw.conf.long_frame_max_tx_count = wiphy->retry_long;
 	local->hw.conf.short_frame_max_tx_count = wiphy->retry_short;
 	local->user_power_level = -1;
@@ -690,6 +691,10 @@ int ieee80211_register_hw(struct ieee802
 		WLAN_CIPHER_SUITE_AES_CMAC
 	};
 
+	if (hw->flags & IEEE80211_HW_QUEUE_CONTROL &&
+	    local->hw.offchannel_tx_hw_queue == IEEE80211_INVAL_HW_QUEUE)
+		return -EINVAL;
+
 	if ((hw->wiphy->wowlan.flags || hw->wiphy->wowlan.n_patterns)
 #ifdef CONFIG_PM
 	    && (!local->ops->suspend || !local->ops->resume)

Re: [RFC 2/2] mac80211: add improved HW queue control

[Johannes Berg]

On Wed, 2012-03-28 at 11:13 +0200, Johannes Berg wrote:
> plain text document attachment (028-mac80211-hw-queue-mgmt.patch)
> From: Johannes Berg <johannes.berg@intel.com>
> 
> 
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Oh my, I forgot to write the commit log, great ... Let's try that
now :-)


mac80211 currently only supports one hardware queue
per AC. This is already problematic for off-channel
uses since if we go off channel while the BE queue 
is full and then try to send an off-channel frame 
the frame will never go out. This will become worse
when we support multi-channel since then a queue on 
one channel might be full, but we have to stop the
software queue for all channels. That is obviously 
not desirable.

To address this problem allow drivers to register
more hardware queues, and allow them to map them to
virtual interfaces. When they stop a hardware queue
the corresponding AC software queues on the correct
interfaces will be stopped as well. Additionally,    
there's an off-channel queue to solve that problem
and a per-interface after-DTIM beacon queue. This
allows drivers to manage software queues closer to
how the hardware works.

Currently, there's a limit of 16 hardware queues.
This may or may not be sufficient, we can adjust it
as needed.



johannes

[RFC v2] mac80211: add improved HW queue control

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

mac80211 currently only supports one hardware queue
per AC. This is already problematic for off-channel
uses since if we go off channel while the BE queue
is full and then try to send an off-channel frame
the frame will never go out. This will become worse
when we support multi-channel since then a queue on
one channel might be full, but we have to stop the
software queue for all channels. That is obviously
not desirable.

To address this problem allow drivers to register
more hardware queues, and allow them to map them to
virtual interfaces. When they stop a hardware queue
the corresponding AC software queues on the correct
interfaces will be stopped as well. Additionally,
there's an off-channel queue to solve that problem
and a per-interface after-DTIM beacon queue. This
allows drivers to manage software queues closer to
how the hardware works.

Currently, there's a limit of 16 hardware queues.
This may or may not be sufficient, we can adjust it
as needed.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
v2: fix aggregation TX queue number

 include/net/mac80211.h     |   81 ++++++++++++++++++++++++++++++++++++++++++---
 net/mac80211/agg-tx.c      |   39 +++++++++++----------
 net/mac80211/cfg.c         |    6 +++
 net/mac80211/ieee80211_i.h |    1 
 net/mac80211/iface.c       |   62 ++++++++++++++++++++++++++++++++++
 net/mac80211/main.c        |    6 +++
 net/mac80211/tx.c          |   38 ++++++++++++++++-----
 net/mac80211/util.c        |   48 +++++++++++++++++++++-----
 8 files changed, 239 insertions(+), 42 deletions(-)

--- a/include/net/mac80211.h	2012-04-02 11:16:23.000000000 +0200
+++ b/include/net/mac80211.h	2012-04-02 11:16:23.000000000 +0200
@@ -93,9 +93,11 @@
  * @IEEE80211_MAX_QUEUES: Maximum number of regular device queues.
  */
 enum ieee80211_max_queues {
-	IEEE80211_MAX_QUEUES =		4,
+	IEEE80211_MAX_QUEUES =		16,
 };
 
+#define IEEE80211_INVAL_HW_QUEUE	0xff
+
 /**
  * enum ieee80211_ac_numbers - AC numbers as used in mac80211
  * @IEEE80211_AC_VO: voice
@@ -520,7 +522,7 @@ struct ieee80211_tx_rate {
  *
  * @flags: transmit info flags, defined above
  * @band: the band to transmit on (use for checking for races)
- * @reserved: reserved for future use
+ * @hw_queue: HW queue to put the frame on, skb_get_queue_mapping() gives the AC
  * @ack_frame_id: internal frame ID for TX status, used internally
  * @control: union for control data
  * @status: union for status data
@@ -536,7 +538,7 @@ struct ieee80211_tx_info {
 	u32 flags;
 	u8 band;
 
-	u8 reserved;
+	u8 hw_queue;
 
 	u16 ack_frame_id;
 
@@ -887,6 +889,8 @@ enum ieee80211_vif_flags {
  *	these need to be set (or cleared) when the interface is added
  *	or, if supported by the driver, the interface type is changed
  *	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
  * @drv_priv: data area for driver use, will always be aligned to
  *	sizeof(void *).
  */
@@ -895,7 +899,12 @@ struct ieee80211_vif {
 	struct ieee80211_bss_conf bss_conf;
 	u8 addr[ETH_ALEN];
 	bool p2p;
+
+	u8 cab_queue;
+	u8 hw_queue[IEEE80211_NUM_ACS];
+
 	u32 driver_flags;
+
 	/* must be last */
 	u8 drv_priv[0] __attribute__((__aligned__(sizeof(void *))));
 };
@@ -1177,6 +1186,11 @@ enum sta_notify_cmd {
  * @IEEE80211_HW_WANT_MONITOR_VIF: The driver would like to be informed of
  *	a virtual monitor interface when monitor interfaces are the only
  *	active interfaces.
+ *
+ * @IEEE80211_HW_QUEUE_CONTROL: The driver wants to control per-interface
+ *	queue mapping in order to use different queues (not just one per AC)
+ *	for different virtual interfaces. See the doc section on HW queue
+ *	control for more details.
  */
 enum ieee80211_hw_flags {
 	IEEE80211_HW_HAS_RATE_CONTROL			= 1<<0,
@@ -1199,7 +1213,7 @@ enum ieee80211_hw_flags {
 	IEEE80211_HW_SUPPORTS_UAPSD			= 1<<17,
 	IEEE80211_HW_REPORTS_TX_ACK_STATUS		= 1<<18,
 	IEEE80211_HW_CONNECTION_MONITOR			= 1<<19,
-	/* reuse bit 20 */
+	IEEE80211_HW_QUEUE_CONTROL			= 1<<20,
 	IEEE80211_HW_SUPPORTS_PER_STA_GTK		= 1<<21,
 	IEEE80211_HW_AP_LINK_PS				= 1<<22,
 	IEEE80211_HW_TX_AMPDU_SETUP_IN_HW		= 1<<23,
@@ -1269,6 +1283,9 @@ enum ieee80211_hw_flags {
  * @max_tx_aggregation_subframes: maximum number of subframes in an
  *	aggregate an HT driver will transmit, used by the peer as a
  *	hint to size its reorder buffer.
+ *
+ * @offchannel_tx_hw_queue: HW queue ID to use for offchannel TX
+ *	(if %IEEE80211_HW_QUEUE_CONTROL is set)
  */
 struct ieee80211_hw {
 	struct ieee80211_conf conf;
@@ -1289,6 +1306,7 @@ struct ieee80211_hw {
 	u8 max_rate_tries;
 	u8 max_rx_aggregation_subframes;
 	u8 max_tx_aggregation_subframes;
+	u8 offchannel_tx_hw_queue;
 };
 
 /**
@@ -1697,6 +1715,61 @@ void ieee80211_free_txskb(struct ieee802
  */
 
 /**
+ * DOC: HW queue control
+ *
+ * Before HW queue control was introduced, mac80211 only had a single static
+ * assignment of per-interface AC software queues to hardware queues. This
+ * was problematic for a few reasons:
+ * 1) off-channel transmissions might get stuck behind other frames
+ * 2) multiple virtual interfaces couldn't be handled correctly
+ * 3) after-DTIM frames could get stuck behind other frames
+ *
+ * To solve this, hardware typically uses multiple different queues for all
+ * the different usages, and this needs to be propagated into mac80211 so it
+ * won't have the same problem with the software queues.
+ *
+ * Therefore, mac80211 now offers the %IEEE80211_HW_QUEUE_CONTROL capability
+ * flag that tells it that the driver implements its own queue control. To do
+ * so, the driver will set up the various queues in each &struct ieee80211_vif
+ * and the offchannel queue in &struct ieee80211_hw. In response, mac80211 will
+ * use those queue IDs in the hw_queue field of &struct ieee80211_tx_info and
+ * if necessary will queue the frame on the right software queue that mirrors
+ * the hardware queue.
+ * Additionally, the driver has to then use these HW queue IDs for the queue
+ * management functions (ieee80211_stop_queue() et al.)
+ *
+ * The driver is free to set up the queue mappings as needed, multiple virtual
+ * interfaces may map to the same hardware queues if needed. The setup has to
+ * happen during add_interface or change_interface callbacks. For example, a
+ * driver supporting station+station and station+AP modes might decide to have
+ * 10 hardware queues to handle different scenarios:
+ *
+ * 4 AC HW queues for 1st vif: 0, 1, 2, 3
+ * 4 AC HW queues for 2nd vif: 4, 5, 6, 7
+ * after-DTIM queue for AP:   8
+ * off-channel queue:         9
+ *
+ * It would then set up the hardware like this:
+ *   hw.offchannel_tx_hw_queue = 9
+ *
+ * and the first virtual interface that is added as follows:
+ *   vif.hw_queue[IEEE80211_AC_VO] = 0
+ *   vif.hw_queue[IEEE80211_AC_VI] = 1
+ *   vif.hw_queue[IEEE80211_AC_BE] = 2
+ *   vif.hw_queue[IEEE80211_AC_BK] = 3
+ *   vif.cab_queue = 8 // if AP mode, otherwise %IEEE80211_INVAL_HW_QUEUE
+ * and the second virtual interface with 4-7.
+ *
+ * If queue 6 gets full, for example, mac80211 would only stop the second
+ * virtual interface's BE queue since virtual interface queues are per AC.
+ *
+ * Note that the vif.cab_queue value should be set to %IEEE80211_INVAL_HW_QUEUE
+ * whenever the queue is not used (i.e. the interface is not in AP mode) if the
+ * queue could potentially be shared since mac80211 will look at cab_queue when
+ * a queue is stopped/woken even if the interface is not in AP mode.
+ */
+
+/**
  * enum ieee80211_filter_flags - hardware filter flags
  *
  * These flags determine what the filter in hardware should be
--- a/net/mac80211/iface.c	2012-04-02 11:16:23.000000000 +0200
+++ b/net/mac80211/iface.c	2012-04-02 11:16:23.000000000 +0200
@@ -149,6 +149,34 @@ static int ieee80211_check_concurrent_if
 	return 0;
 }
 
+static int ieee80211_check_queues(struct ieee80211_sub_if_data *sdata)
+{
+	int n_queues = sdata->local->hw.queues;
+	int i;
+
+	for (i = 0; i < IEEE80211_NUM_ACS; i++) {
+		if (WARN_ON_ONCE(sdata->vif.hw_queue[i] ==
+				 IEEE80211_INVAL_HW_QUEUE))
+			return -EINVAL;
+		if (WARN_ON_ONCE(sdata->vif.hw_queue[i] >=
+				 n_queues))
+			return -EINVAL;
+	}
+
+	if (sdata->vif.type != NL80211_IFTYPE_AP) {
+		sdata->vif.cab_queue = IEEE80211_INVAL_HW_QUEUE;
+		return 0;
+	}
+
+	if (WARN_ON_ONCE(sdata->vif.cab_queue == IEEE80211_INVAL_HW_QUEUE))
+		return -EINVAL;
+
+	if (WARN_ON_ONCE(sdata->vif.cab_queue >= n_queues))
+		return -EINVAL;
+
+	return 0;
+}
+
 void ieee80211_adjust_monitor_flags(struct ieee80211_sub_if_data *sdata,
 				    const int offset)
 {
@@ -169,6 +197,20 @@ void ieee80211_adjust_monitor_flags(stru
 #undef ADJUST
 }
 
+static void ieee80211_set_default_queues(struct ieee80211_sub_if_data *sdata)
+{
+	struct ieee80211_local *local = sdata->local;
+	int i;
+
+	for (i = 0; i < IEEE80211_NUM_ACS; i++) {
+		if (local->hw.flags & IEEE80211_HW_QUEUE_CONTROL)
+			sdata->vif.hw_queue[i] = IEEE80211_INVAL_HW_QUEUE;
+		else
+			sdata->vif.hw_queue[i] = i;
+	}
+	sdata->vif.cab_queue = IEEE80211_INVAL_HW_QUEUE;
+}
+
 static int ieee80211_add_virtual_monitor(struct ieee80211_local *local)
 {
 	struct ieee80211_sub_if_data *sdata;
@@ -190,6 +232,8 @@ static int ieee80211_add_virtual_monitor
 	snprintf(sdata->name, IFNAMSIZ, "%s-monitor",
 		 wiphy_name(local->hw.wiphy));
 
+	ieee80211_set_default_queues(sdata);
+
 	ret = drv_add_interface(local, sdata);
 	if (WARN_ON(ret)) {
 		/* ok .. stupid driver, it asked for this! */
@@ -197,6 +241,12 @@ static int ieee80211_add_virtual_monitor
 		return ret;
 	}
 
+	ret = ieee80211_check_queues(sdata);
+	if (ret) {
+		kfree(sdata);
+		return ret;
+	}
+
 	rcu_assign_pointer(local->monitor_sdata, sdata);
 
 	return 0;
@@ -344,6 +394,9 @@ static int ieee80211_do_open(struct net_
 			res = drv_add_interface(local, sdata);
 			if (res)
 				goto err_stop;
+			res = ieee80211_check_queues(sdata);
+			if (res)
+				goto err_stop;
 		}
 
 		if (sdata->vif.type == NL80211_IFTYPE_AP) {
@@ -1040,6 +1093,13 @@ static int ieee80211_runtime_change_ifty
 	if (ret)
 		type = sdata->vif.type;
 
+	/*
+	 * Ignore return value here, there's not much we can do since
+	 * the driver changed the interface type internally already.
+	 * The warnings will hopefully make driver authors fix it :-)
+	 */
+	ieee80211_check_queues(sdata);
+
 	ieee80211_setup_sdata(sdata, type);
 
 	err = ieee80211_do_open(sdata->dev, false);
@@ -1266,6 +1326,8 @@ int ieee80211_if_add(struct ieee80211_lo
 			       sizeof(sdata->rc_rateidx_mcs_mask[i]));
 	}
 
+	ieee80211_set_default_queues(sdata);
+
 	/* setup type-dependent data */
 	ieee80211_setup_sdata(sdata, type);
 
--- a/net/mac80211/util.c	2012-04-02 11:16:23.000000000 +0200
+++ b/net/mac80211/util.c	2012-04-02 11:16:23.000000000 +0200
@@ -265,11 +265,36 @@ __le16 ieee80211_ctstoself_duration(stru
 }
 EXPORT_SYMBOL(ieee80211_ctstoself_duration);
 
+void ieee80211_propagate_queue_wake(struct ieee80211_local *local, int queue)
+{
+	struct ieee80211_sub_if_data *sdata;
+
+	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
+		int ac;
+
+		if (test_bit(SDATA_STATE_OFFCHANNEL, &sdata->state))
+			continue;
+
+		if (sdata->vif.cab_queue != IEEE80211_INVAL_HW_QUEUE &&
+		    local->queue_stop_reasons[sdata->vif.cab_queue] != 0)
+			continue;
+
+		for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
+			int ac_queue = sdata->vif.hw_queue[ac];
+
+			if (ac_queue == queue ||
+			    (sdata->vif.cab_queue == queue &&
+			     local->queue_stop_reasons[ac_queue] == 0 &&
+			     skb_queue_empty(&local->pending[ac_queue])))
+				netif_wake_subqueue(sdata->dev, ac);
+		}
+	}
+}
+
 static void __ieee80211_wake_queue(struct ieee80211_hw *hw, int queue,
 				   enum queue_stop_reason reason)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
-	struct ieee80211_sub_if_data *sdata;
 
 	trace_wake_queue(local, queue, reason);
 
@@ -287,11 +312,7 @@ static void __ieee80211_wake_queue(struc
 
 	if (skb_queue_empty(&local->pending[queue])) {
 		rcu_read_lock();
-		list_for_each_entry_rcu(sdata, &local->interfaces, list) {
-			if (test_bit(SDATA_STATE_OFFCHANNEL, &sdata->state))
-				continue;
-			netif_wake_subqueue(sdata->dev, queue);
-		}
+		ieee80211_propagate_queue_wake(local, queue);
 		rcu_read_unlock();
 	} else
 		tasklet_schedule(&local->tx_pending_tasklet);
@@ -332,8 +353,15 @@ static void __ieee80211_stop_queue(struc
 	__set_bit(reason, &local->queue_stop_reasons[queue]);
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(sdata, &local->interfaces, list)
-		netif_stop_subqueue(sdata->dev, queue);
+	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
+		int ac;
+
+		for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
+			if (sdata->vif.hw_queue[ac] == queue ||
+			    sdata->vif.cab_queue == queue)
+				netif_stop_subqueue(sdata->dev, ac);
+		}
+	}
 	rcu_read_unlock();
 }
 
@@ -360,8 +388,8 @@ void ieee80211_add_pending_skb(struct ie
 {
 	struct ieee80211_hw *hw = &local->hw;
 	unsigned long flags;
-	int queue = skb_get_queue_mapping(skb);
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+	int queue = info->hw_queue;
 
 	if (WARN_ON(!info->control.vif)) {
 		kfree_skb(skb);
@@ -393,7 +421,7 @@ void ieee80211_add_pending_skbs_fn(struc
 			continue;
 		}
 
-		queue = skb_get_queue_mapping(skb);
+		queue = info->hw_queue;
 
 		__ieee80211_stop_queue(hw, queue,
 				IEEE80211_QUEUE_STOP_REASON_SKB_ADD);
--- a/net/mac80211/tx.c	2012-04-02 11:16:23.000000000 +0200
+++ b/net/mac80211/tx.c	2012-04-02 11:16:23.000000000 +0200
@@ -400,6 +400,8 @@ ieee80211_tx_h_multicast_ps_buf(struct i
 		return TX_CONTINUE;
 
 	info->flags |= IEEE80211_TX_CTL_SEND_AFTER_DTIM;
+	if (tx->local->hw.flags & IEEE80211_HW_QUEUE_CONTROL)
+		info->hw_queue = tx->sdata->vif.cab_queue;
 
 	/* device releases frame after DTIM beacon */
 	if (!(tx->local->hw.flags & IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING))
@@ -1215,11 +1217,19 @@ static bool ieee80211_tx_frags(struct ie
 			       bool txpending)
 {
 	struct sk_buff *skb, *tmp;
-	struct ieee80211_tx_info *info;
 	unsigned long flags;
 
 	skb_queue_walk_safe(skbs, skb, tmp) {
-		int q = skb_get_queue_mapping(skb);
+		struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+		int q = info->hw_queue;
+
+#ifdef CONFIG_MAC80211_VERBOSE_DEBUG
+		if (WARN_ON_ONCE(q >= local->hw.queues)) {
+			__skb_unlink(skb, skbs);
+			dev_kfree_skb(skb);
+			continue;
+		}
+#endif
 
 		spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
 		if (local->queue_stop_reasons[q] ||
@@ -1241,7 +1251,6 @@ static bool ieee80211_tx_frags(struct ie
 		}
 		spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
 
-		info = IEEE80211_SKB_CB(skb);
 		info->control.vif = vif;
 		info->control.sta = sta;
 
@@ -1285,9 +1294,14 @@ static bool __ieee80211_tx(struct ieee80
 	switch (sdata->vif.type) {
 	case NL80211_IFTYPE_MONITOR:
 		sdata = rcu_dereference(local->monitor_sdata);
-		if (sdata)
+		if (sdata) {
 			vif = &sdata->vif;
-		else
+			info->hw_queue =
+				vif->hw_queue[skb_get_queue_mapping(skb)];
+		} else if (local->hw.flags & IEEE80211_HW_QUEUE_CONTROL) {
+			dev_kfree_skb(skb);
+			return true;
+		} else
 			vif = NULL;
 		break;
 	case NL80211_IFTYPE_AP_VLAN:
@@ -1403,6 +1417,12 @@ static bool ieee80211_tx(struct ieee8021
 	tx.channel = local->hw.conf.channel;
 	info->band = tx.channel->band;
 
+	/* set up hw_queue value early */
+	if (!(info->flags & IEEE80211_TX_CTL_TX_OFFCHAN) ||
+	    !(local->hw.flags & IEEE80211_HW_QUEUE_CONTROL))
+		info->hw_queue =
+			sdata->vif.hw_queue[skb_get_queue_mapping(skb)];
+
 	if (!invoke_tx_handlers(&tx))
 		result = __ieee80211_tx(local, &tx.skbs, led_len,
 					tx.sta, txpending);
@@ -2173,7 +2193,6 @@ static bool ieee80211_tx_pending_skb(str
 void ieee80211_tx_pending(unsigned long data)
 {
 	struct ieee80211_local *local = (struct ieee80211_local *)data;
-	struct ieee80211_sub_if_data *sdata;
 	unsigned long flags;
 	int i;
 	bool txok;
@@ -2210,8 +2229,7 @@ void ieee80211_tx_pending(unsigned long
 		}
 
 		if (skb_queue_empty(&local->pending[i]))
-			list_for_each_entry_rcu(sdata, &local->interfaces, list)
-				netif_wake_subqueue(sdata->dev, i);
+			ieee80211_propagate_queue_wake(local, i);
 	}
 	spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
 
@@ -2713,11 +2731,13 @@ EXPORT_SYMBOL(ieee80211_get_buffered_bc)
 void ieee80211_tx_skb_tid(struct ieee80211_sub_if_data *sdata,
 			  struct sk_buff *skb, int tid)
 {
+	int ac = ieee802_1d_to_ac[tid];
+
 	skb_set_mac_header(skb, 0);
 	skb_set_network_header(skb, 0);
 	skb_set_transport_header(skb, 0);
 
-	skb_set_queue_mapping(skb, ieee802_1d_to_ac[tid]);
+	skb_set_queue_mapping(skb, ac);
 	skb->priority = tid;
 
 	/*
--- a/net/mac80211/agg-tx.c	2012-04-02 10:39:34.000000000 +0200
+++ b/net/mac80211/agg-tx.c	2012-04-02 11:16:45.000000000 +0200
@@ -286,25 +286,25 @@ static inline int ieee80211_ac_from_tid(
  * a global "agg_queue_stop" refcount.
  */
 static void __acquires(agg_queue)
-ieee80211_stop_queue_agg(struct ieee80211_local *local, int tid)
+ieee80211_stop_queue_agg(struct ieee80211_sub_if_data *sdata, int tid)
 {
-	int queue = ieee80211_ac_from_tid(tid);
+	int queue = sdata->vif.hw_queue[ieee80211_ac_from_tid(tid)];
 
-	if (atomic_inc_return(&local->agg_queue_stop[queue]) == 1)
+	if (atomic_inc_return(&sdata->local->agg_queue_stop[queue]) == 1)
 		ieee80211_stop_queue_by_reason(
-			&local->hw, queue,
+			&sdata->local->hw, queue,
 			IEEE80211_QUEUE_STOP_REASON_AGGREGATION);
 	__acquire(agg_queue);
 }
 
 static void __releases(agg_queue)
-ieee80211_wake_queue_agg(struct ieee80211_local *local, int tid)
+ieee80211_wake_queue_agg(struct ieee80211_sub_if_data *sdata, int tid)
 {
-	int queue = ieee80211_ac_from_tid(tid);
+	int queue = sdata->vif.hw_queue[ieee80211_ac_from_tid(tid)];
 
-	if (atomic_dec_return(&local->agg_queue_stop[queue]) == 0)
+	if (atomic_dec_return(&sdata->local->agg_queue_stop[queue]) == 0)
 		ieee80211_wake_queue_by_reason(
-			&local->hw, queue,
+			&sdata->local->hw, queue,
 			IEEE80211_QUEUE_STOP_REASON_AGGREGATION);
 	__release(agg_queue);
 }
@@ -314,13 +314,14 @@ ieee80211_wake_queue_agg(struct ieee8021
  * requires a call to ieee80211_agg_splice_finish later
  */
 static void __acquires(agg_queue)
-ieee80211_agg_splice_packets(struct ieee80211_local *local,
+ieee80211_agg_splice_packets(struct ieee80211_sub_if_data *sdata,
 			     struct tid_ampdu_tx *tid_tx, u16 tid)
 {
-	int queue = ieee80211_ac_from_tid(tid);
+	struct ieee80211_local *local = sdata->local;
+	int queue = sdata->vif.hw_queue[ieee80211_ac_from_tid(tid)];
 	unsigned long flags;
 
-	ieee80211_stop_queue_agg(local, tid);
+	ieee80211_stop_queue_agg(sdata, tid);
 
 	if (WARN(!tid_tx, "TID %d gone but expected when splicing aggregates"
 			  " from the pending queue\n", tid))
@@ -336,9 +337,9 @@ ieee80211_agg_splice_packets(struct ieee
 }
 
 static void __releases(agg_queue)
-ieee80211_agg_splice_finish(struct ieee80211_local *local, u16 tid)
+ieee80211_agg_splice_finish(struct ieee80211_sub_if_data *sdata, u16 tid)
 {
-	ieee80211_wake_queue_agg(local, tid);
+	ieee80211_wake_queue_agg(sdata, tid);
 }
 
 void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid)
@@ -376,9 +377,9 @@ void ieee80211_tx_ba_session_handle_star
 					" tid %d\n", tid);
 #endif
 		spin_lock_bh(&sta->lock);
-		ieee80211_agg_splice_packets(local, tid_tx, tid);
+		ieee80211_agg_splice_packets(sdata, tid_tx, tid);
 		ieee80211_assign_tid_tx(sta, tid, NULL);
-		ieee80211_agg_splice_finish(local, tid);
+		ieee80211_agg_splice_finish(sdata, tid);
 		spin_unlock_bh(&sta->lock);
 
 		kfree_rcu(tid_tx, rcu_head);
@@ -586,14 +587,14 @@ static void ieee80211_agg_tx_operational
 	 */
 	spin_lock_bh(&sta->lock);
 
-	ieee80211_agg_splice_packets(local, tid_tx, tid);
+	ieee80211_agg_splice_packets(sta->sdata, tid_tx, tid);
 	/*
 	 * Now mark as operational. This will be visible
 	 * in the TX path, and lets it go lock-free in
 	 * the common case.
 	 */
 	set_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state);
-	ieee80211_agg_splice_finish(local, tid);
+	ieee80211_agg_splice_finish(sta->sdata, tid);
 
 	spin_unlock_bh(&sta->lock);
 }
@@ -778,12 +779,12 @@ void ieee80211_stop_tx_ba_cb(struct ieee
 	 * more.
 	 */
 
-	ieee80211_agg_splice_packets(local, tid_tx, tid);
+	ieee80211_agg_splice_packets(sta->sdata, tid_tx, tid);
 
 	/* future packets must not find the tid_tx struct any more */
 	ieee80211_assign_tid_tx(sta, tid, NULL);
 
-	ieee80211_agg_splice_finish(local, tid);
+	ieee80211_agg_splice_finish(sta->sdata, tid);
 
 	kfree_rcu(tid_tx, rcu_head);
 
--- a/net/mac80211/cfg.c	2012-04-02 11:16:23.000000000 +0200
+++ b/net/mac80211/cfg.c	2012-04-02 11:16:23.000000000 +0200
@@ -2098,6 +2098,10 @@ static int ieee80211_mgmt_tx(struct wiph
 
 	IEEE80211_SKB_CB(skb)->flags = flags;
 
+	if (flags & IEEE80211_TX_CTL_TX_OFFCHAN)
+		IEEE80211_SKB_CB(skb)->hw_queue =
+			local->hw.offchannel_tx_hw_queue;
+
 	skb->dev = sdata->dev;
 
 	*cookie = (unsigned long) skb;
@@ -2139,6 +2143,8 @@ static int ieee80211_mgmt_tx(struct wiph
 		/* modify cookie to prevent API mismatches */
 		*cookie ^= 2;
 		IEEE80211_SKB_CB(skb)->flags |= IEEE80211_TX_CTL_TX_OFFCHAN;
+		IEEE80211_SKB_CB(skb)->hw_queue =
+			local->hw.offchannel_tx_hw_queue;
 		local->hw_roc_skb = skb;
 		local->hw_roc_skb_for_status = skb;
 		mutex_unlock(&local->mtx);
--- a/net/mac80211/ieee80211_i.h	2012-04-02 11:16:23.000000000 +0200
+++ b/net/mac80211/ieee80211_i.h	2012-04-02 11:16:23.000000000 +0200
@@ -1426,6 +1426,7 @@ void ieee80211_wake_queue_by_reason(stru
 				    enum queue_stop_reason reason);
 void ieee80211_stop_queue_by_reason(struct ieee80211_hw *hw, int queue,
 				    enum queue_stop_reason reason);
+void ieee80211_propagate_queue_wake(struct ieee80211_local *local, int queue);
 void ieee80211_add_pending_skb(struct ieee80211_local *local,
 			       struct sk_buff *skb);
 void ieee80211_add_pending_skbs_fn(struct ieee80211_local *local,
--- a/net/mac80211/main.c	2012-04-02 10:39:34.000000000 +0200
+++ b/net/mac80211/main.c	2012-04-02 11:16:23.000000000 +0200
@@ -591,6 +591,7 @@ struct ieee80211_hw *ieee80211_alloc_hw(
 	local->hw.max_report_rates = 0;
 	local->hw.max_rx_aggregation_subframes = IEEE80211_MAX_AMPDU_BUF;
 	local->hw.max_tx_aggregation_subframes = IEEE80211_MAX_AMPDU_BUF;
+	local->hw.offchannel_tx_hw_queue = IEEE80211_INVAL_HW_QUEUE;
 	local->hw.conf.long_frame_max_tx_count = wiphy->retry_long;
 	local->hw.conf.short_frame_max_tx_count = wiphy->retry_short;
 	local->user_power_level = -1;
@@ -687,6 +688,11 @@ int ieee80211_register_hw(struct ieee802
 		WLAN_CIPHER_SUITE_AES_CMAC
 	};
 
+	if (hw->flags & IEEE80211_HW_QUEUE_CONTROL &&
+	    (local->hw.offchannel_tx_hw_queue == IEEE80211_INVAL_HW_QUEUE ||
+	     local->hw.offchannel_tx_hw_queue >= local->hw.queues))
+		return -EINVAL;
+
 	if ((hw->wiphy->wowlan.flags || hw->wiphy->wowlan.n_patterns)
 #ifdef CONFIG_PM
 	    && (!local->ops->suspend || !local->ops->resume)

Re: [RFC 0/2] mac80211 queue redesign

[Johannes Berg]

On Wed, 2012-03-28 at 11:13 +0200, Johannes Berg wrote:

> The monitor feature I've tested in hwsim, it works. The
> queue feature isn't tested at all, I haven't even done
> any kind of regression tests for drivers not using it.

I ran some regression testing now and it seems to work. Haven't tested
the new features yet though.

johannes

Re: [RFC 0/2] mac80211 queue redesign

[Johannes Berg]

On Wed, 2012-03-28 at 14:00 +0200, Johannes Berg wrote:
> On Wed, 2012-03-28 at 11:13 +0200, Johannes Berg wrote:
> 
> > The monitor feature I've tested in hwsim, it works. The
> > queue feature isn't tested at all, I haven't even done
> > any kind of regression tests for drivers not using it.
> 
> I ran some regression testing now and it seems to work. Haven't tested
> the new features yet though.

Woot. I started testing now with a modified iwlwifi and found no bugs so
far :-)

I did add three minor improvements, but those just aid debugging...

johannes