[ath9k-devel] [PATCH 0/3] ath9k: P2P patches when chanctx used

[ath9k-devel] [PATCH 0/3] ath9k: P2P patches when chanctx used

[Janusz Dziedzic]

Patches for problems I hit during P2P tests when
multichannel used (driver loaded with use_chanctx=1).

Janusz Dziedzic (3):
  ath9k: handle RoC abort correctly
  ath9k: make rxfilter per HW
  ath9k: advertise p2p dev support when chanctx

 drivers/net/wireless/ath/ath9k/ath9k.h   |  3 ++-
 drivers/net/wireless/ath/ath9k/channel.c |  3 ++-
 drivers/net/wireless/ath/ath9k/init.c    |  6 +++++-
 drivers/net/wireless/ath/ath9k/main.c    |  2 +-
 drivers/net/wireless/ath/ath9k/recv.c    | 12 ++++++------
 5 files changed, 16 insertions(+), 10 deletions(-)

-- 
1.9.1

[ath9k-devel] [PATCH 1/3] ath9k: handle RoC abort correctly

[Janusz Dziedzic]

In case we will get ROC abort we should not call
ieee80211_remain_on_channel_expired().

In other case I hit such warning on MIPS and
p2p negotiation failed (tested with use_chanctx=1).

ath: phy0: Starting RoC period
ath: phy0: Channel definition created: 2412 MHz
ath: phy0: Assigned next_chan to 2412 MHz
ath: phy0: Offchannel duration for chan 2412 MHz : 506632
ath: phy0: ath_chanctx_set_next: current: 2412 MHz, next: 2412 MHz
ath: phy0: Stopping current chanctx: 2412
ath: phy0: Flush timeout: 200
ath: phy0: ath_chanctx_set_next: Set channel 2412 MHz
ath: phy0: Set channel: 2412 MHz width: 0
ath: phy0: Reset to 2412 MHz, HT40: 0 fastcc: 0
ath: phy0: cur_chan: 2412 MHz, event: ATH_CHANCTX_EVENT_TSF_TIMER, state: ATH_CHANCTX_STATE_IDLE
ath: phy0: ath_offchannel_channel_change: offchannel state: ATH_OFFCHANNEL_ROC_START
ath: phy0: cur_chan: 2412 MHz, event: ATH_CHANCTX_EVENT_SWITCH, state: ATH_CHANCTX_STATE_IDLE
ath: phy0: Cancel RoC
ath: phy0: RoC aborted
ath: phy0: RoC request on vif: 00:03:7f:4e:a0:cd, type: 1 duration: 500
ath: phy0: Starting RoC period
ath: phy0: Channel definition created: 2412 MHz
ath: phy0: Assigned next_chan to 2412 MHz
ath: phy0: Offchannel duration for chan 2412 MHz : 506705
ath: phy0: ath_chanctx_set_next: current: 2412 MHz, next: 2412 MHz
ath: phy0: ath_offchannel_channel_change: offchannel state: ATH_OFFCHANNEL_ROC_START
ath: phy0: cur_chan: 2412 MHz, event: ATH_CHANCTX_EVENT_SWITCH, state: ATH_CHANCTX_STATE_IDLE
------------[ cut here ]------------
WARNING: CPU: 0 PID: 3312 at drivers/net/wireless/ath/ath9k/main.c:2319
Modules linked in: ath9k ath9k_common ath9k_hw ath mac80211 cfg80211

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
---
 drivers/net/wireless/ath/ath9k/channel.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/channel.c b/drivers/net/wireless/ath/ath9k/channel.c
index 2066650..e211325 100644
--- a/drivers/net/wireless/ath/ath9k/channel.c
+++ b/drivers/net/wireless/ath/ath9k/channel.c
@@ -926,7 +926,8 @@ void ath_roc_complete(struct ath_softc *sc, bool abort)
 
 	sc->offchannel.roc_vif = NULL;
 	sc->offchannel.roc_chan = NULL;
-	ieee80211_remain_on_channel_expired(sc->hw);
+	if (!abort)
+		ieee80211_remain_on_channel_expired(sc->hw);
 	ath_offchannel_next(sc);
 	ath9k_ps_restore(sc);
 }
