2012-02-23 15:53:46

by Nicolas Cavallari

[permalink] [raw]
Subject: [RFC] carl9170: Fix memory accounting when sta is in power-save mode.

On Access Point mode, when transmitting a packet, if the destination
station is in powersave mode, we abort transmitting the packet to the
device queue, but we do not reclaim the allocated memory. Given enough
packets, we can go in a state where there is no packet on the device
queue, but we think the device has no memory left, so no packet gets
transmitted, connections breaks and the AP stops working.

This undo the allocation done in the TX path when the station is in
power-save mode.

Signed-off-by: Nicolas Cavallari <[email protected]>
---
While this seems to fix my problem, there could still be problems remaining
in this code path. Particularly, carl9170_tx_status will do lots of things
like incrementing counters and waking up mac80211 queues...

diff --git a/drivers/net/wireless/ath/carl9170/tx.c b/drivers/net/wireless/ath/carl9170/tx.c
index 771e1a9..8d52ddd 100644
--- a/drivers/net/wireless/ath/carl9170/tx.c
+++ b/drivers/net/wireless/ath/carl9170/tx.c
@@ -1253,6 +1253,7 @@ static bool carl9170_tx_ps_drop(struct ar9170 *ar, struct sk_buff *skb)
atomic_dec(&ar->tx_ampdu_upload);

tx_info->flags |= IEEE80211_TX_STAT_TX_FILTERED;
+ carl9170_release_dev_space(ar, skb);
carl9170_tx_status(ar, skb, false);
return true;
}
--
1.7.9



2012-02-23 18:08:30

by Christian Lamparter

[permalink] [raw]
Subject: Re: [RFC] carl9170: Fix memory accounting when sta is in power-save mode.

On Thursday, February 23, 2012 06:27:59 PM Nicolas Cavallari wrote:
> On 23/02/2012 17:52, Christian Lamparter wrote:
> > On Thursday, February 23, 2012 04:53:34 PM Nicolas Cavallari wrote:
> >> While this seems to fix my problem, there could still be problems remaining
> >> in this code path. Particularly carl9170_tx_status will do lots of things
> >> like incrementing counters and waking up mac80211 queues...
> > That should be alright. As a rule of thumb, for every call to
> > carl9170_tx_accounting [called by carl9170_op_tx tx handler]
> > we need to call carl9170_tx_status. If the station is still in
> > ps mode, mac80211 will cache the station's frames and sets
> > the TIM bits.
>
> Part of the reason i wrote this, is because, while this patch fixes the
> memory accounting problem and prevent the AP from stopping working, the
> powersaving station sometimes cannot communicate with the AP anymore
> (other stations can). The station uses ath9k, and is just really doing a
> background scan. The first traces that i have just indicate that the
> station entered powersave mode, then sends probe requests and probe
> requests directed to the AP, then sends authentication requests to the
> AP (?), but the AP does not respond to any of them...
>
Do you run an 802.11n ap on carl9170?

Regards,
Chr

2012-02-25 20:12:57

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC] carl9170: Fix memory accounting when sta is in power-save mode.

On Sat, 2012-02-25 at 11:32 +0200, Jouni Malinen wrote:
> On Fri, Feb 24, 2012 at 03:09:02PM +0100, Johannes Berg wrote:
> > On Fri, 2012-02-24 at 15:01 +0100, Nicolas Cavallari wrote:
> > > Hostapd already does that¹ ... after the association response is acked.
> >
> > Ok maybe then we should send out assoc resp etc. frames and not subject
> > them to PS queueing? Jouni?
>
> We should try to follow the clarified description of management frame
> buffering in P802.11-REVmb/D12.0 (i.e., soon to be published IEEE Std
> 802.11-2012). (Re)Association Response frame is not a bufferable
> management frame and as such, it needs to be sent out without being
> subject to PS queuing.

Ah yes, you mentioned before that this was being clarified but I forgot
about it again. I'll take a look.

johannes



2012-02-27 12:00:27

by Nicolas Cavallari

[permalink] [raw]
Subject: Re: [PATCH -stable] carl9170: fix frame delivery if sta is in powersave mode

