mac80211: unify SW/offload remain-on-channel

Redesign all the off-channel code, getting rid of
the generic off-channel work concept, replacing
it with a simple remain-on-channel list.

This fixes a number of small issues with the ROC
implementation:
 * offloaded remain-on-channel couldn't be queued,
   now we can queue it as well, if needed
 * in iwlwifi (the only user) offloaded ROC is
   mutually exclusive with scanning, use the new
   queue to handle that case -- I expect that it
   will later depend on a HW flag

The bigger issue though is that there's a bad bug
in the current implementation: if we get a mgmt
TX request while HW roc is active, and this new
request has a wait time, we actually schedule a
software ROC instead since we can't guarantee the
existing offloaded ROC will still be that long.
To fix this, the queuing mechanism was needed.

The queuing mechanism for offloaded ROC isn't yet
optimal, ideally we should add API to have the HW
extend the ROC if needed. We could add that later
but for now use a software implementation.

Overall, this unifies the behaviour between the
offloaded and software-implemented case as much
as possible.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>

1 files changed, 259 insertions, 219 deletions

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a169079..498c94e 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2112,35 +2112,171 @@ static int ieee80211_set_bitrate_mask(struct wiphy *wiphy,
 	return 0;
 }
 
-static int ieee80211_remain_on_channel_hw(struct ieee80211_local *local,
-					  struct net_device *dev,
-					  struct ieee80211_channel *chan,
-					  enum nl80211_channel_type chantype,
-					  unsigned int duration, u64 *cookie)
-{
+static int ieee80211_start_roc_work(struct ieee80211_local *local,
+				    struct ieee80211_sub_if_data *sdata,
+				    struct ieee80211_channel *channel,
+				    enum nl80211_channel_type channel_type,
+				    unsigned int duration, u64 *cookie,
+				    struct sk_buff *txskb)
+{
+	struct ieee80211_roc_work *roc, *tmp;
+	bool queued = false;
 	int ret;
-	u32 random_cookie;
 
 	lockdep_assert_held(&local->mtx);
 
-	if (local->hw_roc_cookie)
-		return -EBUSY;
-	/* must be nonzero */
-	random_cookie = random32() | 1;
-
-	*cookie = random_cookie;
-	local->hw_roc_dev = dev;
-	local->hw_roc_cookie = random_cookie;
-	local->hw_roc_channel = chan;
-	local->hw_roc_channel_type = chantype;
-	local->hw_roc_duration = duration;
-	ret = drv_remain_on_channel(local, chan, chantype, duration);
+	roc = kzalloc(sizeof(*roc), GFP_KERNEL);
+	if (!roc)
+		return -ENOMEM;
+
+	roc->chan = channel;
+	roc->chan_type = channel_type;
+	roc->duration = duration;
+	roc->req_duration = duration;
+	roc->frame = txskb;
+	roc->mgmt_tx_cookie = (unsigned long)txskb;
+	roc->sdata = sdata;
+	INIT_DELAYED_WORK(&roc->work, ieee80211_sw_roc_work);
+	INIT_LIST_HEAD(&roc->dependents);
+
+	/* if there's one pending or we're scanning, queue this one */
+	if (!list_empty(&local->roc_list) || local->scanning)
+		goto out_check_combine;
+
+	/* if not HW assist, just queue & schedule work */
+	if (!local->ops->remain_on_channel) {
+		ieee80211_queue_delayed_work(&local->hw, &roc->work, 0);
+		goto out_queue;
+	}
+
+	/* otherwise actually kick it off here (for error handling) */
+
+	/*
+	 * If the duration is zero, then the driver
+	 * wouldn't actually do anything. Set it to
+	 * 10 for now.
+	 *
+	 * TODO: cancel the off-channel operation
+	 *       when we get the SKB's TX status and
+	 *       the wait time was zero before.
+	 */
+	if (!duration)
+		duration = 10;
+
+	ret = drv_remain_on_channel(local, channel, channel_type, duration);
 	if (ret) {
-		local->hw_roc_channel = NULL;
-		local->hw_roc_cookie = 0;
+		kfree(roc);
+		return ret;
 	}
 
-	return ret;
+	roc->started = true;
+	goto out_queue;
+
+ out_check_combine:
+	list_for_each_entry(tmp, &local->roc_list, list) {
+		if (tmp->chan != channel || tmp->chan_type != channel_type)
+			continue;
+
+		/*
+		 * Extend this ROC if possible:
+		 *
+		 * If it hasn't started yet, just increase the duration
+		 * and add the new one to the list of dependents.
+		 */
+		if (!tmp->started) {
+			list_add_tail(&roc->list, &tmp->dependents);
+			tmp->duration = max(tmp->duration, roc->duration);
+			queued = true;
+			break;
+		}
+
+		/* If it has already started, it's more difficult ... */
+		if (local->ops->remain_on_channel) {
+			unsigned long j = jiffies;
+
+			/*
+			 * In the offloaded ROC case, if it hasn't begun, add
+			 * this new one to the dependent list to be handled
+			 * when the the master one begins. If it has begun,
+			 * check that there's still a minimum time left and
+			 * if so, start this one, transmitting the frame, but
+			 * add it to the list directly after this one with a
+			 * a reduced time so we'll ask the driver to execute
+			 * it right after finishing the previous one, in the
+			 * hope that it'll also be executed right afterwards,
+			 * effectively extending the old one.
+			 * If there's no minimum time left, just add it to the
+			 * normal list.
+			 */
+			if (!tmp->hw_begun) {
+				list_add_tail(&roc->list, &tmp->dependents);
+				queued = true;
+				break;
+			}
+
+			if (time_before(j + IEEE80211_ROC_MIN_LEFT,
+					tmp->hw_start_time +
+					msecs_to_jiffies(tmp->duration))) {
+				int new_dur;
+
+				ieee80211_handle_roc_started(roc);
+
+				new_dur = roc->duration -
+					  jiffies_to_msecs(tmp->hw_start_time +
+							   msecs_to_jiffies(
+								tmp->duration) -
+							   j);
+
+				if (new_dur > 0) {
+					/* add right after tmp */
+					list_add(&roc->list, &tmp->list);
+				} else {
+					list_add_tail(&roc->list,
+						      &tmp->dependents);
+				}
+				queued = true;
+			}
+		} else if (del_timer_sync(&tmp->work.timer)) {
+			unsigned long new_end;
+
+			/*
+			 * In the software ROC case, cancel the timer, if
+			 * that fails then the finish work is already
+			 * queued/pending and thus we queue the new ROC
+			 * normally, if that succeeds then we can extend
+			 * the timer duration and TX the frame (if any.)
+			 */
+
+			list_add_tail(&roc->list, &tmp->dependents);
+			queued = true;
+
+			new_end = jiffies + msecs_to_jiffies(roc->duration);
+
+			/* ok, it was started & we canceled timer */
+			if (time_after(new_end, tmp->work.timer.expires))
+				mod_timer(&tmp->work.timer, new_end);
+			else
+				add_timer(&tmp->work.timer);
+
+			ieee80211_handle_roc_started(roc);
+		}
+		break;
+	}
+
+ out_queue:
+	if (!queued)
+		list_add_tail(&roc->list, &local->roc_list);
+
+	/*
+	 * cookie is either the roc (for normal roc)
+	 * or the SKB (for mgmt TX)
+	 */
+	if (txskb)
+		*cookie = (unsigned long)txskb;
+	else
+		*cookie = (unsigned long)roc;
+
+	return 0;
 }
 
 static int ieee80211_remain_on_channel(struct wiphy *wiphy,
@@ -2152,84 +2288,76 @@ static int ieee80211_remain_on_channel(struct wiphy *wiphy,
 {
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	struct ieee80211_local *local = sdata->local;
+	int ret;
 
-	if (local->ops->remain_on_channel) {
-		int ret;
-
-		mutex_lock(&local->mtx);
-		ret = ieee80211_remain_on_channel_hw(local, dev,
-						     chan, channel_type,
-						     duration, cookie);
-		local->hw_roc_for_tx = false;
-		mutex_unlock(&local->mtx);
-
-		return ret;
-	}
+	mutex_lock(&local->mtx);
+	ret = ieee80211_start_roc_work(local, sdata, chan, channel_type,
+				       duration, cookie, NULL);
+	mutex_unlock(&local->mtx);
 
-	return ieee80211_wk_remain_on_channel(sdata, chan, channel_type,
-					      duration, cookie);
+	return ret;
 }
 
-static int ieee80211_cancel_remain_on_channel_hw(struct ieee80211_local *local,
-						 u64 cookie)
+static int ieee80211_cancel_roc(struct ieee80211_local *local,
+				u64 cookie, bool mgmt_tx)
 {
+	struct ieee80211_roc_work *roc, *tmp, *found = NULL;
 	int ret;
 
-	lockdep_assert_held(&local->mtx);
+	mutex_lock(&local->mtx);
+	list_for_each_entry_safe(roc, tmp, &local->roc_list, list) {
+		if (!mgmt_tx && (unsigned long)roc != cookie)
+			continue;
+		else if (mgmt_tx && roc->mgmt_tx_cookie != cookie)
+			continue;
 
-	if (local->hw_roc_cookie != cookie)
-		return -ENOENT;
+		found = roc;
+		break;
+	}
 
-	ret = drv_cancel_remain_on_channel(local);
-	if (ret)
-		return ret;
+	if (!found) {
+		mutex_unlock(&local->mtx);
+		return -ENOENT;
+	}
 
-	local->hw_roc_cookie = 0;
-	local->hw_roc_channel = NULL;
+	if (local->ops->remain_on_channel) {
+		if (found->started) {
+			ret = drv_cancel_remain_on_channel(local);
+			if (WARN_ON_ONCE(ret)) {
+				mutex_unlock(&local->mtx);
+				return ret;
+			}
+		}
 
-	return 0;
-}
+		list_del(&found->list);
 
-static int ieee80211_cancel_remain_on_channel(struct wiphy *wiphy,
-					      struct net_device *dev,
-					      u64 cookie)
-{
-	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
-	struct ieee80211_local *local = sdata->local;
+		ieee80211_run_deferred_scan(local);
+		ieee80211_start_next_roc(local);
+		mutex_unlock(&local->mtx);
 
-	if (local->ops->cancel_remain_on_channel) {
-		int ret;
+		ieee80211_roc_notify_destroy(found);
+	} else {
+		/* work may be pending so use it all the time */
+		found->abort = true;
+		ieee80211_queue_delayed_work(&local->hw, &found->work, 0);
 
-		mutex_lock(&local->mtx);
-		ret = ieee80211_cancel_remain_on_channel_hw(local, cookie);
 		mutex_unlock(&local->mtx);
 
-		return ret;
+		/* work will clean up etc */
+		flush_delayed_work(&found->work);
 	}
 
-	return ieee80211_wk_cancel_remain_on_channel(sdata, cookie);
+	return 0;
 }
 
-static enum work_done_result
-ieee80211_offchan_tx_done(struct ieee80211_work *wk, struct sk_buff *skb)
+static int ieee80211_cancel_remain_on_channel(struct wiphy *wiphy,
+					      struct net_device *dev,
+					      u64 cookie)
 {
-	/*
-	 * Use the data embedded in the work struct for reporting
-	 * here so if the driver mangled the SKB before dropping
-	 * it (which is the only way we really should get here)
-	 * then we don't report mangled data.
-	 *
-	 * If there was no wait time, then by the time we get here
-	 * the driver will likely not have reported the status yet,
-	 * so in that case userspace will have to deal with it.
-	 */
-
-	if (wk->offchan_tx.wait && !wk->offchan_tx.status)
-		cfg80211_mgmt_tx_status(wk->sdata->dev,
-					(unsigned long) wk->offchan_tx.frame,
-					wk->data, wk->data_len, false, GFP_KERNEL);
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	struct ieee80211_local *local = sdata->local;
 
-	return WORK_DONE_DESTROY;
+	return ieee80211_cancel_roc(local, cookie, false);
 }
 
 static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct net_device *dev,
@@ -2243,10 +2371,10 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct net_device *dev,
 	struct ieee80211_local *local = sdata->local;
 	struct sk_buff *skb;
 	struct sta_info *sta;
-	struct ieee80211_work *wk;
 	const struct ieee80211_mgmt *mgmt = (void *)buf;
+	bool need_offchan = false;
 	u32 flags;
-	bool is_offchan = false, in_hw_roc = false;
+	int ret;
 
 	if (dont_wait_for_ack)
 		flags = IEEE80211_TX_CTL_NO_ACK;
@@ -2254,34 +2382,28 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct net_device *dev,
 		flags = IEEE80211_TX_INTFL_NL80211_FRAME_TX |
 			IEEE80211_TX_CTL_REQ_TX_STATUS;
 
-	/* Check that we are on the requested channel for transmission */
-	if (chan != local->tmp_channel &&
-	    chan != local->oper_channel)
-		is_offchan = true;
-	if (channel_type_valid &&
-	    (channel_type != local->tmp_channel_type &&
-	     channel_type != local->_oper_channel_type))
-		is_offchan = true;
-
-	if (chan == local->hw_roc_channel) {
-		/* TODO: check channel type? */
-		is_offchan = false;
-		in_hw_roc = true;
-		flags |= IEEE80211_TX_CTL_TX_OFFCHAN;
-	}
-
 	if (no_cck)
 		flags |= IEEE80211_TX_CTL_NO_CCK_RATE;
 
-	if (is_offchan && !offchan)
-		return -EBUSY;
-
 	switch (sdata->vif.type) {
 	case NL80211_IFTYPE_ADHOC:
+		if (!sdata->vif.bss_conf.ibss_joined)
+			need_offchan = true;
+		/* fall through */
+#ifdef CONFIG_MAC80211_MESH
+	case NL80211_IFTYPE_MESH_POINT:
+		if (ieee80211_vif_is_mesh(&sdata->vif) &&
+		    !sdata->u.mesh.mesh_id_len)
+			need_offchan = true;
+		/* fall through */
+#endif
 	case NL80211_IFTYPE_AP:
 	case NL80211_IFTYPE_AP_VLAN:
 	case NL80211_IFTYPE_P2P_GO:
-	case NL80211_IFTYPE_MESH_POINT:
+		if (sdata->vif.type != NL80211_IFTYPE_ADHOC &&
+		    !ieee80211_vif_is_mesh(&sdata->vif) &&
+		    !rcu_access_pointer(sdata->bss->beacon))
+			need_offchan = true;
 		if (!ieee80211_is_action(mgmt->frame_control) ||
 		    mgmt->u.action.category == WLAN_CATEGORY_PUBLIC)
 			break;
@@ -2293,105 +2415,60 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct net_device *dev,
 		break;
 	case NL80211_IFTYPE_STATION:
 	case NL80211_IFTYPE_P2P_CLIENT:
+		if (!sdata->u.mgd.associated)
+			need_offchan = true;
 		break;
 	default:
 		return -EOPNOTSUPP;
 	}
 
+	mutex_lock(&local->mtx);
+
+	/* Check if the operating channel is the requested channel */
+	if (!need_offchan) {
+		need_offchan = chan != local->oper_channel;
+		if (channel_type_valid &&
+		    channel_type != local->_oper_channel_type)
+			need_offchan = true;
+	}
+
+	if (need_offchan && !offchan) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
 	skb = dev_alloc_skb(local->hw.extra_tx_headroom + len);
-	if (!skb)
-		return -ENOMEM;
+	if (!skb) {
+		ret = -ENOMEM;
+		goto out_unlock;
+	}
 	skb_reserve(skb, local->hw.extra_tx_headroom);
 
 	memcpy(skb_put(skb, len), buf, len);
 
 	IEEE80211_SKB_CB(skb)->flags = flags;
 
-	if (local->hw.flags & IEEE80211_HW_QUEUE_CONTROL &&
-	    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;
-
-	if (is_offchan && local->ops->remain_on_channel) {
-		unsigned int duration;
-		int ret;
-
-		mutex_lock(&local->mtx);
-		/*
-		 * If the duration is zero, then the driver
-		 * wouldn't actually do anything. Set it to
-		 * 100 for now.
-		 *
-		 * TODO: cancel the off-channel operation
-		 *       when we get the SKB's TX status and
-		 *       the wait time was zero before.
-		 */
-		duration = 100;
-		if (wait)
-			duration = wait;
-		ret = ieee80211_remain_on_channel_hw(local, dev, chan,
-						     channel_type,
-						     duration, cookie);
-		if (ret) {
-			kfree_skb(skb);
-			mutex_unlock(&local->mtx);
-			return ret;
-		}
-
-		local->hw_roc_for_tx = true;
-		local->hw_roc_duration = wait;
-
-		/*
-		 * queue up frame for transmission after
-		 * ieee80211_ready_on_channel call
-		 */
-
-		/* modify cookie to prevent API mismatches */
-		*cookie ^= 2;
-		IEEE80211_SKB_CB(skb)->flags |= IEEE80211_TX_CTL_TX_OFFCHAN;
-		if (local->hw.flags & IEEE80211_HW_QUEUE_CONTROL)
-			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);
-
-		return 0;
-	}
-
-	/*
-	 * Can transmit right away if the channel was the
-	 * right one and there's no wait involved... If a
-	 * wait is involved, we might otherwise not be on
-	 * the right channel for long enough!
-	 */
-	if (!is_offchan && !wait && (in_hw_roc || !sdata->vif.bss_conf.idle)) {
+	if (!need_offchan) {
 		ieee80211_tx_skb(sdata, skb);
-		return 0;
-	}
-
-	wk = kzalloc(sizeof(*wk) + len, GFP_KERNEL);
-	if (!wk) {
-		kfree_skb(skb);
-		return -ENOMEM;
+		ret = 0;
+		goto out_unlock;
 	}
 
-	wk->type = IEEE80211_WORK_OFFCHANNEL_TX;
-	wk->chan = chan;
-	wk->chan_type = channel_type;
-	wk->sdata = sdata;
-	wk->done = ieee80211_offchan_tx_done;
-	wk->offchan_tx.frame = skb;
-	wk->offchan_tx.wait = wait;
-	wk->data_len = len;
-	memcpy(wk->data, buf, len);
+	IEEE80211_SKB_CB(skb)->flags |= IEEE80211_TX_CTL_TX_OFFCHAN;
+	if (local->hw.flags & IEEE80211_HW_QUEUE_CONTROL)
+		IEEE80211_SKB_CB(skb)->hw_queue =
+			local->hw.offchannel_tx_hw_queue;
 
-	ieee80211_add_work(wk);
-	return 0;
+	/* This will handle all kinds of coalescing and immediate TX */
+	ret = ieee80211_start_roc_work(local, sdata, chan, channel_type,
+				       wait, cookie, skb);
+	if (ret)
+		kfree_skb(skb);
+ out_unlock:
+	mutex_unlock(&local->mtx);
+	return ret;
 }
 
 static int ieee80211_mgmt_tx_cancel_wait(struct wiphy *wiphy,
@@ -2400,45 +2477,8 @@ static int ieee80211_mgmt_tx_cancel_wait(struct wiphy *wiphy,
 {
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	struct ieee80211_local *local = sdata->local;
-	struct ieee80211_work *wk;
-	int ret = -ENOENT;
 
-	mutex_lock(&local->mtx);
-
-	if (local->ops->cancel_remain_on_channel) {
-		cookie ^= 2;
-		ret = ieee80211_cancel_remain_on_channel_hw(local, cookie);
-
-		if (ret == 0) {
-			kfree_skb(local->hw_roc_skb);
-			local->hw_roc_skb = NULL;
-			local->hw_roc_skb_for_status = NULL;
-		}
-
-		mutex_unlock(&local->mtx);
-
-		return ret;
-	}
-
-	list_for_each_entry(wk, &local->work_list, list) {
-		if (wk->sdata != sdata)
-			continue;
-
-		if (wk->type != IEEE80211_WORK_OFFCHANNEL_TX)
-			continue;
-
-		if (cookie != (unsigned long) wk->offchan_tx.frame)
-			continue;
-
-		wk->timeout = jiffies;
-
-		ieee80211_queue_work(&local->hw, &local->work_work);
-		ret = 0;
-		break;
-	}
-	mutex_unlock(&local->mtx);
-
-	return ret;
+	return ieee80211_cancel_roc(local, cookie, true);
 }
 
 static void ieee80211_mgmt_frame_register(struct wiphy *wiphy,