-- 
1.9.1

[ath9k-devel] [PATCH 1/3] ath9k: handle RoC abort correctly

[Krishna Chaitanya]

On Mon, Jun 22, 2015 at 5:13 PM, Janusz Dziedzic
<janusz.dziedzic@tieto.com> wrote:
> In case we will get ROC abort we should not call
> ieee80211_remain_on_channel_expired().
>
> In other case I hit such warning on MIPS and
> p2p negotiation failed (tested with use_chanctx=1).
>
> ath: phy0: Starting RoC period
> ath: phy0: Channel definition created: 2412 MHz
> ath: phy0: Assigned next_chan to 2412 MHz
> ath: phy0: Offchannel duration for chan 2412 MHz : 506632
> ath: phy0: ath_chanctx_set_next: current: 2412 MHz, next: 2412 MHz
> ath: phy0: Stopping current chanctx: 2412
> ath: phy0: Flush timeout: 200
> ath: phy0: ath_chanctx_set_next: Set channel 2412 MHz
> ath: phy0: Set channel: 2412 MHz width: 0
> ath: phy0: Reset to 2412 MHz, HT40: 0 fastcc: 0
> ath: phy0: cur_chan: 2412 MHz, event: ATH_CHANCTX_EVENT_TSF_TIMER, state: ATH_CHANCTX_STATE_IDLE
> ath: phy0: ath_offchannel_channel_change: offchannel state: ATH_OFFCHANNEL_ROC_START
> ath: phy0: cur_chan: 2412 MHz, event: ATH_CHANCTX_EVENT_SWITCH, state: ATH_CHANCTX_STATE_IDLE
> ath: phy0: Cancel RoC
> ath: phy0: RoC aborted
> ath: phy0: RoC request on vif: 00:03:7f:4e:a0:cd, type: 1 duration: 500
> ath: phy0: Starting RoC period
> ath: phy0: Channel definition created: 2412 MHz
> ath: phy0: Assigned next_chan to 2412 MHz
> ath: phy0: Offchannel duration for chan 2412 MHz : 506705
> ath: phy0: ath_chanctx_set_next: current: 2412 MHz, next: 2412 MHz
> ath: phy0: ath_offchannel_channel_change: offchannel state: ATH_OFFCHANNEL_ROC_START
> ath: phy0: cur_chan: 2412 MHz, event: ATH_CHANCTX_EVENT_SWITCH, state: ATH_CHANCTX_STATE_IDLE
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 3312 at drivers/net/wireless/ath/ath9k/main.c:2319
> Modules linked in: ath9k ath9k_common ath9k_hw ath mac80211 cfg80211
>
> Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
> ---
>  drivers/net/wireless/ath/ath9k/channel.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/channel.c b/drivers/net/wireless/ath/ath9k/channel.c
> index 2066650..e211325 100644
> --- a/drivers/net/wireless/ath/ath9k/channel.c
> +++ b/drivers/net/wireless/ath/ath9k/channel.c
> @@ -926,7 +926,8 @@ void ath_roc_complete(struct ath_softc *sc, bool abort)
>
>         sc->offchannel.roc_vif = NULL;
>         sc->offchannel.roc_chan = NULL;
> -       ieee80211_remain_on_channel_expired(sc->hw);
> +       if (!abort)
> +               ieee80211_remain_on_channel_expired(sc->hw);
>         ath_offchannel_next(sc);
>         ath9k_ps_restore(sc);
>  }
If HW aborts RoC in middle, should not we inform mac80211
that RoC is expired?
Also the we are clearing roc_vif independent of abort, so the warning
indicates that roc_complete has not come from FW, may be we should
understand that first?

[ath9k-devel] [PATCH 1/3] ath9k: handle RoC abort correctly

[Michal Kazior]