On 25/02/2012 21:36, Christian Lamparter wrote:
> Nicolas Cavallari discovered that carl9170 has some
> serious problems delivering data to sleeping stations.
>
> It turns out that the driver was not honoring two
> important flags (IEEE80211_TX_CTL_POLL_RESPONSE and
> IEEE80211_TX_CTL_CLEAR_PS_FILT) which are set on
> frames that should be sent although the receiving
> station is still in powersave mode.
>
> Cc: stable <[email protected]>
> Reported-by: Nicolas Cavallari <[email protected]>
> Signed-off-by: Christian Lamparter <[email protected]>
> ---
>> Part of the reason i wrote this, is because, while this patch fixes the
>> memory accounting problem and prevent the AP from stopping working, the
>> powersaving station sometimes cannot communicate with the AP anymore
>> (other stations can). The station uses ath9k, and is just really doing a
>> background scan. The first traces that i have just indicate that the
>> station entered powersave mode, then sends probe requests and probe
>> requests directed to the AP, then sends authentication requests to the
>> AP (?), but the AP does not respond to any of them...
>>
> Ok, I think this patch [on top of yours] should fix the problem.

This effectively fixes the problem.

Thanks !

2012-02-25 20:20:07

by Christian Lamparter

[permalink] [raw]
Subject: Re: [RFC] carl9170: Fix memory accounting when sta is in power-save mode.

On Saturday 25 February 2012 21:12:52 Johannes Berg wrote:
> On Sat, 2012-02-25 at 11:32 +0200, Jouni Malinen wrote:
> > On Fri, Feb 24, 2012 at 03:09:02PM +0100, Johannes Berg wrote:
> > > On Fri, 2012-02-24 at 15:01 +0100, Nicolas Cavallari wrote:
> > > > Hostapd already does that¹ ... after the association response is acked.
> > >
> > > Ok maybe then we should send out assoc resp etc. frames and not subject
> > > them to PS queueing? Jouni?
> >
> > We should try to follow the clarified description of management frame
> > buffering in P802.11-REVmb/D12.0 (i.e., soon to be published IEEE Std
> > 802.11-2012). (Re)Association Response frame is not a bufferable
> > management frame and as such, it needs to be sent out without being
> > subject to PS queuing.
>
> Ah yes, you mentioned before that this was being clarified but I forgot
> about it again. I'll take a look.
>
Actually, I think I found the problem in carl9170 [writing a patch atm].
[I forgot to check IEEE80211_TX_CTL_POLL_RESPONSE and
IEEE80211_TX_CTL_CLEAR_PS_FILT flags].

[A simple fix: we could set something like TX_CTL_IGNORE_PS
(now known as TX_CTL_POLL_RESPONSE) on outgoing
auth & (re-)assoc frames]

Regards,
Chr

2012-02-25 10:34:55

by Jouni Malinen

[permalink] [raw]
Subject: Re: [RFC] carl9170: Fix memory accounting when sta is in power-save mode.

On Fri, Feb 24, 2012 at 03:09:02PM +0100, Johannes Berg wrote:
> On Fri, 2012-02-24 at 15:01 +0100, Nicolas Cavallari wrote:
> > Hostapd already does that¹ ... after the association response is acked.
>
> Ok maybe then we should send out assoc resp etc. frames and not subject
> them to PS queueing? Jouni?

We should try to follow the clarified description of management frame
buffering in P802.11-REVmb/D12.0 (i.e., soon to be published IEEE Std
802.11-2012). (Re)Association Response frame is not a bufferable
management frame and as such, it needs to be sent out without being
subject to PS queuing.

--
Jouni Malinen PGP id EFC895FA

2012-02-24 13:35:29

by Nicolas Cavallari

[permalink] [raw]
Subject: Re: [RFC] carl9170: Fix memory accounting when sta is in power-save mode.

