[RFC 1/2] mac80211: Stop BA session event from device

[RFC 1/2] mac80211: Stop BA session event from device

[Shahar Levi]

The wl12xx device support BT/WLAN co-existence algorithm.
In order not to harm the system performance and user experience, the device may
request not to allow any RX BA session and tear down existing RX BA sessions
based on system constraints such as: For periodic BT activity limiting WLAN
activity - For example: SCO / A2DP.
In such cases, the intention is to limit the duration of the RX PPDU and
therefore prevent the peer device to use A-MPDU aggregation.

Adding new EXPORT_SYMBOL ieee80211_stop_rx_ba_session() in order
to support stop BA session event from device.

Signed-off-by: Shahar Levi <shahar_levi@ti.com>
---
 include/net/mac80211.h |    2 ++
 net/mac80211/agg-rx.c  |   10 ++++++++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 5b0fff2..e6ab26d 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2833,4 +2833,6 @@ ieee80211_vif_type_p2p(struct ieee80211_vif *vif)
 	return ieee80211_iftype_p2p(vif->type, vif->p2p);
 }
 
+void ieee80211_stop_rx_ba_session(struct ieee80211_vif *vif, u16 tid,
+				  const u8 *addr);
 #endif /* MAC80211_H */
diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c
index 720b7a8..09f7cd9 100644
--- a/net/mac80211/agg-rx.c
+++ b/net/mac80211/agg-rx.c
@@ -99,6 +99,16 @@ void __ieee80211_stop_rx_ba_session(struct sta_info *sta, u16 tid,
 	mutex_unlock(&sta->ampdu_mlme.mtx);
 }
 