On 22 June 2015 at 13:56, Krishna Chaitanya <chaitanya.mgit@gmail.com> wrote:
> On Mon, Jun 22, 2015 at 5:13 PM, Janusz Dziedzic
> <janusz.dziedzic@tieto.com> wrote:
>> In case we will get ROC abort we should not call
>> ieee80211_remain_on_channel_expired().
>>
>> In other case I hit such warning on MIPS and
>> p2p negotiation failed (tested with use_chanctx=1).
>>
>> ath: phy0: Starting RoC period
>> ath: phy0: Channel definition created: 2412 MHz
>> ath: phy0: Assigned next_chan to 2412 MHz
>> ath: phy0: Offchannel duration for chan 2412 MHz : 506632
>> ath: phy0: ath_chanctx_set_next: current: 2412 MHz, next: 2412 MHz
>> ath: phy0: Stopping current chanctx: 2412
>> ath: phy0: Flush timeout: 200
>> ath: phy0: ath_chanctx_set_next: Set channel 2412 MHz
>> ath: phy0: Set channel: 2412 MHz width: 0
>> ath: phy0: Reset to 2412 MHz, HT40: 0 fastcc: 0
>> ath: phy0: cur_chan: 2412 MHz, event: ATH_CHANCTX_EVENT_TSF_TIMER, state: ATH_CHANCTX_STATE_IDLE
>> ath: phy0: ath_offchannel_channel_change: offchannel state: ATH_OFFCHANNEL_ROC_START
>> ath: phy0: cur_chan: 2412 MHz, event: ATH_CHANCTX_EVENT_SWITCH, state: ATH_CHANCTX_STATE_IDLE
>> ath: phy0: Cancel RoC
>> ath: phy0: RoC aborted
>> ath: phy0: RoC request on vif: 00:03:7f:4e:a0:cd, type: 1 duration: 500
>> ath: phy0: Starting RoC period
>> ath: phy0: Channel definition created: 2412 MHz
>> ath: phy0: Assigned next_chan to 2412 MHz
>> ath: phy0: Offchannel duration for chan 2412 MHz : 506705
>> ath: phy0: ath_chanctx_set_next: current: 2412 MHz, next: 2412 MHz
>> ath: phy0: ath_offchannel_channel_change: offchannel state: ATH_OFFCHANNEL_ROC_START
>> ath: phy0: cur_chan: 2412 MHz, event: ATH_CHANCTX_EVENT_SWITCH, state: ATH_CHANCTX_STATE_IDLE
>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 3312 at drivers/net/wireless/ath/ath9k/main.c:2319
>> Modules linked in: ath9k ath9k_common ath9k_hw ath mac80211 cfg80211
>>
>> Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
>> ---
>>  drivers/net/wireless/ath/ath9k/channel.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/channel.c b/drivers/net/wireless/ath/ath9k/channel.c
>> index 2066650..e211325 100644
>> --- a/drivers/net/wireless/ath/ath9k/channel.c
>> +++ b/drivers/net/wireless/ath/ath9k/channel.c
>> @@ -926,7 +926,8 @@ void ath_roc_complete(struct ath_softc *sc, bool abort)
>>
>>         sc->offchannel.roc_vif = NULL;
>>         sc->offchannel.roc_chan = NULL;
>> -       ieee80211_remain_on_channel_expired(sc->hw);
>> +       if (!abort)
>> +               ieee80211_remain_on_channel_expired(sc->hw);
>>         ath_offchannel_next(sc);
>>         ath9k_ps_restore(sc);
>>  }
> If HW aborts RoC in middle, should not we inform mac80211
> that RoC is expired?

Good point. The ath_roc_complete() can be called with abort=true from
ath9k_cancel_pending_offchannel() as well. I guess ath_roc_complete()
needs a "reason" argument (instead of "abort") with: expired, aborted,
cancelled values. ieee80211_remain_on_channel_expired() should be
called whenever reason != cancelled.

By the way - is ath_roc_complete() lock protected properly? It looks
like it isn't from a quick glance. Neither sdata lock nor local->mtx
can be implied in all contexts and sc->mutex isn't always held while
it's called, hmm.. or am I missing something?

> Also the we are clearing roc_vif independent of abort, so the warning
> indicates that roc_complete has not come from FW, may be we should
> understand that first?

There's no FW in ath9k.

The problem is the following sequence:
 1. mac80211 requests roc A
 2. mac80211 cancels roc A
   a. ath9k calls expired() and hw_roc_done work is scheduled
 3. mac80211 requests roc B
 4. mac80211 starts to process the scheduled hw_roc_done
 5. mac80211 thinks roc B has expired
 6. mac80211 requests roc C
 7. ath9k WARN_ON is hit