On 23/02/2012 19:51, Nicolas Cavallari wrote:
> On 23/02/2012 19:08, Christian Lamparter wrote:
>> On Thursday, February 23, 2012 06:27:59 PM Nicolas Cavallari wrote:
>>> On 23/02/2012 17:52, Christian Lamparter wrote:
>>>> On Thursday, February 23, 2012 04:53:34 PM Nicolas Cavallari wrote:
>>>>> While this seems to fix my problem, there could still be problems remaining
>>>>> in this code path. Particularly carl9170_tx_status will do lots of things
>>>>> like incrementing counters and waking up mac80211 queues...
>>>> That should be alright. As a rule of thumb, for every call to
>>>> carl9170_tx_accounting [called by carl9170_op_tx tx handler]
>>>> we need to call carl9170_tx_status. If the station is still in
>>>> ps mode, mac80211 will cache the station's frames and sets
>>>> the TIM bits.
>>> Part of the reason i wrote this, is because, while this patch fixes the
>>> memory accounting problem and prevent the AP from stopping working, the
>>> powersaving station sometimes cannot communicate with the AP anymore
>>> (other stations can). The station uses ath9k, and is just really doing a
>>> background scan. The first traces that i have just indicate that the
>>> station entered powersave mode, then sends probe requests and probe
>>> requests directed to the AP, then sends authentication requests to the
>>> AP (?), but the AP does not respond to any of them...
>>>
>> Do you run an 802.11n ap on carl9170?
> No, it's a 802.11g currently.
> My current wild guess is that, the responses to those requests are
> somehow still queued ...

And i was right...

/*
* Ignore doze->wake transitions that are
* indicated by non-data frames, the standard
* is unclear here, but for example going to
* PS mode and then scanning would cause a
* doze->wake transition for the probe request,
* and that is clearly undesirable.
*/
if (ieee80211_is_data(hdr->frame_control) &&
!ieee80211_has_pm(hdr->frame_control))
ap_sta_ps_end(sta);

As (re)associations requests and Auth frames are not data frames,
mac80211 still considers that the station is still in powersave mode, so
it still queues frames, and even queues the responses to the auth
frames. Except that the station disassociated silently, and is trying to
re-associate after a short time.

Shouldn't mac80211 transition the sta to "awake" when receiving
auth/assoc frames ?

2012-02-24 14:09:04

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC] carl9170: Fix memory accounting when sta is in power-save mode.

On Fri, 2012-02-24 at 15:01 +0100, Nicolas Cavallari wrote:

> >> Shouldn't mac80211 transition the sta to "awake" when receiving
> >> auth/assoc frames ?
> >
> > I don't think so, that would make us send frames to the station it no
> > longer deserves to have. Hostapd should instead delete & re-create the
> > station entry, I think?

>
> Hostapd already does that¹ ... after the association response is acked.

Ok maybe then we should send out assoc resp etc. frames and not subject
them to PS queueing? Jouni?

johannes


2012-02-23 17:28:00

by Nicolas Cavallari

[permalink] [raw]
Subject: Re: [RFC] carl9170: Fix memory accounting when sta is in power-save mode.

On 23/02/2012 17:52, Christian Lamparter wrote:
> On Thursday, February 23, 2012 04:53:34 PM Nicolas Cavallari wrote:
>> While this seems to fix my problem, there could still be problems remaining
>> in this code path. Particularly carl9170_tx_status will do lots of things
>> like incrementing counters and waking up mac80211 queues...
> That should be alright. As a rule of thumb, for every call to
> carl9170_tx_accounting [called by carl9170_op_tx tx handler]
> we need to call carl9170_tx_status. If the station is still in
> ps mode, mac80211 will cache the station's frames and sets
> the TIM bits.

Part of the reason i wrote this, is because, while this patch fixes the
memory accounting problem and prevent the AP from stopping working, the
powersaving station sometimes cannot communicate with the AP anymore
(other stations can). The station uses ath9k, and is just really doing a
background scan. The first traces that i have just indicate that the
station entered powersave mode, then sends probe requests and probe
requests directed to the AP, then sends authentication requests to the
AP (?), but the AP does not respond to any of them...

I did thoses traces with an external device in monitor mode, so i may
have missed some packets. I'll try with a monitor vif on the AP interface.

2012-02-24 14:01:45

by Nicolas Cavallari

[permalink] [raw]
Subject: Re: [RFC] carl9170: Fix memory accounting when sta is in power-save mode.

