WARN_ON message hit on enabling power save.

WARN_ON message hit on enabling power save.

[Vivek Natarajan]

Hello Kalle,

In the function 'ieee80211_master_start_xmit',
ieee80211_stop_queues_by_reason is called and a  ps_disable_work is
queued where wake_queues is called. Can you please clarify me on why the
queues are stopped and started. If at all that needs to be stopped,
wouldn't ieee80211_dynamic_ps_enable_work be a better place to do it?

The issue is  while this ps_disable_work is queued,
netif_subqueue_stopped() is checked in _ieee80211_tx which ultimately
results in a WARN_ON(info->flags & IEEE80211_TX_CTL_AMPDU) in
ieee80211_tx. Please see the code for clarification.

So, while I try to ping after enabling power save, the packets are
dropped due to this warning.
The warning message is not at all related to the issue and hence I feel
that this is an inappropriate place for that WARN_ON. 

The fix for the issue is to remove the stop/start queues if it is not
really needed or the better fix is to remove this inappropriate warning
message. Please clarify me if I'm wrong.


Thanks for your time,
Vivek.

Re: WARN_ON message hit on enabling power save.

[Kalle Valo]

Vivek Natarajan <vivek.natraj@gmail.com> writes:

> Hello Kalle,

Hi Vivek,

> In the function 'ieee80211_master_start_xmit',
> ieee80211_stop_queues_by_reason is called and a  ps_disable_work is
> queued where wake_queues is called. Can you please clarify me on why the
> queues are stopped and started.

Originally they were in ieee80211_subif_start_xmit() before you moved
them. The reason why added them was to preserve the order of first
disabling IEEE80211_CONF_PS and after that sending the data frames.
Because ieee80211_hw_config() must sleep, I decided to do it in a
workqueue.

> If at all that needs to be stopped, wouldn't
> ieee80211_dynamic_ps_enable_work be a better place to do it?

That's was too late. In that case some of the date frames might have
been already transmitted to the driver before IEEE80211_CONF_PS is
disabled.

> The issue is  while this ps_disable_work is queued,
> netif_subqueue_stopped() is checked in _ieee80211_tx which ultimately
> results in a WARN_ON(info->flags & IEEE80211_TX_CTL_AMPDU) in
> ieee80211_tx. Please see the code for clarification.
>
> So, while I try to ping after enabling power save, the packets are
> dropped due to this warning. The warning message is not at all
> related to the issue and hence I feel that this is an inappropriate
> place for that WARN_ON.
>
> The fix for the issue is to remove the stop/start queues if it is not
> really needed or the better fix is to remove this inappropriate warning
> message. Please clarify me if I'm wrong.

Unfortunately I'm not familiar with the 11n implementation. I will
take a look at this more closely tomorrow and will get back to you.

-- 
Kalle Valo