There's a race between (3) and (4). Depending on circumstances (3) and
(4) may be reordered so the current code doesn't fail all the time.


Micha?

[ath9k-devel] [PATCH 1/3] ath9k: handle RoC abort correctly

[Krishna Chaitanya]

On Mon, Jun 22, 2015 at 6:32 PM, Michal Kazior <michal.kazior@tieto.com> wrote:
> On 22 June 2015 at 13:56, Krishna Chaitanya <chaitanya.mgit@gmail.com> wrote:
>> On Mon, Jun 22, 2015 at 5:13 PM, Janusz Dziedzic
>> <janusz.dziedzic@tieto.com> wrote:
>>> In case we will get ROC abort we should not call
>>> ieee80211_remain_on_channel_expired().
>>>
>>> In other case I hit such warning on MIPS and
>>> p2p negotiation failed (tested with use_chanctx=1).
>>>
>>> ath: phy0: Starting RoC period
>>> ath: phy0: Channel definition created: 2412 MHz
>>> ath: phy0: Assigned next_chan to 2412 MHz
>>> ath: phy0: Offchannel duration for chan 2412 MHz : 506632
>>> ath: phy0: ath_chanctx_set_next: current: 2412 MHz, next: 2412 MHz
>>> ath: phy0: Stopping current chanctx: 2412
>>> ath: phy0: Flush timeout: 200
>>> ath: phy0: ath_chanctx_set_next: Set channel 2412 MHz
>>> ath: phy0: Set channel: 2412 MHz width: 0
>>> ath: phy0: Reset to 2412 MHz, HT40: 0 fastcc: 0
>>> ath: phy0: cur_chan: 2412 MHz, event: ATH_CHANCTX_EVENT_TSF_TIMER, state: ATH_CHANCTX_STATE_IDLE
>>> ath: phy0: ath_offchannel_channel_change: offchannel state: ATH_OFFCHANNEL_ROC_START
>>> ath: phy0: cur_chan: 2412 MHz, event: ATH_CHANCTX_EVENT_SWITCH, state: ATH_CHANCTX_STATE_IDLE
>>> ath: phy0: Cancel RoC
>>> ath: phy0: RoC aborted
>>> ath: phy0: RoC request on vif: 00:03:7f:4e:a0:cd, type: 1 duration: 500
>>> ath: phy0: Starting RoC period
>>> ath: phy0: Channel definition created: 2412 MHz
>>> ath: phy0: Assigned next_chan to 2412 MHz
>>> ath: phy0: Offchannel duration for chan 2412 MHz : 506705
>>> ath: phy0: ath_chanctx_set_next: current: 2412 MHz, next: 2412 MHz
>>> ath: phy0: ath_offchannel_channel_change: offchannel state: ATH_OFFCHANNEL_ROC_START
>>> ath: phy0: cur_chan: 2412 MHz, event: ATH_CHANCTX_EVENT_SWITCH, state: ATH_CHANCTX_STATE_IDLE
>>> ------------[ cut here ]------------
>>> WARNING: CPU: 0 PID: 3312 at drivers/net/wireless/ath/ath9k/main.c:2319
>>> Modules linked in: ath9k ath9k_common ath9k_hw ath mac80211 cfg80211
>>>
>>> Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
>>> ---
>>>  drivers/net/wireless/ath/ath9k/channel.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath9k/channel.c b/drivers/net/wireless/ath/ath9k/channel.c
>>> index 2066650..e211325 100644
>>> --- a/drivers/net/wireless/ath/ath9k/channel.c
>>> +++ b/drivers/net/wireless/ath/ath9k/channel.c
>>> @@ -926,7 +926,8 @@ void ath_roc_complete(struct ath_softc *sc, bool abort)
>>>
>>>         sc->offchannel.roc_vif = NULL;
>>>         sc->offchannel.roc_chan = NULL;
>>> -       ieee80211_remain_on_channel_expired(sc->hw);
>>> +       if (!abort)
>>> +               ieee80211_remain_on_channel_expired(sc->hw);
>>>         ath_offchannel_next(sc);
>>>         ath9k_ps_restore(sc);
>>>  }
>> If HW aborts RoC in middle, should not we inform mac80211
>> that RoC is expired?
>
> Good point. The ath_roc_complete() can be called with abort=true from
> ath9k_cancel_pending_offchannel() as well. I guess ath_roc_complete()
> needs a "reason" argument (instead of "abort") with: expired, aborted,
> cancelled values. ieee80211_remain_on_channel_expired() should be
> called whenever reason != cancelled.
Agree, make sense.
> By the way - is ath_roc_complete() lock protected properly? It looks
> like it isn't from a quick glance. Neither sdata lock nor local->mtx
> can be implied in all contexts and sc->mutex isn't always held while
> it's called, hmm.. or am I missing something?
>
>> Also the we are clearing roc_vif independent of abort, so the warning
>> indicates that roc_complete has not come from FW, may be we should
>> understand that first?
>
> There's no FW in ath9k.
>
> The problem is the following sequence:
>  1. mac80211 requests roc A
>  2. mac80211 cancels roc A
>    a. ath9k calls expired() and hw_roc_done work is scheduled
>  3. mac80211 requests roc B
>  4. mac80211 starts to process the scheduled hw_roc_done
>  5. mac80211 thinks roc B has expired
>  6. mac80211 requests roc C
>  7. ath9k WARN_ON is hit
>
> There's a race between (3) and (4). Depending on circumstances (3) and
> (4) may be reordered so the current code doesn't fail all the time.
Ok i understand, but if we get roc_complete for B before 6, then it works
fine at least at ath9k level, C will be unblocked.