On 24/02/2012 14:39, Johannes Berg wrote:
> On Fri, 2012-02-24 at 14:34 +0100, Nicolas Cavallari wrote:
>> On 23/02/2012 19:51, Nicolas Cavallari wrote:
>>> On 23/02/2012 19:08, Christian Lamparter wrote:
>>>> On Thursday, February 23, 2012 06:27:59 PM Nicolas Cavallari wrote:
>>>>> On 23/02/2012 17:52, Christian Lamparter wrote:
>>>>>> On Thursday, February 23, 2012 04:53:34 PM Nicolas Cavallari wrote:
>>>>>>> While this seems to fix my problem, there could still be problems remaining
>>>>>>> in this code path. Particularly carl9170_tx_status will do lots of things
>>>>>>> like incrementing counters and waking up mac80211 queues...
>>>>>> That should be alright. As a rule of thumb, for every call to
>>>>>> carl9170_tx_accounting [called by carl9170_op_tx tx handler]
>>>>>> we need to call carl9170_tx_status. If the station is still in
>>>>>> ps mode, mac80211 will cache the station's frames and sets
>>>>>> the TIM bits.
>>>>> Part of the reason i wrote this, is because, while this patch fixes the
>>>>> memory accounting problem and prevent the AP from stopping working, the
>>>>> powersaving station sometimes cannot communicate with the AP anymore
>>>>> (other stations can). The station uses ath9k, and is just really doing a
>>>>> background scan. The first traces that i have just indicate that the
>>>>> station entered powersave mode, then sends probe requests and probe
>>>>> requests directed to the AP, then sends authentication requests to the
>>>>> AP (?), but the AP does not respond to any of them...
>>>>>
>>>> Do you run an 802.11n ap on carl9170?
>>> No, it's a 802.11g currently.
>>> My current wild guess is that, the responses to those requests are
>>> somehow still queued ...
>>
>> And i was right...
>>
>> /*
>> * Ignore doze->wake transitions that are
>> * indicated by non-data frames, the standard
>> * is unclear here, but for example going to
>> * PS mode and then scanning would cause a
>> * doze->wake transition for the probe request,
>> * and that is clearly undesirable.
>> */
>> if (ieee80211_is_data(hdr->frame_control) &&
>> !ieee80211_has_pm(hdr->frame_control))
>> ap_sta_ps_end(sta);
>>
>> As (re)associations requests and Auth frames are not data frames,
>> mac80211 still considers that the station is still in powersave mode, so
>> it still queues frames, and even queues the responses to the auth
>> frames. Except that the station disassociated silently, and is trying to
>> re-associate after a short time.
>>
>> Shouldn't mac80211 transition the sta to "awake" when receiving
>> auth/assoc frames ?
>
> I don't think so, that would make us send frames to the station it no
> longer deserves to have. Hostapd should instead delete & re-create the
> station entry, I think?
>
> johannes
>

Hostapd already does that¹ ... after the association response is acked.

Will send a patch for hostapd then.

[¹] :
/*
* Remove the STA entry in order to make sure the STA PS state gets
* cleared and configuration gets updated in case of reassociation back
* to the same AP.
*/
hostapd_drv_sta_remove(hapd, sta->addr);

2012-02-23 18:52:20

by Nicolas Cavallari

[permalink] [raw]
Subject: Re: [RFC] carl9170: Fix memory accounting when sta is in power-save mode.

On 23/02/2012 19:08, Christian Lamparter wrote:
> On Thursday, February 23, 2012 06:27:59 PM Nicolas Cavallari wrote:
>> On 23/02/2012 17:52, Christian Lamparter wrote:
>>> On Thursday, February 23, 2012 04:53:34 PM Nicolas Cavallari wrote:
>>>> While this seems to fix my problem, there could still be problems remaining
>>>> in this code path. Particularly carl9170_tx_status will do lots of things
>>>> like incrementing counters and waking up mac80211 queues...
>>> That should be alright. As a rule of thumb, for every call to
>>> carl9170_tx_accounting [called by carl9170_op_tx tx handler]
>>> we need to call carl9170_tx_status. If the station is still in
>>> ps mode, mac80211 will cache the station's frames and sets
>>> the TIM bits.
>> Part of the reason i wrote this, is because, while this patch fixes the
>> memory accounting problem and prevent the AP from stopping working, the
>> powersaving station sometimes cannot communicate with the AP anymore
>> (other stations can). The station uses ath9k, and is just really doing a
>> background scan. The first traces that i have just indicate that the
>> station entered powersave mode, then sends probe requests and probe
>> requests directed to the AP, then sends authentication requests to the
>> AP (?), but the AP does not respond to any of them...
>>
> Do you run an 802.11n ap on carl9170?
No, it's a 802.11g currently.
My current wild guess is that, the responses to those requests are
somehow still queued ...

