[PATCH] mac80211: allow Rx in reconfig only after removing BA sessions

[PATCH] mac80211: allow Rx in reconfig only after removing BA sessions

[Arik Nemtsov]

Previously, a connected STA/AP could send us some AMPDUs right after
recovery, without the driver knowing anything about it.

Signed-off-by: Arik Nemtsov <arik@wizery.com>
---
 net/mac80211/util.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 242ecde..a041f20 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1411,9 +1411,6 @@ int ieee80211_reconfig(struct ieee80211_local *local)
 		if (ieee80211_sdata_running(sdata))
 			ieee80211_enable_keys(sdata);
 
-	local->in_reconfig = false;
-	barrier();
-
  wake_up:
 	/*
 	 * Clear the WLAN_STA_BLOCK_BA flag so new aggregation
@@ -1436,6 +1433,8 @@ int ieee80211_reconfig(struct ieee80211_local *local)
 		mutex_unlock(&local->sta_mtx);
 	}
 
+	local->in_reconfig = false;
+
 	ieee80211_wake_queues_by_reason(hw,
 			IEEE80211_QUEUE_STOP_REASON_SUSPEND);
 
-- 
1.7.9.5

Re: [PATCH] mac80211: allow Rx in reconfig only after removing BA sessions

[Johannes Berg]

On Wed, 2012-06-27 at 21:25 +0300, Arik Nemtsov wrote:
> Previously, a connected STA/AP could send us some AMPDUs right after
> recovery, without the driver knowing anything about it.

Huh, that description doesn't make a lot of sense? The STA/AP can send
us AMPDUs anyway without the driver knowing anything about it since it
has no idea we're restarting ...

johannes

Re: [PATCH] mac80211: allow Rx in reconfig only after removing BA sessions

[Arik Nemtsov]

On Thu, Jun 28, 2012 at 11:17 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Wed, 2012-06-27 at 21:25 +0300, Arik Nemtsov wrote:
>> Previously, a connected STA/AP could send us some AMPDUs right after
>> recovery, without the driver knowing anything about it.
>
> Huh, that description doesn't make a lot of sense? The STA/AP can send
> us AMPDUs anyway without the driver knowing anything about it since it
> has no idea we're restarting ...

Well the point is to drop them early in the Rx path. Should I change
the description or you don't like the patch in general?

Re: [PATCH] mac80211: allow Rx in reconfig only after removing BA sessions

[Johannes Berg]

On Thu, 2012-06-28 at 11:37 +0300, Arik Nemtsov wrote:
> On Thu, Jun 28, 2012 at 11:17 AM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Wed, 2012-06-27 at 21:25 +0300, Arik Nemtsov wrote:
> >> Previously, a connected STA/AP could send us some AMPDUs right after
> >> recovery, without the driver knowing anything about it.
> >
> > Huh, that description doesn't make a lot of sense? The STA/AP can send
> > us AMPDUs anyway without the driver knowing anything about it since it
> > has no idea we're restarting ...
> 
> Well the point is to drop them early in the Rx path. Should I change
> the description or you don't like the patch in general?

I don't mind the patch, I just don't quite understand it still.

The driver is receiving the AMPDUs anyway, and if it's passing them up
why do we need to drop them?

johannes

Re: [PATCH] mac80211: allow Rx in reconfig only after removing BA sessions

[Arik Nemtsov]

On Thu, Jun 28, 2012 at 12:30 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Thu, 2012-06-28 at 11:37 +0300, Arik Nemtsov wrote:
>> On Thu, Jun 28, 2012 at 11:17 AM, Johannes Berg
>> <johannes@sipsolutions.net> wrote:
>> > On Wed, 2012-06-27 at 21:25 +0300, Arik Nemtsov wrote:
>> >> Previously, a connected STA/AP could send us some AMPDUs right after
>> >> recovery, without the driver knowing anything about it.
>> >
>> > Huh, that description doesn't make a lot of sense? The STA/AP can send
>> > us AMPDUs anyway without the driver knowing anything about it since it
>> > has no idea we're restarting ...
>>
>> Well the point is to drop them early in the Rx path. Should I change
>> the description or you don't like the patch in general?
>
> I don't mind the patch, I just don't quite understand it still.
>
> The driver is receiving the AMPDUs anyway, and if it's passing them up
> why do we need to drop them?

Well if the de-aggregration is in HW, they won't make it as far as
mac80211. So this patch is for SW de-aggregators.

But come to think of it, if the de-aggregation is done in SW, I guess
there's no real issue with accepting them, since mac80211 didn't
really reboot.

I guess we can drop the patch? It just seemed more correct to put the
in_reconfig to false there.

Re: [PATCH] mac80211: allow Rx in reconfig only after removing BA sessions

[Johannes Berg]

On Thu, 2012-06-28 at 12:38 +0300, Arik Nemtsov wrote:
> On Thu, Jun 28, 2012 at 12:30 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Thu, 2012-06-28 at 11:37 +0300, Arik Nemtsov wrote:
> >> On Thu, Jun 28, 2012 at 11:17 AM, Johannes Berg
> >> <johannes@sipsolutions.net> wrote:
> >> > On Wed, 2012-06-27 at 21:25 +0300, Arik Nemtsov wrote:
> >> >> Previously, a connected STA/AP could send us some AMPDUs right after
> >> >> recovery, without the driver knowing anything about it.
> >> >
> >> > Huh, that description doesn't make a lot of sense? The STA/AP can send
> >> > us AMPDUs anyway without the driver knowing anything about it since it
> >> > has no idea we're restarting ...
> >>
> >> Well the point is to drop them early in the Rx path. Should I change
> >> the description or you don't like the patch in general?
> >
> > I don't mind the patch, I just don't quite understand it still.
> >
> > The driver is receiving the AMPDUs anyway, and if it's passing them up
> > why do we need to drop them?
> 
> Well if the de-aggregration is in HW, they won't make it as far as
> mac80211. So this patch is for SW de-aggregators.

I don't think there's anyone doing AMPDU SW deaggregation? There
definitely isn't any code in mac80211 to do it ;-)

> But come to think of it, if the de-aggregation is done in SW, I guess
> there's no real issue with accepting them, since mac80211 didn't
> really reboot.

Or are you thinking of the reorder buffer?

> I guess we can drop the patch? It just seemed more correct to put the
> in_reconfig to false there.

I don't see how it's more correct? We're done reconfiguring and then
decide to drop all BA sessions ;-)

In a way, heck, it seems more correct to leave as-is. If we send a BA
action frame and receive a response to it maybe (is there a response to
delBA?) we don't want to drop it. Or if we send delBA and the peer wants
to start right away again, ...?

johannes

Re: [PATCH] mac80211: allow Rx in reconfig only after removing BA sessions

[Arik Nemtsov]

On Thu, Jun 28, 2012 at 12:42 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Thu, 2012-06-28 at 12:38 +0300, Arik Nemtsov wrote:
>> On Thu, Jun 28, 2012 at 12:30 PM, Johannes Berg
>> <johannes@sipsolutions.net> wrote:
>> > On Thu, 2012-06-28 at 11:37 +0300, Arik Nemtsov wrote:
>> >> On Thu, Jun 28, 2012 at 11:17 AM, Johannes Berg
>> >> <johannes@sipsolutions.net> wrote:
>> >> > On Wed, 2012-06-27 at 21:25 +0300, Arik Nemtsov wrote:
>> >> >> Previously, a connected STA/AP could send us some AMPDUs right after
>> >> >> recovery, without the driver knowing anything about it.
>> >> >
>> >> > Huh, that description doesn't make a lot of sense? The STA/AP can send
>> >> > us AMPDUs anyway without the driver knowing anything about it since it
>> >> > has no idea we're restarting ...
>> >>
>> >> Well the point is to drop them early in the Rx path. Should I change
>> >> the description or you don't like the patch in general?
>> >
>> > I don't mind the patch, I just don't quite understand it still.
>> >
>> > The driver is receiving the AMPDUs anyway, and if it's passing them up
>> > why do we need to drop them?
>>
>> Well if the de-aggregration is in HW, they won't make it as far as
>> mac80211. So this patch is for SW de-aggregators.
>
> I don't think there's anyone doing AMPDU SW deaggregation? There
> definitely isn't any code in mac80211 to do it ;-)
>
>> But come to think of it, if the de-aggregation is done in SW, I guess
>> there's no real issue with accepting them, since mac80211 didn't
>> really reboot.
>
> Or are you thinking of the reorder buffer?
>
>> I guess we can drop the patch? It just seemed more correct to put the
>> in_reconfig to false there.
>
> I don't see how it's more correct? We're done reconfiguring and then
> decide to drop all BA sessions ;-)
>
> In a way, heck, it seems more correct to leave as-is. If we send a BA
> action frame and receive a response to it maybe (is there a response to
> delBA?) we don't want to drop it. Or if we send delBA and the peer wants
> to start right away again, ...?

Yea it's a good point. Let's drop this.

Arik