Anyways, handling the cancel case should resolve it along with proper locking.

[ath9k-devel] [PATCH 1/3] ath9k: handle RoC abort correctly

[Janusz Dziedzic]

On 22 June 2015 at 16:01, Krishna Chaitanya <chaitanya.mgit@gmail.com> wrote:
> On Mon, Jun 22, 2015 at 6:32 PM, Michal Kazior <michal.kazior@tieto.com> wrote:
>> On 22 June 2015 at 13:56, Krishna Chaitanya <chaitanya.mgit@gmail.com> wrote:
>>> On Mon, Jun 22, 2015 at 5:13 PM, Janusz Dziedzic
>>> <janusz.dziedzic@tieto.com> wrote:
>>>> In case we will get ROC abort we should not call
>>>> ieee80211_remain_on_channel_expired().
>>>>
>>>> In other case I hit such warning on MIPS and
>>>> p2p negotiation failed (tested with use_chanctx=1).
>>>>
>>>> ath: phy0: Starting RoC period
>>>> ath: phy0: Channel definition created: 2412 MHz
>>>> ath: phy0: Assigned next_chan to 2412 MHz
>>>> ath: phy0: Offchannel duration for chan 2412 MHz : 506632
>>>> ath: phy0: ath_chanctx_set_next: current: 2412 MHz, next: 2412 MHz
>>>> ath: phy0: Stopping current chanctx: 2412
>>>> ath: phy0: Flush timeout: 200
>>>> ath: phy0: ath_chanctx_set_next: Set channel 2412 MHz
>>>> ath: phy0: Set channel: 2412 MHz width: 0
>>>> ath: phy0: Reset to 2412 MHz, HT40: 0 fastcc: 0
>>>> ath: phy0: cur_chan: 2412 MHz, event: ATH_CHANCTX_EVENT_TSF_TIMER, state: ATH_CHANCTX_STATE_IDLE
>>>> ath: phy0: ath_offchannel_channel_change: offchannel state: ATH_OFFCHANNEL_ROC_START
>>>> ath: phy0: cur_chan: 2412 MHz, event: ATH_CHANCTX_EVENT_SWITCH, state: ATH_CHANCTX_STATE_IDLE
>>>> ath: phy0: Cancel RoC
>>>> ath: phy0: RoC aborted
>>>> ath: phy0: RoC request on vif: 00:03:7f:4e:a0:cd, type: 1 duration: 500
>>>> ath: phy0: Starting RoC period
>>>> ath: phy0: Channel definition created: 2412 MHz
>>>> ath: phy0: Assigned next_chan to 2412 MHz
>>>> ath: phy0: Offchannel duration for chan 2412 MHz : 506705
>>>> ath: phy0: ath_chanctx_set_next: current: 2412 MHz, next: 2412 MHz
>>>> ath: phy0: ath_offchannel_channel_change: offchannel state: ATH_OFFCHANNEL_ROC_START
>>>> ath: phy0: cur_chan: 2412 MHz, event: ATH_CHANCTX_EVENT_SWITCH, state: ATH_CHANCTX_STATE_IDLE
>>>> ------------[ cut here ]------------
>>>> WARNING: CPU: 0 PID: 3312 at drivers/net/wireless/ath/ath9k/main.c:2319
>>>> Modules linked in: ath9k ath9k_common ath9k_hw ath mac80211 cfg80211
>>>>
>>>> Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
>>>> ---
>>>>  drivers/net/wireless/ath/ath9k/channel.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/wireless/ath/ath9k/channel.c b/drivers/net/wireless/ath/ath9k/channel.c
>>>> index 2066650..e211325 100644
>>>> --- a/drivers/net/wireless/ath/ath9k/channel.c
>>>> +++ b/drivers/net/wireless/ath/ath9k/channel.c
>>>> @@ -926,7 +926,8 @@ void ath_roc_complete(struct ath_softc *sc, bool abort)
>>>>
>>>>         sc->offchannel.roc_vif = NULL;
>>>>         sc->offchannel.roc_chan = NULL;
>>>> -       ieee80211_remain_on_channel_expired(sc->hw);
>>>> +       if (!abort)
>>>> +               ieee80211_remain_on_channel_expired(sc->hw);
>>>>         ath_offchannel_next(sc);
>>>>         ath9k_ps_restore(sc);
>>>>  }
>>> If HW aborts RoC in middle, should not we inform mac80211
>>> that RoC is expired?
>>
>> Good point. The ath_roc_complete() can be called with abort=true from
>> ath9k_cancel_pending_offchannel() as well. I guess ath_roc_complete()
>> needs a "reason" argument (instead of "abort") with: expired, aborted,
>> cancelled values. ieee80211_remain_on_channel_expired() should be
>> called whenever reason != cancelled.
> Agree, make sense.
>> By the way - is ath_roc_complete() lock protected properly? It looks
>> like it isn't from a quick glance. Neither sdata lock nor local->mtx
>> can be implied in all contexts and sc->mutex isn't always held while
>> it's called, hmm.. or am I missing something?
>>
>>> Also the we are clearing roc_vif independent of abort, so the warning
>>> indicates that roc_complete has not come from FW, may be we should
>>> understand that first?
>>
>> There's no FW in ath9k.
>>
>> The problem is the following sequence:
>>  1. mac80211 requests roc A
>>  2. mac80211 cancels roc A
>>    a. ath9k calls expired() and hw_roc_done work is scheduled
>>  3. mac80211 requests roc B
>>  4. mac80211 starts to process the scheduled hw_roc_done
>>  5. mac80211 thinks roc B has expired
>>  6. mac80211 requests roc C
>>  7. ath9k WARN_ON is hit
>>
>> There's a race between (3) and (4). Depending on circumstances (3) and
>> (4) may be reordered so the current code doesn't fail all the time.
> Ok i understand, but if we get roc_complete for B before 6, then it works
> fine at least at ath9k level, C will be unblocked.
>
> Anyways, handling the cancel case should resolve it along with proper locking.

