[PATCH 1/2] mac80211: add flag to indicate HW only Tx-agg support

[PATCH 1/2] mac80211: add flag to indicate HW only Tx-agg support

[Arik Nemtsov]

When this flag is set, Tx aggregations will not be initiated even
if the hardware supports A-MPDU aggregation in general. This flag
is required for devices supporting (partial) software Rx-aggregation.

Signed-off-by: Arik Nemtsov <arik@wizery.com>
---
 include/net/mac80211.h |    4 ++++
 net/mac80211/agg-tx.c  |    3 ++-
 net/mac80211/debugfs.c |    2 ++
 net/mac80211/tx.c      |    3 ++-
 4 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 2f01d84..b21a5bb 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1094,6 +1094,9 @@ enum sta_notify_cmd {
  *	stations based on the PM bit of incoming frames.
  *	Use ieee80211_start_ps()/ieee8021_end_ps() to manually configure
  *	the PS mode of connected stations.
+ *
+ * @IEEE80211_HW_TX_AMPDU_IN_HW_ONLY: The device handles TX aggregation
+ *	strictly in HW. Packets should not be aggregated in software.
  */
 enum ieee80211_hw_flags {
 	IEEE80211_HW_HAS_RATE_CONTROL			= 1<<0,
@@ -1119,6 +1122,7 @@ enum ieee80211_hw_flags {
 	IEEE80211_HW_SUPPORTS_CQM_RSSI			= 1<<20,
 	IEEE80211_HW_SUPPORTS_PER_STA_GTK		= 1<<21,
 	IEEE80211_HW_AP_LINK_PS				= 1<<22,
+	IEEE80211_HW_TX_AMPDU_IN_HW_ONLY		= 1<<23,
 };
 
 /**
diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index b7075f3..be59aef 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -364,7 +364,8 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid,
 		return -EINVAL;
 
 	if ((tid >= STA_TID_NUM) ||
-	    !(local->hw.flags & IEEE80211_HW_AMPDU_AGGREGATION))
+	    !(local->hw.flags & IEEE80211_HW_AMPDU_AGGREGATION) ||
+	    (local->hw.flags & IEEE80211_HW_TX_AMPDU_IN_HW_ONLY))
 		return -EINVAL;
 
 #ifdef CONFIG_MAC80211_HT_DEBUG
diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index 186e02f..6f1515d 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -355,6 +355,8 @@ static ssize_t hwflags_read(struct file *file, char __user *user_buf,
 		sf += snprintf(buf + sf, mxln - sf, "SUPPORTS_PER_STA_GTK\n");
 	if (local->hw.flags & IEEE80211_HW_AP_LINK_PS)
 		sf += snprintf(buf + sf, mxln - sf, "AP_LINK_PS\n");
+	if (local->hw.flags & IEEE80211_HW_TX_AMPDU_IN_HW_ONLY)
+		sf += snprintf(buf + sf, mxln - sf, "HW_TX_AGGREGATION\n");
 
 	rv = simple_read_from_buffer(user_buf, count, ppos, buf, strlen(buf));
 	kfree(buf);
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 69fd494..a0af2f3 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1232,7 +1232,8 @@ ieee80211_tx_prepare(struct ieee80211_sub_if_data *sdata,
 		tx->sta = sta_info_get(sdata, hdr->addr1);
 
 	if (tx->sta && ieee80211_is_data_qos(hdr->frame_control) &&
-	    (local->hw.flags & IEEE80211_HW_AMPDU_AGGREGATION)) {
+	    (local->hw.flags & IEEE80211_HW_AMPDU_AGGREGATION) &&
+	    !(local->hw.flags & IEEE80211_HW_TX_AMPDU_IN_HW_ONLY)) {
 		struct tid_ampdu_tx *tid_tx;
 
 		qc = ieee80211_get_qos_ctl(hdr);
-- 
1.7.4.1

[PATCH 2/2] wl12xx: set mac80211 flags for aggregation support

[Arik Nemtsov]

We set the flag for A-MPDU support and also indicate that Tx aggregation
is performed in HW. This will prevent mac80211 from aggregating Tx
packets in software.

This patch depends on
"mac80211: add flag to indicate HW only Tx-agg support"

Signed-off-by: Arik Nemtsov <arik@wizery.com>
---
 drivers/net/wireless/wl12xx/main.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 7027a7e..fdeea4d 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -4478,7 +4478,9 @@ int wl1271_init_ieee80211(struct wl1271 *wl)
 		IEEE80211_HW_SUPPORTS_CQM_RSSI |
 		IEEE80211_HW_REPORTS_TX_ACK_STATUS |
 		IEEE80211_HW_SPECTRUM_MGMT |
-		IEEE80211_HW_AP_LINK_PS;
+		IEEE80211_HW_AP_LINK_PS |
+		IEEE80211_HW_AMPDU_AGGREGATION |
+		IEEE80211_HW_TX_AMPDU_IN_HW_ONLY;
 
 	wl->hw->wiphy->cipher_suites = cipher_suites;
 	wl->hw->wiphy->n_cipher_suites = ARRAY_SIZE(cipher_suites);
-- 
1.7.4.1

Re: [PATCH 2/2] wl12xx: set mac80211 flags for aggregation support

[Luciano Coelho]

On Thu, 2011-08-25 at 12:51 +0300, Arik Nemtsov wrote: 
> We set the flag for A-MPDU support and also indicate that Tx aggregation
> is performed in HW. This will prevent mac80211 from aggregating Tx
> packets in software.
> 
> This patch depends on
> "mac80211: add flag to indicate HW only Tx-agg support"

Please send patches with this kind of dependency separately.  One for
mac80211 (which John will apply) and one for wl12xx, which I will apply
once the dependency is solved.

This avoids confusion as to who should apply what.

-- 
Cheers,
Luca.

Re: [PATCH 1/2] mac80211: add flag to indicate HW only Tx-agg support

[Helmut Schaa]

Hi,

On Thu, Aug 25, 2011 at 11:51 AM, Arik Nemtsov <arik@wizery.com> wrote:
> When this flag is set, Tx aggregations will not be initiated even
> if the hardware supports A-MPDU aggregation in general. This flag
> is required for devices supporting (partial) software Rx-aggregation.

Is the hardware unable to do TX aggregation?
Or is it just not implemented?
Or is it entirely done in hw?

Instead of cluttering mac80211 with hw flags you could also just disallow tx
aggregation sessions in the ampdu_action callback in the driver.

Helmut

Re: [PATCH 1/2] mac80211: add flag to indicate HW only Tx-agg support

[Arik Nemtsov]

On Thu, Aug 25, 2011 at 15:18, Helmut Schaa <helmut.schaa@googlemail.com> wrote:
> Hi,
>
> On Thu, Aug 25, 2011 at 11:51 AM, Arik Nemtsov <arik@wizery.com> wrote:
>> When this flag is set, Tx aggregations will not be initiated even
>> if the hardware supports A-MPDU aggregation in general. This flag
>> is required for devices supporting (partial) software Rx-aggregation.
>
> Is the hardware unable to do TX aggregation?
> Or is it just not implemented?
> Or is it entirely done in hw?

Tx aggregations are done entirely in HW.

>
> Instead of cluttering mac80211 with hw flags you could also just disallow tx
> aggregation sessions in the ampdu_action callback in the driver.

Well the driver is doing some pretty heavy lifting before even calling
the ampdu_action callback. Namely, ieee80211_start_tx_ba_session()
doesn't call it. AFAICT, the return from the callback is only used as
an indication that Tx BA session are temporary unavailable, and
mac80211 should try again. This is not the case here.

Regards,
Arik

Re: [PATCH 1/2] mac80211: add flag to indicate HW only Tx-agg support

[Helmut Schaa]

On Thu, Aug 25, 2011 at 2:38 PM, Arik Nemtsov <arik@wizery.com> wrote:
> Tx aggregations are done entirely in HW.

Then your patch makes sense :)

Might be worth to note that in the commit message though.

Thanks,
Helmut

Re: [PATCH 1/2] mac80211: add flag to indicate HW only Tx-agg support

[Arik Nemtsov]

On Thu, Aug 25, 2011 at 15:54, Helmut Schaa <helmut.schaa@googlemail.com> wrote:
> On Thu, Aug 25, 2011 at 2:38 PM, Arik Nemtsov <arik@wizery.com> wrote:
>> Tx aggregations are done entirely in HW.
>
> Then your patch makes sense :)

Thanks.

>
> Might be worth to note that in the commit message though.

I'm waiting for more comments for now, but I agree the commit message
should be improved.

Arik

Re: [PATCH 1/2] mac80211: add flag to indicate HW only Tx-agg support

[Johannes Berg]

On Thu, 2011-08-25 at 12:51 +0300, Arik Nemtsov wrote:
> When this flag is set, Tx aggregations will not be initiated even
> if the hardware supports A-MPDU aggregation in general. This flag
> is required for devices supporting (partial) software Rx-aggregation.
> 
> Signed-off-by: Arik Nemtsov <arik@wizery.com>
> ---
>  include/net/mac80211.h |    4 ++++
>  net/mac80211/agg-tx.c  |    3 ++-
>  net/mac80211/debugfs.c |    2 ++
>  net/mac80211/tx.c      |    3 ++-
>  4 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 2f01d84..b21a5bb 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -1094,6 +1094,9 @@ enum sta_notify_cmd {
>   *	stations based on the PM bit of incoming frames.
>   *	Use ieee80211_start_ps()/ieee8021_end_ps() to manually configure
>   *	the PS mode of connected stations.
> + *
> + * @IEEE80211_HW_TX_AMPDU_IN_HW_ONLY: The device handles TX aggregation
> + *	strictly in HW. Packets should not be aggregated in software.

In addition to the discussion with Helmut -- this is also misleading
since mac80211 doesn't, right now, aggregate anything in software -- it
just handles session setup (which is what you don't want).

johannes

Re: [PATCH 1/2] mac80211: add flag to indicate HW only Tx-agg support

[Arik Nemtsov]

On Mon, Aug 29, 2011 at 09:21, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Thu, 2011-08-25 at 12:51 +0300, Arik Nemtsov wrote:
>> When this flag is set, Tx aggregations will not be initiated even
>> if the hardware supports A-MPDU aggregation in general. This flag
>> is required for devices supporting (partial) software Rx-aggregation.
>>
>> Signed-off-by: Arik Nemtsov <arik@wizery.com>
>> ---
>>  include/net/mac80211.h |    4 ++++
>>  net/mac80211/agg-tx.c  |    3 ++-
>>  net/mac80211/debugfs.c |    2 ++
>>  net/mac80211/tx.c      |    3 ++-
>>  4 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
>> index 2f01d84..b21a5bb 100644
>> --- a/include/net/mac80211.h
>> +++ b/include/net/mac80211.h
>> @@ -1094,6 +1094,9 @@ enum sta_notify_cmd {
>>   *   stations based on the PM bit of incoming frames.
>>   *   Use ieee80211_start_ps()/ieee8021_end_ps() to manually configure
>>   *   the PS mode of connected stations.
>> + *
>> + * @IEEE80211_HW_TX_AMPDU_IN_HW_ONLY: The device handles TX aggregation
>> + *   strictly in HW. Packets should not be aggregated in software.
>
> In addition to the discussion with Helmut -- this is also misleading
> since mac80211 doesn't, right now, aggregate anything in software -- it
> just handles session setup (which is what you don't want).

Sure. I'll rename it to something like
IEEE80211_HW_TX_AMPDU_SETUP_IN_HW (and change the description).

Arik