+void ieee80211_stop_rx_ba_session(struct ieee80211_vif *vif, u16 tid,
+				  const u8 *addr)
+{
+	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+	struct sta_info *sta = sta_info_get(sdata, addr);
+
+	__ieee80211_stop_rx_ba_session(sta, tid, WLAN_BACK_INITIATOR, 0, true);
+}
+EXPORT_SYMBOL(ieee80211_stop_rx_ba_session);
+
 /*
  * After accepting the AddBA Request we activated a timer,
  * resetting it after each frame that arrives from the originator.
-- 
1.7.0.4

[RFC 2/2] wl12xx: Stop BA session event from device

[Shahar Levi]

Adding new event that close RX BA session in case of periodic BT activity
limiting WLAN activity.

Signed-off-by: Shahar Levi <shahar_levi@ti.com>
---
Dependencies:
- BA Initiator patches have a runtime dependency on commit
   "[PATCH v3] wl12xx: BA Initiator support" and
   "[PATCH ] wl12xx: BA receiver support"

 drivers/net/wireless/wl12xx/boot.c  |    3 ++-
 drivers/net/wireless/wl12xx/event.c |   24 +++++++++++++++++++++++-
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/boot.c b/drivers/net/wireless/wl12xx/boot.c
index 1eafb81..1270f5d 100644
--- a/drivers/net/wireless/wl12xx/boot.c
+++ b/drivers/net/wireless/wl12xx/boot.c
@@ -430,7 +430,8 @@ static int wl1271_boot_run_firmware(struct wl1271 *wl)
 		DISCONNECT_EVENT_COMPLETE_ID |
 		RSSI_SNR_TRIGGER_0_EVENT_ID |
 		PSPOLL_DELIVERY_FAILURE_EVENT_ID |
-		SOFT_GEMINI_SENSE_EVENT_ID;
+		SOFT_GEMINI_SENSE_EVENT_ID |
+		BA_SESSION_RX_CONSTRAINT_EVENT_ID;
 
 	ret = wl1271_event_unmask(wl);
 	if (ret < 0) {
diff --git a/drivers/net/wireless/wl12xx/event.c b/drivers/net/wireless/wl12xx/event.c
index 9815b7b..7cbeb2b 100644
--- a/drivers/net/wireless/wl12xx/event.c
+++ b/drivers/net/wireless/wl12xx/event.c
@@ -174,6 +174,25 @@ static void wl1271_event_rssi_trigger(struct wl1271 *wl,
 	wl->last_rssi_event = event;
 }
 
+static void wl1271_event_ba_rx_constraint(struct wl1271 *wl,
+					  struct event_mailbox *mbox)
+{
+	u8 tid_index = 0;
+
+	wl1271_debug(DEBUG_EVENT, "BA RX constraint event. ba_allowed = %d",
+		     mbox->ba_allowed);
+
+	wl->ba_allowed = mbox->ba_allowed;
+
+	if (!wl->ba_rx_bitmap)
+		return;
+
+	for (tid_index = 0; tid_index < CONF_TX_MAX_TID_COUNT; ++tid_index)
+		if (wl->ba_rx_bitmap & (BIT(0) << tid_index))
+			ieee80211_stop_rx_ba_session(wl->vif, tid_index,
+						     wl->bssid);
+}
+
 static void wl1271_event_mbox_dump(struct event_mailbox *mbox)
 {
 	wl1271_debug(DEBUG_EVENT, "MBOX DUMP:");
@@ -241,8 +260,11 @@ static int wl1271_event_process(struct wl1271 *wl, struct event_mailbox *mbox)
 			wl1271_event_rssi_trigger(wl, mbox);
 	}
 
-	if (vector & BA_SESSION_RX_CONSTRAINT_EVENT_ID)
+	if (vector & BA_SESSION_RX_CONSTRAINT_EVENT_ID) {
 		wl1271_debug(DEBUG_EVENT, "BA_SESSION_RX_CONSTRAINT_EVENT_ID");
+		if (wl->vif)
+			wl1271_event_ba_rx_constraint(wl, mbox);
+	}
 
 	if (wl->vif && beacon_loss)
 		ieee80211_connection_loss(wl->vif);
-- 
1.7.0.4

Re: [RFC 2/2] wl12xx: Stop BA session event from device

[Luciano Coelho]

On Thu, 2010-11-25 at 16:39 +0200, ext Shahar Levi wrote:
> Adding new event that close RX BA session in case of periodic BT activity
> limiting WLAN activity.
> 
> Signed-off-by: Shahar Levi <shahar_levi@ti.com>
> ---

[...]

> diff --git a/drivers/net/wireless/wl12xx/event.c b/drivers/net/wireless/wl12xx/event.c
> index 9815b7b..7cbeb2b 100644
> --- a/drivers/net/wireless/wl12xx/event.c
> +++ b/drivers/net/wireless/wl12xx/event.c
> @@ -174,6 +174,25 @@ static void wl1271_event_rssi_trigger(struct wl1271 *wl,
>  	wl->last_rssi_event = event;
>  }
>  
> +static void wl1271_event_ba_rx_constraint(struct wl1271 *wl,
> +					  struct event_mailbox *mbox)
> +{
> +	u8 tid_index = 0;
> +
> +	wl1271_debug(DEBUG_EVENT, "BA RX constraint event. ba_allowed = %d",
> +		     mbox->ba_allowed);
> +
> +	wl->ba_allowed = mbox->ba_allowed;
> +
> +	if (!wl->ba_rx_bitmap)
> +		return;
> +
> +	for (tid_index = 0; tid_index < CONF_TX_MAX_TID_COUNT; ++tid_index)

Please use the more classic tid_index++ instead.


-- 
Cheers,
Luca.

Re: [RFC 2/2] wl12xx: Stop BA session event from device

[Shahar Levi]

On 11/29/2010 02:53 PM, Luciano Coelho wrote:
> On Thu, 2010-11-25 at 16:39 +0200, ext Shahar Levi wrote:
>> Adding new event that close RX BA session in case of periodic BT activity
>> limiting WLAN activity.
>>
>> Signed-off-by: Shahar Levi<shahar_levi@ti.com>
>> ---
>
> [...]
>
>> diff --git a/drivers/net/wireless/wl12xx/event.c b/drivers/net/wireless/wl12xx/event.c
>> index 9815b7b..7cbeb2b 100644
>> --- a/drivers/net/wireless/wl12xx/event.c
>> +++ b/drivers/net/wireless/wl12xx/event.c
>> @@ -174,6 +174,25 @@ static void wl1271_event_rssi_trigger(struct wl1271 *wl,
>>   	wl->last_rssi_event = event;
>>   }
>>
>> +static void wl1271_event_ba_rx_constraint(struct wl1271 *wl,
>> +					  struct event_mailbox *mbox)
>> +{
>> +	u8 tid_index = 0;
>> +
>> +	wl1271_debug(DEBUG_EVENT, "BA RX constraint event. ba_allowed = %d",
>> +		     mbox->ba_allowed);
>> +
>> +	wl->ba_allowed = mbox->ba_allowed;
>> +
>> +	if (!wl->ba_rx_bitmap)
>> +		return;
>> +
>> +	for (tid_index = 0; tid_index<  CONF_TX_MAX_TID_COUNT; ++tid_index)
>
> Please use the more classic tid_index++ instead.
OK, will be fix on v2.
Thanks

Re: [RFC 2/2] wl12xx: Stop BA session event from device

[Luciano Coelho]

On Mon, 2010-11-29 at 14:55 +0200, ext Shahar Levi wrote:
> On 11/29/2010 02:53 PM, Luciano Coelho wrote:
> > On Thu, 2010-11-25 at 16:39 +0200, ext Shahar Levi wrote:
> >> Adding new event that close RX BA session in case of periodic BT activity
> >> limiting WLAN activity.
> >>
> >> Signed-off-by: Shahar Levi<shahar_levi@ti.com>
> >> ---
> >
> > [...]
> >
> >> diff --git a/drivers/net/wireless/wl12xx/event.c b/drivers/net/wireless/wl12xx/event.c
> >> index 9815b7b..7cbeb2b 100644
> >> --- a/drivers/net/wireless/wl12xx/event.c
> >> +++ b/drivers/net/wireless/wl12xx/event.c
> >> @@ -174,6 +174,25 @@ static void wl1271_event_rssi_trigger(struct wl1271 *wl,
> >>   	wl->last_rssi_event = event;
> >>   }
> >>
> >> +static void wl1271_event_ba_rx_constraint(struct wl1271 *wl,
> >> +					  struct event_mailbox *mbox)
> >> +{
> >> +	u8 tid_index = 0;
> >> +
> >> +	wl1271_debug(DEBUG_EVENT, "BA RX constraint event. ba_allowed = %d",
> >> +		     mbox->ba_allowed);
> >> +
> >> +	wl->ba_allowed = mbox->ba_allowed;
> >> +
> >> +	if (!wl->ba_rx_bitmap)
> >> +		return;
> >> +
> >> +	for (tid_index = 0; tid_index<  CONF_TX_MAX_TID_COUNT; ++tid_index)
> >
> > Please use the more classic tid_index++ instead.
> OK, will be fix on v2.

This is really nitpicking now, but if you change tid_index to i instead,
you won't need two lines for the call to ieee80211_stop_rx_ba_session(),
because it will fit in one line. :P

-- 
Cheers,
Luca.

Re: [RFC 2/2] wl12xx: Stop BA session event from device

[Shahar Levi]

On 11/29/2010 02:58 PM, Luciano Coelho wrote:
> On Mon, 2010-11-29 at 14:55 +0200, ext Shahar Levi wrote:
>> On 11/29/2010 02:53 PM, Luciano Coelho wrote:
>>> On Thu, 2010-11-25 at 16:39 +0200, ext Shahar Levi wrote:
>>>> Adding new event that close RX BA session in case of periodic BT activity
>>>> limiting WLAN activity.
>>>>
>>>> Signed-off-by: Shahar Levi<shahar_levi@ti.com>
>>>> ---
>>>
>>> [...]
>>>
>>>> diff --git a/drivers/net/wireless/wl12xx/event.c b/drivers/net/wireless/wl12xx/event.c
>>>> index 9815b7b..7cbeb2b 100644
>>>> --- a/drivers/net/wireless/wl12xx/event.c
>>>> +++ b/drivers/net/wireless/wl12xx/event.c
>>>> @@ -174,6 +174,25 @@ static void wl1271_event_rssi_trigger(struct wl1271 *wl,
>>>>    	wl->last_rssi_event = event;
>>>>    }
>>>>
>>>> +static void wl1271_event_ba_rx_constraint(struct wl1271 *wl,
>>>> +					  struct event_mailbox *mbox)
>>>> +{
>>>> +	u8 tid_index = 0;
>>>> +
>>>> +	wl1271_debug(DEBUG_EVENT, "BA RX constraint event. ba_allowed = %d",
>>>> +		     mbox->ba_allowed);
>>>> +
>>>> +	wl->ba_allowed = mbox->ba_allowed;
>>>> +
>>>> +	if (!wl->ba_rx_bitmap)
>>>> +		return;
>>>> +
>>>> +	for (tid_index = 0; tid_index<   CONF_TX_MAX_TID_COUNT; ++tid_index)
>>>
>>> Please use the more classic tid_index++ instead.
>> OK, will be fix on v2.
>
> This is really nitpicking now, but if you change tid_index to i instead,
> you won't need two lines for the call to ieee80211_stop_rx_ba_session(),
> because it will fit in one line. :P
>
;-)
no problem. will do.

Re: [RFC 1/2] mac80211: Stop BA session event from device

[Johannes Berg]

On Thu, 2010-11-25 at 16:39 +0200, Shahar Levi wrote:

> +void ieee80211_stop_rx_ba_session(struct ieee80211_vif *vif, u16 tid,
> +				  const u8 *addr);

missing docs

> +void ieee80211_stop_rx_ba_session(struct ieee80211_vif *vif, u16 tid,
> +				  const u8 *addr)
> +{
> +	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
> +	struct sta_info *sta = sta_info_get(sdata, addr);
> +
> +	__ieee80211_stop_rx_ba_session(sta, tid, WLAN_BACK_INITIATOR, 0, true);
> +}
> +EXPORT_SYMBOL(ieee80211_stop_rx_ba_session);

Locking-wise, this is a disaster, as the function will acquire a mutex
that it also holds across other calls into the driver. It would seem
that this should have caused lockdep to warn you about ABBA style
deadlock since you call this with wl->mutex held, and previously you
said you were going to acquire that mutex from ampdu_action.

This should use a similar trick to the one used in TX aggregation.

johannes

RE: [RFC 1/2] mac80211: Stop BA session event from device

[Levi, Shahar]

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBKb2hhbm5lcyBCZXJnIFttYWls
dG86am9oYW5uZXNAc2lwc29sdXRpb25zLm5ldF0NCj4gU2VudDogVGh1cnNkYXksIE5vdmVtYmVy
IDI1LCAyMDEwIDg6MTEgUE0NCj4gVG86IExldmksIFNoYWhhcg0KPiBDYzogbGludXgtd2lyZWxl
c3NAdmdlci5rZXJuZWwub3JnOyBMdWNpYW5vIENvZWxobw0KPiBTdWJqZWN0OiBSZTogW1JGQyAx
LzJdIG1hYzgwMjExOiBTdG9wIEJBIHNlc3Npb24gZXZlbnQgZnJvbSBkZXZpY2UNCj4gDQo+IE9u
IFRodSwgMjAxMC0xMS0yNSBhdCAxNjozOSArMDIwMCwgU2hhaGFyIExldmkgd3JvdGU6DQo+IA0K
PiA+ICt2b2lkIGllZWU4MDIxMV9zdG9wX3J4X2JhX3Nlc3Npb24oc3RydWN0IGllZWU4MDIxMV92
aWYgKnZpZiwgdTE2DQo+IHRpZCwNCj4gPiArCQkJCSAgY29uc3QgdTggKmFkZHIpOw0KPiANCj4g
bWlzc2luZyBkb2NzDQo+IA0KPiA+ICt2b2lkIGllZWU4MDIxMV9zdG9wX3J4X2JhX3Nlc3Npb24o
c3RydWN0IGllZWU4MDIxMV92aWYgKnZpZiwgdTE2DQo+IHRpZCwNCj4gPiArCQkJCSAgY29uc3Qg
dTggKmFkZHIpDQo+ID4gK3sNCj4gPiArCXN0cnVjdCBpZWVlODAyMTFfc3ViX2lmX2RhdGEgKnNk
YXRhID0gdmlmX3RvX3NkYXRhKHZpZik7DQo+ID4gKwlzdHJ1Y3Qgc3RhX2luZm8gKnN0YSA9IHN0
YV9pbmZvX2dldChzZGF0YSwgYWRkcik7DQo+ID4gKw0KPiA+ICsJX19pZWVlODAyMTFfc3RvcF9y
eF9iYV9zZXNzaW9uKHN0YSwgdGlkLCBXTEFOX0JBQ0tfSU5JVElBVE9SLCAwLA0KPiB0cnVlKTsN
Cj4gPiArfQ0KPiA+ICtFWFBPUlRfU1lNQk9MKGllZWU4MDIxMV9zdG9wX3J4X2JhX3Nlc3Npb24p
Ow0KPiANCj4gTG9ja2luZy13aXNlLCB0aGlzIGlzIGEgZGlzYXN0ZXIsIGFzIHRoZSBmdW5jdGlv
biB3aWxsIGFjcXVpcmUgYSBtdXRleA0KPiB0aGF0IGl0IGFsc28gaG9sZHMgYWNyb3NzIG90aGVy
IGNhbGxzIGludG8gdGhlIGRyaXZlci4gSXQgd291bGQgc2VlbQ0KPiB0aGF0IHRoaXMgc2hvdWxk
IGhhdmUgY2F1c2VkIGxvY2tkZXAgdG8gd2FybiB5b3UgYWJvdXQgQUJCQSBzdHlsZQ0KPiBkZWFk
bG9jayBzaW5jZSB5b3UgY2FsbCB0aGlzIHdpdGggd2wtPm11dGV4IGhlbGQsIGFuZCBwcmV2aW91
c2x5IHlvdQ0KPiBzYWlkIHlvdSB3ZXJlIGdvaW5nIHRvIGFjcXVpcmUgdGhhdCBtdXRleCBmcm9t
IGFtcGR1X2FjdGlvbi4NCj4gDQo+IFRoaXMgc2hvdWxkIHVzZSBhIHNpbWlsYXIgdHJpY2sgdG8g
dGhlIG9uZSB1c2VkIGluIFRYIGFnZ3JlZ2F0aW9uLg0KPiANCj4gSm9oYW5uZXMNClRoYW5rcyBm
b3IgZGlyZWN0aW9uLCBJIHdpbGwgcmV2aWV3IGFuZCB1cGRhdGUuDQpTaGFoYXIgDQoNCg==