Thanks for comments, will send v2

BR
Janusz

[ath9k-devel] [PATCH 2/3] ath9k: make rxfilter per HW

[Janusz Dziedzic]

mac80211 configure rxfilter per HW,
so we don't need this per channel.

This fix problem when chanctx used and ath9k
allocate new internal ath_chanctx (eg. when
offchannel) and we loose rxfilter configuration.

Eg. when p2p_find (with use_chanctx=1), during
remain on channel, driver create new ath_chanctx
with incorrect rxfilter. Then we didn't receive
probe requests and fail p2p_find.

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
---
 drivers/net/wireless/ath/ath9k/ath9k.h |  3 ++-
 drivers/net/wireless/ath/ath9k/main.c  |  2 +-
 drivers/net/wireless/ath/ath9k/recv.c  | 12 ++++++------
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index a7a81b3..030fd0f 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -356,7 +356,6 @@ struct ath_chanctx {
 
 	short nvifs;
 	short nvifs_assigned;
-	unsigned int rxfilter;
 };
 
 enum ath_chanctx_event {
@@ -1001,8 +1000,10 @@ struct ath_softc {
 	struct cfg80211_chan_def cur_chandef;
 	struct ath_chanctx chanctx[ATH9K_NUM_CHANCTX];
 	struct ath_chanctx *cur_chan;
+	unsigned int rxfilter;
 	spinlock_t chan_lock;
 
+
 #ifdef CONFIG_MAC80211_LEDS
 	bool led_registered;
 	char led_name[32];
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index d285e3a..945f002 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1463,7 +1463,7 @@ static void ath9k_configure_filter(struct ieee80211_hw *hw,
 	*total_flags &= SUPPORTED_FILTERS;
 
 	spin_lock_bh(&sc->chan_lock);
-	sc->cur_chan->rxfilter = *total_flags;
+	sc->rxfilter = *total_flags;
 	spin_unlock_bh(&sc->chan_lock);
 
 	ath9k_ps_wakeup(sc);
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 6c75fb1..5f72c65 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -389,31 +389,31 @@ u32 ath_calcrxfilter(struct ath_softc *sc)
 
 	spin_lock_bh(&sc->chan_lock);
 
-	if (sc->cur_chan->rxfilter & FIF_PROBE_REQ)
+	if (sc->rxfilter & FIF_PROBE_REQ)
 		rfilt |= ATH9K_RX_FILTER_PROBEREQ;
 
 	if (sc->sc_ah->is_monitoring)
 		rfilt |= ATH9K_RX_FILTER_PROM;
 
-	if ((sc->cur_chan->rxfilter & FIF_CONTROL) ||
+	if ((sc->rxfilter & FIF_CONTROL) ||
 	    sc->sc_ah->dynack.enabled)
 		rfilt |= ATH9K_RX_FILTER_CONTROL;
 
 	if ((sc->sc_ah->opmode == NL80211_IFTYPE_STATION) &&
 	    (sc->cur_chan->nvifs <= 1) &&
-	    !(sc->cur_chan->rxfilter & FIF_BCN_PRBRESP_PROMISC))
+	    !(sc->rxfilter & FIF_BCN_PRBRESP_PROMISC))
 		rfilt |= ATH9K_RX_FILTER_MYBEACON;
 	else
 		rfilt |= ATH9K_RX_FILTER_BEACON;
 
 	if ((sc->sc_ah->opmode == NL80211_IFTYPE_AP) ||
-	    (sc->cur_chan->rxfilter & FIF_PSPOLL))
+	    (sc->rxfilter & FIF_PSPOLL))
 		rfilt |= ATH9K_RX_FILTER_PSPOLL;
 
 	if (sc->cur_chandef.width != NL80211_CHAN_WIDTH_20_NOHT)
 		rfilt |= ATH9K_RX_FILTER_COMP_BAR;
 
-	if (sc->cur_chan->nvifs > 1 || (sc->cur_chan->rxfilter & FIF_OTHER_BSS)) {
+	if (sc->cur_chan->nvifs > 1 || (sc->rxfilter & FIF_OTHER_BSS)) {
 		/* This is needed for older chips */
 		if (sc->sc_ah->hw_version.macVersion <= AR_SREV_VERSION_9160)
 			rfilt |= ATH9K_RX_FILTER_PROM;
@@ -878,7 +878,7 @@ static int ath9k_rx_skb_preprocess(struct ath_softc *sc,
 	 */
 	spin_lock_bh(&sc->chan_lock);
 	if (!ath9k_cmn_rx_accept(common, hdr, rx_status, rx_stats, decrypt_error,
-				 sc->cur_chan->rxfilter)) {
+				 sc->rxfilter)) {
 		spin_unlock_bh(&sc->chan_lock);
 		return -EINVAL;
 	}
-- 
1.9.1

[ath9k-devel] [PATCH 2/3] ath9k: make rxfilter per HW

[Johannes Berg]

On Mon, 2015-06-22 at 13:43 +0200, Janusz Dziedzic wrote:
> mac80211 configure rxfilter per HW,
> so we don't need this per channel.

As I said before, I think there's value in mac80211 doing it per chanctx
or even per vif, and it should be more efficient to do so.

It's tempting to do it per vif and leave the chanctx work up to the
driver, but perhaps with CSA and all that it gets complicated enough
that doing it per chanctx in mac80211 would make sense?

johannes

[ath9k-devel] [PATCH 2/3] ath9k: make rxfilter per HW

[Johannes Berg]

On Mon, 2015-06-22 at 13:58 +0200, Johannes Berg wrote:
> On Mon, 2015-06-22 at 13:43 +0200, Janusz Dziedzic wrote:
> > mac80211 configure rxfilter per HW,
> > so we don't need this per channel.
> 
> As I said before, I think there's value in mac80211 doing it per chanctx
> or even per vif, and it should be more efficient to do so.
> 
> It's tempting to do it per vif and leave the chanctx work up to the
> driver, but perhaps with CSA and all that it gets complicated enough
> that doing it per chanctx in mac80211 would make sense?

On the other hand, I think our device requires it per vif, so we'd
probably have to do both.

+Andrei, who was looking into this.

johannes

[ath9k-devel] [PATCH 2/3] ath9k: make rxfilter per HW

[Felix Fietkau]

On 2015-06-22 13:59, Johannes Berg wrote:
> On Mon, 2015-06-22 at 13:58 +0200, Johannes Berg wrote:
>> On Mon, 2015-06-22 at 13:43 +0200, Janusz Dziedzic wrote:
>> > mac80211 configure rxfilter per HW,
>> > so we don't need this per channel.
>> 
>> As I said before, I think there's value in mac80211 doing it per chanctx
>> or even per vif, and it should be more efficient to do so.
>> 
>> It's tempting to do it per vif and leave the chanctx work up to the
>> driver, but perhaps with CSA and all that it gets complicated enough
>> that doing it per chanctx in mac80211 would make sense?
> 
> On the other hand, I think our device requires it per vif, so we'd
> probably have to do both.
> 
> +Andrei, who was looking into this.
There's value in it, but I think it makes more sense to fix this bug
now, and rework the code again later when mac80211 has support for
per-vif or per-chanctx rxfilter.

- Felix

[ath9k-devel] [PATCH 3/3] ath9k: advertise p2p dev support when chanctx

[Janusz Dziedzic]

Advertise p2p device support when ath9k loaded with
use_chanctx=1.

This will fix problem, when first interface is an AP
and next we would like to run p2p_find.
Before p2p find (scan phase) failed with EOPNOTSUPP.

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index f8d11ef..7da1a17 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -736,13 +736,14 @@ static const struct ieee80211_iface_limit if_limits_multi[] = {
 				 BIT(NL80211_IFTYPE_P2P_CLIENT) |
 				 BIT(NL80211_IFTYPE_P2P_GO) },
 	{ .max = 1,	.types = BIT(NL80211_IFTYPE_ADHOC) },
+	{ .max = 1,	.types = BIT(NL80211_IFTYPE_P2P_DEVICE) },
 };
 
 static const struct ieee80211_iface_combination if_comb_multi[] = {
 	{
 		.limits = if_limits_multi,
 		.n_limits = ARRAY_SIZE(if_limits_multi),
-		.max_interfaces = 2,
+		.max_interfaces = 3,
 		.num_different_channels = 2,
 		.beacon_int_infra_match = true,
 	},
@@ -855,6 +856,9 @@ static void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
 			BIT(NL80211_IFTYPE_MESH_POINT) |
 			BIT(NL80211_IFTYPE_WDS);
 
+		if (ath9k_is_chanctx_enabled())
+			hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_P2P_DEVICE);
+
 			hw->wiphy->iface_combinations = if_comb;
 			hw->wiphy->n_iface_combinations = ARRAY_SIZE(if_comb);
 	}
-- 
1.9.1