2012-02-24 13:39:03

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC] carl9170: Fix memory accounting when sta is in power-save mode.

On Fri, 2012-02-24 at 14:34 +0100, Nicolas Cavallari wrote:
> On 23/02/2012 19:51, Nicolas Cavallari wrote:
> > On 23/02/2012 19:08, Christian Lamparter wrote:
> >> On Thursday, February 23, 2012 06:27:59 PM Nicolas Cavallari wrote:
> >>> On 23/02/2012 17:52, Christian Lamparter wrote:
> >>>> On Thursday, February 23, 2012 04:53:34 PM Nicolas Cavallari wrote:
> >>>>> While this seems to fix my problem, there could still be problems remaining
> >>>>> in this code path. Particularly carl9170_tx_status will do lots of things
> >>>>> like incrementing counters and waking up mac80211 queues...
> >>>> That should be alright. As a rule of thumb, for every call to
> >>>> carl9170_tx_accounting [called by carl9170_op_tx tx handler]
> >>>> we need to call carl9170_tx_status. If the station is still in
> >>>> ps mode, mac80211 will cache the station's frames and sets
> >>>> the TIM bits.
> >>> Part of the reason i wrote this, is because, while this patch fixes the
> >>> memory accounting problem and prevent the AP from stopping working, the
> >>> powersaving station sometimes cannot communicate with the AP anymore
> >>> (other stations can). The station uses ath9k, and is just really doing a
> >>> background scan. The first traces that i have just indicate that the
> >>> station entered powersave mode, then sends probe requests and probe
> >>> requests directed to the AP, then sends authentication requests to the
> >>> AP (?), but the AP does not respond to any of them...
> >>>
> >> Do you run an 802.11n ap on carl9170?
> > No, it's a 802.11g currently.
> > My current wild guess is that, the responses to those requests are
> > somehow still queued ...
>
> And i was right...
>
> /*
> * Ignore doze->wake transitions that are
> * indicated by non-data frames, the standard
> * is unclear here, but for example going to
> * PS mode and then scanning would cause a
> * doze->wake transition for the probe request,
> * and that is clearly undesirable.
> */
> if (ieee80211_is_data(hdr->frame_control) &&
> !ieee80211_has_pm(hdr->frame_control))
> ap_sta_ps_end(sta);
>
> As (re)associations requests and Auth frames are not data frames,
> mac80211 still considers that the station is still in powersave mode, so
> it still queues frames, and even queues the responses to the auth
> frames. Except that the station disassociated silently, and is trying to
> re-associate after a short time.
>
> Shouldn't mac80211 transition the sta to "awake" when receiving
> auth/assoc frames ?

I don't think so, that would make us send frames to the station it no
longer deserves to have. Hostapd should instead delete & re-create the
station entry, I think?

johannes


2012-02-24 14:07:24

by Christian Lamparter

[permalink] [raw]
Subject: Re: [RFC] carl9170: Fix memory accounting when sta is in power-save mode.

