[PATCH 16/16] mac80211: synchronously reserve TID per station

2014-11-09 Thread Arik Nemtsov
From: Liad Kaufman 

In TDLS (e.g., TDLS off-channel) there is a requirement for
some drivers to supply an unused TID between the AP and the
device to the FW, to allow sending PTI requests and to allow
the FW to aggregate on a specific TID for better throughput.

To ensure that the allocated TID is indeed unused, this patch
introduces an API for blocking the driver from TXing on that
TID.

Signed-off-by: Liad Kaufman 
Reviewed-by: Johannes Berg 
Signed-off-by: Arik Nemtsov 
---
 include/net/mac80211.h | 37 +++
 net/mac80211/agg-tx.c  |  7 
 net/mac80211/ieee80211_i.h |  1 +
 net/mac80211/sta_info.c|  3 ++
 net/mac80211/sta_info.h|  6 +++
 net/mac80211/tx.c  | 91 ++
 net/mac80211/wme.c | 39 
 7 files changed, 184 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 7dfcfe1..4c60cc4 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -5034,6 +5034,43 @@ void ieee80211_tdls_oper_request(struct ieee80211_vif 
*vif, const u8 *peer,
 u16 reason_code, gfp_t gfp);
 
 /**
+ * ieee80211_reserve_tid - request to reserve a specific TID
+ *
+ * There is sometimes a need (such as in TDLS) for blocking the driver from
+ * using a specific TID so that the FW can use it for certain operations such
+ * as sending PTI requests. To make sure that the driver doesn't use that TID,
+ * this function must be called as it flushes out packets on this TID and marks
+ * it as blocked, so that any transmit for the station on this TID will be
+ * redirected to the alternative TID in the same AC.
+ *
+ * Note that this function blocks and may call back into the driver, so it
+ * should be called without driver locks held. Also note this function should
+ * only be called from the driver's @sta_state callback.
+ *
+ * @sta: the station to reserve the TID for
+ * @tid: the TID to reserve
+ *
+ * Returns: 0 on success, else on failure
+ */
+int ieee80211_reserve_tid(struct ieee80211_sta *sta, u8 tid);
+
+/**
+ * ieee80211_unreserve_tid - request to unreserve a specific TID
+ *
+ * Once there is no longer any need for reserving a certain TID, this function
+ * should be called, and no longer will packets have their TID modified for
+ * preventing use of this TID in the driver.
+ *
+ * Note that this function blocks and acquires a lock, so it should be called
+ * without driver locks held. Also note this function should only be called
+ * from the driver's @sta_state callback.
+ *
+ * @sta: the station
+ * @tid: the TID to unreserve
+ */
+void ieee80211_unreserve_tid(struct ieee80211_sta *sta, u8 tid);
+
+/**
  * ieee80211_ie_split - split an IE buffer according to ordering
  *
  * @ies: the IE buffer
diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index 9242c60..a360c15 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -509,6 +509,10 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta 
*pubsta, u16 tid,
struct tid_ampdu_tx *tid_tx;
int ret = 0;
 
+   if (WARN(sta->reserved_tid == tid,
+"Requested to start BA session on reserved tid=%d", tid))
+   return -EINVAL;
+
trace_api_start_tx_ba_session(pubsta, tid);
 
if (WARN_ON_ONCE(!local->ops->ampdu_action))
@@ -765,6 +769,9 @@ int ieee80211_stop_tx_ba_session(struct ieee80211_sta 
*pubsta, u16 tid)
goto unlock;
}
 
+   WARN(sta->reserved_tid == tid,
+"Requested to stop BA session on reserved tid=%d", tid);
+
if (test_bit(HT_AGG_STATE_STOPPING, &tid_tx->state)) {
/* already in progress stopping it */
ret = 0;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index a30d408..34168c2 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1011,6 +1011,7 @@ enum queue_stop_reason {
IEEE80211_QUEUE_STOP_REASON_OFFCHANNEL,
IEEE80211_QUEUE_STOP_REASON_FLUSH,
IEEE80211_QUEUE_STOP_REASON_TDLS_TEARDOWN,
+   IEEE80211_QUEUE_STOP_REASON_RESERVE_TID,
 
IEEE80211_QUEUE_STOP_REASONS,
 };
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 86ca627..a42f5b2 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -351,6 +351,9 @@ struct sta_info *sta_info_alloc(struct 
ieee80211_sub_if_data *sdata,
 
sta->sta_state = IEEE80211_STA_NONE;
 
+   /* Mark TID as unreserved */
+   sta->reserved_tid = IEEE80211_TID_UNRESERVED;
+
ktime_get_ts(&uptime);
sta->last_connected = uptime.tv_sec;
ewma_init(&sta->avg_signal, 1024, 8);
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 00f56eb..4f052bb 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -254,6 +254,9 @@ struct ieee80211_tx_latency_stat {
u32 bin_count;
 };
 
+/* Value to indicate no TID reservation */
+#define IEE

Re: [PATCH 16/16] mac80211: synchronously reserve TID per station

2014-11-19 Thread Johannes Berg
On Sun, 2014-11-09 at 18:50 +0200, Arik Nemtsov wrote:

> + if (WARN_ON(tid >= IEEE80211_NUM_TIDS))
> + return -EINVAL;

That validates < 16

> + queues = BIT(sdata->vif.hw_queue[ieee802_1d_to_ac[tid]]);

but that's only valid for < 8, causing a smatch warning.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 16/16] mac80211: synchronously reserve TID per station

2014-11-19 Thread Arik Nemtsov
On Wed, Nov 19, 2014 at 1:22 PM, Johannes Berg
 wrote:
> On Sun, 2014-11-09 at 18:50 +0200, Arik Nemtsov wrote:
>
>> + if (WARN_ON(tid >= IEEE80211_NUM_TIDS))
>> + return -EINVAL;
>
> That validates < 16
>
>> + queues = BIT(sdata->vif.hw_queue[ieee802_1d_to_ac[tid]]);
>
> but that's only valid for < 8, causing a smatch warning.

It's a valid warning. It should be tid & 7 here. I'll send a fix.

Arik
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 16/16] mac80211: synchronously reserve TID per station

2014-11-19 Thread Johannes Berg
On Wed, 2014-11-19 at 13:40 +0200, Arik Nemtsov wrote:
> On Wed, Nov 19, 2014 at 1:22 PM, Johannes Berg
>  wrote:
> > On Sun, 2014-11-09 at 18:50 +0200, Arik Nemtsov wrote:
> >
> >> + if (WARN_ON(tid >= IEEE80211_NUM_TIDS))
> >> + return -EINVAL;
> >
> > That validates < 16
> >
> >> + queues = BIT(sdata->vif.hw_queue[ieee802_1d_to_ac[tid]]);
> >
> > but that's only valid for < 8, causing a smatch warning.
> 
> It's a valid warning. It should be tid & 7 here. I'll send a fix.

yes, but I think &7 is wrong too - TIDs (really TSIDs) 8-15 don't have a
static AC mapping, and we don't support them anyway. I think the sanity
check should just make sure it's < 8.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 16/16] mac80211: synchronously reserve TID per station

2014-11-19 Thread Arik Nemtsov
On Wed, Nov 19, 2014 at 1:41 PM, Johannes Berg
 wrote:
> On Wed, 2014-11-19 at 13:40 +0200, Arik Nemtsov wrote:
>> On Wed, Nov 19, 2014 at 1:22 PM, Johannes Berg
>>  wrote:
>> > On Sun, 2014-11-09 at 18:50 +0200, Arik Nemtsov wrote:
>> >
>> >> + if (WARN_ON(tid >= IEEE80211_NUM_TIDS))
>> >> + return -EINVAL;
>> >
>> > That validates < 16
>> >
>> >> + queues = BIT(sdata->vif.hw_queue[ieee802_1d_to_ac[tid]]);
>> >
>> > but that's only valid for < 8, causing a smatch warning.
>>
>> It's a valid warning. It should be tid & 7 here. I'll send a fix.
>
> yes, but I think &7 is wrong too - TIDs (really TSIDs) 8-15 don't have a
> static AC mapping, and we don't support them anyway. I think the sanity
> check should just make sure it's < 8.

yea you're right. was just writing this :)

Arik
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html