On Friday, February 24, 2012 02:34:52 PM Nicolas Cavallari wrote:
> On 23/02/2012 19:51, Nicolas Cavallari wrote:
> > On 23/02/2012 19:08, Christian Lamparter wrote:
> >> On Thursday, February 23, 2012 06:27:59 PM Nicolas Cavallari wrote:
> >>> On 23/02/2012 17:52, Christian Lamparter wrote:
> >>>> On Thursday, February 23, 2012 04:53:34 PM Nicolas Cavallari wrote:
> >>>>> While this seems to fix my problem, there could still be problems remaining
> >>>>> in this code path. Particularly carl9170_tx_status will do lots of things
> >>>>> like incrementing counters and waking up mac80211 queues...
> >>>> That should be alright. As a rule of thumb, for every call to
> >>>> carl9170_tx_accounting [called by carl9170_op_tx tx handler]
> >>>> we need to call carl9170_tx_status. If the station is still in
> >>>> ps mode, mac80211 will cache the station's frames and sets
> >>>> the TIM bits.
> >>> Part of the reason i wrote this, is because, while this patch fixes the
> >>> memory accounting problem and prevent the AP from stopping working, the
> >>> powersaving station sometimes cannot communicate with the AP anymore
> >>> (other stations can). The station uses ath9k, and is just really doing a
> >>> background scan. The first traces that i have just indicate that the
> >>> station entered powersave mode, then sends probe requests and probe
> >>> requests directed to the AP, then sends authentication requests to the
> >>> AP (?), but the AP does not respond to any of them...
> >>>
> >> Do you run an 802.11n ap on carl9170?
> > No, it's a 802.11g currently.
> > My current wild guess is that, the responses to those requests are
> > somehow still queued ...
>
> And i was right...
>
> /*
> * Ignore doze->wake transitions that are
> * indicated by non-data frames, the standard
> * is unclear here, but for example going to
> * PS mode and then scanning would cause a
> * doze->wake transition for the probe request,
> * and that is clearly undesirable.
> */
> if (ieee80211_is_data(hdr->frame_control) &&
> !ieee80211_has_pm(hdr->frame_control))
> ap_sta_ps_end(sta);
>
> As (re)associations requests and Auth frames are not data frames,
> mac80211 still considers that the station is still in powersave mode, so
> it still queues frames, and even queues the responses to the auth
> frames. Except that the station disassociated silently, and is trying to
> re-associate after a short time.

That's really odd... Doesn't your ath9k client say why it deauthed from
the ap in the first place? Furthermore, [I think] there's no such thing
as a "silent disassociation", I bet we will find the missing deauth
frames stuck in a queue as well...

Regards,
Chr

2012-02-25 20:36:42

by Christian Lamparter

[permalink] [raw]
Subject: [PATCH -stable] carl9170: fix frame delivery if sta is in powersave mode

Nicolas Cavallari discovered that carl9170 has some
serious problems delivering data to sleeping stations.

It turns out that the driver was not honoring two
important flags (IEEE80211_TX_CTL_POLL_RESPONSE and
IEEE80211_TX_CTL_CLEAR_PS_FILT) which are set on
frames that should be sent although the receiving
station is still in powersave mode.

Cc: stable <[email protected]>
Reported-by: Nicolas Cavallari <[email protected]>
Signed-off-by: Christian Lamparter <[email protected]>
---
> Part of the reason i wrote this, is because, while this patch fixes the
> memory accounting problem and prevent the AP from stopping working, the
> powersaving station sometimes cannot communicate with the AP anymore
> (other stations can). The station uses ath9k, and is just really doing a
> background scan. The first traces that i have just indicate that the
> station entered powersave mode, then sends probe requests and probe
> requests directed to the AP, then sends authentication requests to the
> AP (?), but the AP does not respond to any of them...
>
Ok, I think this patch [on top of yours] should fix the problem.
---
diff --git a/drivers/net/wireless/ath/carl9170/tx.c b/drivers/net/wireless/ath/carl9170/tx.c
index 771e1a9..0f00721 100644
--- a/drivers/net/wireless/ath/carl9170/tx.c
+++ b/drivers/net/wireless/ath/carl9170/tx.c
@@ -1236,6 +1236,7 @@ static bool carl9170_tx_ps_drop(struct ar9170 *ar, struct sk_buff *skb)
{
struct ieee80211_sta *sta;
struct carl9170_sta_info *sta_info;
+ struct ieee80211_tx_info *tx_info;

rcu_read_lock();
sta = __carl9170_get_tx_sta(ar, skb);
@@ -1243,17 +1244,18 @@ static bool carl9170_tx_ps_drop(struct ar9170 *ar, struct sk_buff *skb)
goto out_rcu;

sta_info = (void *) sta->drv_priv;
- if (unlikely(sta_info->sleeping)) {
- struct ieee80211_tx_info *tx_info;
+ tx_info = IEEE80211_SKB_CB(skb);

+ if (unlikely(sta_info->sleeping) &&
+ !(tx_info->flags & (IEEE80211_TX_CTL_POLL_RESPONSE |
+ IEEE80211_TX_CTL_CLEAR_PS_FILT))) {
rcu_read_unlock();

- tx_info = IEEE80211_SKB_CB(skb);
if (tx_info->flags & IEEE80211_TX_CTL_AMPDU)
atomic_dec(&ar->tx_ampdu_upload);

tx_info->flags |= IEEE80211_TX_STAT_TX_FILTERED;
carl9170_release_dev_space(ar, skb);
carl9170_tx_status(ar, skb, false);
return true;
}

2012-02-24 20:43:48

by Christian Lamparter

[permalink] [raw]
Subject: Re: [RFC] carl9170: Fix memory accounting when sta is in power-save mode.

On Thursday 23 February 2012 18:27:59 Nicolas Cavallari wrote:
> On 23/02/2012 17:52, Christian Lamparter wrote:
> > On Thursday, February 23, 2012 04:53:34 PM Nicolas Cavallari wrote:
> >> While this seems to fix my problem, there could still be problems remaining
> >> in this code path. Particularly carl9170_tx_status will do lots of things
> >> like incrementing counters and waking up mac80211 queues...
> > That should be alright. As a rule of thumb, for every call to
> > carl9170_tx_accounting [called by carl9170_op_tx tx handler]
> > we need to call carl9170_tx_status. If the station is still in
> > ps mode, mac80211 will cache the station's frames and sets
> > the TIM bits.
>
> Part of the reason i wrote this, is because, while this patch fixes the
> memory accounting problem and prevent the AP from stopping working, the
> powersaving station sometimes cannot communicate with the AP anymore
> (other stations can). The station uses ath9k, and is just really doing a
> background scan. The first traces that i have just indicate that the
> station entered powersave mode, then sends probe requests and probe
> requests directed to the AP, then sends authentication requests to the
> AP (?), but the AP does not respond to any of them...
understood.

In the meantime, I think we can put this fix into the kernel queue right away.

John,
would you please do the honors? :-D

Regards,
Chr

2012-02-23 16:52:58

by Christian Lamparter

[permalink] [raw]
Subject: Re: [RFC] carl9170: Fix memory accounting when sta is in power-save mode.

On Thursday, February 23, 2012 04:53:34 PM Nicolas Cavallari wrote:
> On Access Point mode, when transmitting a packet, if the destination
> station is in powersave mode, we abort transmitting the packet to the
> device queue, but we do not reclaim the allocated memory. Given enough
> packets, we can go in a state where there is no packet on the device
> queue, but we think the device has no memory left, so no packet gets
> transmitted, connections breaks and the AP stops working.
>
> This undo the allocation done in the TX path when the station is in
> power-save mode.
>
> Signed-off-by: Nicolas Cavallari <[email protected]>
Acked-by: Christian Lamparter <[email protected]>
Cc: stable <[email protected]>

> ---
> While this seems to fix my problem, there could still be problems remaining
> in this code path. Particularly carl9170_tx_status will do lots of things
> like incrementing counters and waking up mac80211 queues...
That should be alright. As a rule of thumb, for every call to
carl9170_tx_accounting [called by carl9170_op_tx tx handler]
we need to call carl9170_tx_status. If the station is still in
ps mode, mac80211 will cache the station's frames and sets
the TIM bits.

>
> diff --git a/drivers/net/wireless/ath/carl9170/tx.c b/drivers/net/wireless/ath/carl9170/tx.c
> index 771e1a9..8d52ddd 100644
> --- a/drivers/net/wireless/ath/carl9170/tx.c
> +++ b/drivers/net/wireless/ath/carl9170/tx.c
> @@ -1253,6 +1253,7 @@ static bool carl9170_tx_ps_drop(struct ar9170 *ar, struct sk_buff *skb)
> atomic_dec(&ar->tx_ampdu_upload);
>
> tx_info->flags |= IEEE80211_TX_STAT_TX_FILTERED;
> + carl9170_release_dev_space(ar, skb);
> carl9170_tx_status(ar, skb, false);
> return true;
> }
>