mac80211 configure rxfilter per HW,
so we don't need this per channel.
This fix problem when chanctx used and
ath9k allocate new ath_chanctx. Then we loose
rxfilter configuration.
Eg. during p2p_find (when 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 <[email protected]>
---
@Sujith, Felix please review
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
On Wed, 2015-06-17 at 11:42 +0200, Felix Fietkau wrote:
> On 2015-06-17 11:39, Sujith Manoharan wrote:
> > Janusz Dziedzic wrote:
> >> This is what I have with use_chanctx=1
> >> Jun 17 06:57:54 dell6430 kernel: [53297.753256] xxx ath_calcrxfilter
> >> filter: 0x207
> >>
> >> 0x000000080 is for probe_req
> >
> > Ok, so the probe_req filter is set before actually initiating
> > RoC - this appears to be the current design in wpa_s.
> >
> > I am not sure if using a global filter that disregards
> > the opmode of a context is the right approach, though...
> The filter is intended to be global in the mac80211 API.
> Treating it as per channel-context inside the driver is a bug.
We have a patch somewhere in our pipeline (not ready/complete yet) that
will change it to be per interface. From there, you could derive per
channel context too if you wish.
johannes
On 2015-06-17 10:20, Sujith Manoharan wrote:
> Janusz Dziedzic wrote:
>> mac80211 configure rxfilter per HW,
>> so we don't need this per channel.
>>
>> This fix problem when chanctx used and
>> ath9k allocate new ath_chanctx. Then we loose
>> rxfilter configuration.
>>
>> Eg. during p2p_find (when 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.
>
> The RX filter is calculated based on the operating mode
> of the HW. If we have a concurrent P2P-GO/station
> setup, for example, then the RX filter needs to be different for each
> context, (MYBEACON vs. BEACON etc.). I don't see how
> having a global filter will allow this ?
mac80211 calculates the filter per hw, not per channel context or per
vif. Setting it for the current channel context only would mean it might
be applied to the wrong context. This patch fixes that issue.
I agree that it might be a useful optimization to selectively apply
filters per channel context, but mac80211 does not provide an API to do
that right now.
- Felix
Janusz Dziedzic wrote:
> This is what I have with use_chanctx=1
> Jun 17 06:57:54 dell6430 kernel: [53297.753256] xxx ath_calcrxfilter
> filter: 0x207
>
> 0x000000080 is for probe_req
Ok, so the probe_req filter is set before actually initiating
RoC - this appears to be the current design in wpa_s.
I am not sure if using a global filter that disregards
the opmode of a context is the right approach, though...
Sujith
Janusz Dziedzic wrote:
> mac80211 configure rxfilter per HW,
> so we don't need this per channel.
>
> This fix problem when chanctx used and
> ath9k allocate new ath_chanctx. Then we loose
> rxfilter configuration.
>
> Eg. during p2p_find (when 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.
The RX filter is calculated based on the operating mode
of the HW. If we have a concurrent P2P-GO/station
setup, for example, then the RX filter needs to be different for each
context, (MYBEACON vs. BEACON etc.). I don't see how
having a global filter will allow this ?
Sujith
Felix Fietkau wrote:
> mac80211 calculates the filter per hw, not per channel context or per
> vif. Setting it for the current channel context only would mean it might
> be applied to the wrong context. This patch fixes that issue.
> I agree that it might be a useful optimization to selectively apply
> filters per channel context, but mac80211 does not provide an API to do
> that right now.
But wpa_s already takes care of this requirement ? This is what I see
when running p2p_find:
wlan0: Starting radio work 'p2p-listen'@0x17c2500 after 0.000040 second wait
nl80211: Enable Probe Request reporting nl_preq=0x17c2560
nl80211: Register frame type=0x40 (WLAN_FC_STYPE_PROBE_REQ) nl_handle=0x17c2560 match=
nl80211: Remain-on-channel cookie 0x351 for freq=2437 MHz duration=204
nl80211: Drv Event 55 (NL80211_CMD_REMAIN_ON_CHANNEL) received for wlan0
nl80211: Remain-on-channel event (cancel=0 freq=2437 channel_type=0 duration=204 cookie=0x351 (match))
wlan0: Event REMAIN_ON_CHANNEL (20) received
And in the driver:
ath: phy1: Starting RoC period
ath: phy1: Channel definition created: 2437 MHz
ath: phy1: Assigned next_chan to 2437 MHz
ath: phy1: Offchannel duration for chan 2437 MHz : 112825
ath: phy1: Set HW RX filter: 0x687
ath: phy1: ath_chanctx_set_next: current: 2412 MHz, next: 2437 MHz
So, the filter for probe requests appears to be set correctly ?
Sujith
On 2015-06-17 07:46, Janusz Dziedzic wrote:
> mac80211 configure rxfilter per HW,
> so we don't need this per channel.
>
> This fix problem when chanctx used and
> ath9k allocate new ath_chanctx. Then we loose
> rxfilter configuration.
>
> Eg. during p2p_find (when 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 <[email protected]>
Acked-by: Felix Fietkau <[email protected]>
- Felix
On 17 June 2015 at 11:00, Sujith Manoharan <[email protected]> wrote:
> Felix Fietkau wrote:
>> mac80211 calculates the filter per hw, not per channel context or per
>> vif. Setting it for the current channel context only would mean it might
>> be applied to the wrong context. This patch fixes that issue.
>> I agree that it might be a useful optimization to selectively apply
>> filters per channel context, but mac80211 does not provide an API to do
>> that right now.
>
> But wpa_s already takes care of this requirement ? This is what I see
> when running p2p_find:
>
> wlan0: Starting radio work 'p2p-listen'@0x17c2500 after 0.000040 second wait
> nl80211: Enable Probe Request reporting nl_preq=0x17c2560
> nl80211: Register frame type=0x40 (WLAN_FC_STYPE_PROBE_REQ) nl_handle=0x17c2560 match=
> nl80211: Remain-on-channel cookie 0x351 for freq=2437 MHz duration=204
> nl80211: Drv Event 55 (NL80211_CMD_REMAIN_ON_CHANNEL) received for wlan0
> nl80211: Remain-on-channel event (cancel=0 freq=2437 channel_type=0 duration=204 cookie=0x351 (match))
> wlan0: Event REMAIN_ON_CHANNEL (20) received
>
> And in the driver:
>
> ath: phy1: Starting RoC period
> ath: phy1: Channel definition created: 2437 MHz
> ath: phy1: Assigned next_chan to 2437 MHz
> ath: phy1: Offchannel duration for chan 2437 MHz : 112825
> ath: phy1: Set HW RX filter: 0x687
> ath: phy1: ath_chanctx_set_next: current: 2412 MHz, next: 2437 MHz
>
> So, the filter for probe requests appears to be set correctly ?
>
This is what I have with use_chanctx=1
un 17 06:57:54 dell6430 kernel: [53297.748030] ath: phy0: RoC request
on vif: 2c:d0:5a:d3:f1:e9, type: 0 duration: 204
Jun 17 06:57:54 dell6430 kernel: [53297.748030] ath: phy0: Starting RoC period
Jun 17 06:57:54 dell6430 kernel: [53297.748031] ath: phy0: Channel
definition created: 2437 MHz
Jun 17 06:57:54 dell6430 kernel: [53297.748032] ath: phy0: Assigned
next_chan to 2437 MHz
Jun 17 06:57:54 dell6430 kernel: [53297.748033] ath: phy0: Offchannel
duration for chan 2437 MHz : 209170
Jun 17 06:57:54 dell6430 kernel: [53297.748038] [<ffffffffc0a3e641>]
ieee80211_configure_filter+0x141/0x290 [mac80211]
Jun 17 06:57:54 dell6430 kernel: [53297.748043] [<ffffffffc0a3e7a5>]
ieee80211_reconfig_filter+0x15/0x20 [mac80211]
Jun 17 06:57:54 dell6430 kernel: [53297.748045] [<ffffffff81090cdf>]
process_one_work+0x14f/0x420
Jun 17 06:57:54 dell6430 kernel: [53297.748046] [<ffffffff81091498>]
worker_thread+0x118/0x530
Jun 17 06:57:54 dell6430 kernel: [53297.748048] [<ffffffff81091380>]
? rescuer_thread+0x3d0/0x3d0
Jun 17 06:57:54 dell6430 kernel: [53297.748049] [<ffffffff81096a02>]
kthread+0xd2/0xf0
Jun 17 06:57:54 dell6430 kernel: [53297.748051] [<ffffffff81096930>]
? kthread_create_on_node+0x180/0x180
Jun 17 06:57:54 dell6430 kernel: [53297.748053] [<ffffffff817c7662>]
ret_from_fork+0x42/0x70
Jun 17 06:57:54 dell6430 kernel: [53297.748054] [<ffffffff81096930>]
? kthread_create_on_node+0x180/0x180
Jun 17 06:57:54 dell6430 kernel: [53297.748055] ---[ end trace
cd36a6099a6f1ca0 ]---
Jun 17 06:57:54 dell6430 kernel: [53297.748056] xxx ath_calcrxfilter
filter: 0x687
Jun 17 06:57:54 dell6430 kernel: [53297.748060] ath: phy0: Set HW RX
filter: 0x687
Jun 17 06:57:54 dell6430 kernel: [53297.748062] ath: phy0:
ath_chanctx_set_next: current: 2412 MHz, next: 2437 MHz
Jun 17 06:57:54 dell6430 kernel: [53297.748062] ath: phy0: Stopping
current chanctx: 2412
Jun 17 06:57:54 dell6430 kernel: [53297.748065] ath: phy0: Flush timeout: 200
Jun 17 06:57:54 dell6430 kernel: [53297.748076] ath: phy0:
ath_chanctx_set_next: Set channel 2437 MHz
Jun 17 06:57:54 dell6430 kernel: [53297.748077] ath: phy0: Set
channel: 2437 MHz width: 0
Jun 17 06:57:54 dell6430 kernel: [53297.748211] ath: phy0: Reset to
2437 MHz, HT40: 0 fastcc: 0
Jun 17 06:57:54 dell6430 kernel: [53297.753144] ------------[ cut here
]------------
Jun 17 06:57:54 dell6430 kernel: [53297.753147] WARNING: CPU: 3 PID:
6140 at drivers/net/wireless/ath/ath9k/recv.c:380
ath_calcrxfilter+0x33/0x1c0 [ath9k]()
Jun 17 06:57:54 dell6430 kernel: [53297.753147] Modules linked in:
iwlmvm(E) iwlwifi(E) ath10k_pci(OE) ath10k_core(OE) ath9k(OE)
ath9k_common(OE) ath9k_hw(OE) ath(E) mac80211(OE) cfg80211(OE) ctr(E)
ccm(E) arc4(E) ftdi_sio(E) usbserial(E) cmac(E) dm_crypt(E)
dell_wmi(E) sparse_keymap(E) dell_laptop(E) dcdbas(E) i8k(E)
intel_rapl(E) iosf_mbi(E) x86_pkg_temp_thermal(E) intel_powerclamp(E)
coretemp(E) rfcomm(E) kvm_intel(E) bnep(E) kvm(E) crct10dif_pclmul(E)
crc32_pclmul(E) uvcvideo(E) ghash_clmulni_intel(E) aesni_intel(E)
videobuf2_vmalloc(E) aes_x86_64(E) videobuf2_memops(E) lrw(E)
gf128mul(E) glue_helper(E) videobuf2_core(E) ablk_helper(E) cryptd(E)
v4l2_common(E) videodev(E) snd_hda_codec_idt(E)
snd_hda_codec_generic(E) media(E) joydev(E) serio_raw(E) ath3k(E)
btusb(E) btbcm(E) btintel(E) snd_hda_intel(E) snd_hda_controller(E)
bluetooth(E) snd_hda_codec(E) snd_hda_core(E) snd_hwdep(E) snd_pcm(E)
snd_seq_midi(E) snd_seq_midi_event(E) snd_rawmidi(E) dell_smo8800(E)
snd_seq(E) snd_seq_device(E) snd_timer(E) lpc_ich(E) snd(E) mei_me(E)
soundcore(E) shpchp(E) mac_hid(E) mei(E) binfmt_misc(E) parport_pc(E)
ppdev(E) lp(E) parport(E) nouveau(E) e1000e(E) i915(E) mxm_wmi(E)
ttm(E) i2c_algo_bit(E) drm_kms_helper(E) ptp(E) ahci(E) sdhci_pci(E)
psmouse(E) libahci(E) drm(E) sdhci(E) pps_core(E) wmi(E) video(E)
[last unloaded: cfg80211]
Jun 17 06:57:54 dell6430 kernel: [53297.753177] CPU: 3 PID: 6140 Comm:
kworker/u16:1 Tainted: G W OE
4.1.0-rc1master-2015-05-14-00-wl-ath+ #21
Jun 17 06:57:54 dell6430 kernel: [53297.753178] Hardware name: Dell
Inc. Latitude E6430/0H3MT5, BIOS A13 09/02/2013
Jun 17 06:57:54 dell6430 kernel: [53297.753181] Workqueue: phy0
ath_chanctx_work [ath9k]
Jun 17 06:57:54 dell6430 kernel: [53297.753186] ffffffffc0799150
ffff8801fe767b48 ffffffff817bf491 0000000000008a34
Jun 17 06:57:54 dell6430 kernel: [53297.753188] 0000000000000000
ffff8801fe767b88 ffffffff8107746a ffff8801fe767b68
Jun 17 06:57:54 dell6430 kernel: [53297.753189] ffff8800ca000018
ffff8800a26893e0 ffff8800a268b960 ffff8800ca000018
Jun 17 06:57:54 dell6430 kernel: [53297.753190] Call Trace:
Jun 17 06:57:54 dell6430 kernel: [53297.753192] [<ffffffff817bf491>]
dump_stack+0x45/0x57
Jun 17 06:57:54 dell6430 kernel: [53297.753194] [<ffffffff8107746a>]
warn_slowpath_common+0x8a/0xc0
Jun 17 06:57:54 dell6430 kernel: [53297.753195] [<ffffffff8107755a>]
warn_slowpath_null+0x1a/0x20
Jun 17 06:57:54 dell6430 kernel: [53297.753197] [<ffffffffc0781de3>]
ath_calcrxfilter+0x33/0x1c0 [ath9k]
Jun 17 06:57:54 dell6430 kernel: [53297.753199] [<ffffffffc0781f8a>]
ath_opmode_init+0x1a/0x50 [ath9k]
Jun 17 06:57:54 dell6430 kernel: [53297.753212] [<ffffffffc07820d1>]
ath_startrecv+0x111/0x140 [ath9k]
Jun 17 06:57:54 dell6430 kernel: [53297.753217] [<ffffffffc077fec3>]
ath_complete_reset+0x33/0x150 [ath9k]
Jun 17 06:57:54 dell6430 kernel: [53297.753219] [<ffffffffc0780138>]
ath_reset_internal+0x158/0x280 [ath9k]
Jun 17 06:57:54 dell6430 kernel: [53297.753222] [<ffffffffc07802a1>]
ath_reset+0x41/0x60 [ath9k]
Jun 17 06:57:54 dell6430 kernel: [53297.753224] [<ffffffffc078a497>]
ath_set_channel+0x187/0x360 [ath9k]
Jun 17 06:57:54 dell6430 kernel: [53297.753226] [<ffffffffc078cb03>]
ath_chanctx_set_next+0x533/0x670 [ath9k]
Jun 17 06:57:54 dell6430 kernel: [53297.753229] [<ffffffffc077e882>]
? ath9k_configure_filter+0xb2/0xc0 [ath9k]
Jun 17 06:57:54 dell6430 kernel: [53297.753234] [<ffffffffc0a3e641>]
? ieee80211_configure_filter+0x141/0x290 [mac80211]
Jun 17 06:57:54 dell6430 kernel: [53297.753238] [<ffffffff8108e5cd>]
? pwq_activate_delayed_work+0x3d/0x90
Jun 17 06:57:54 dell6430 kernel: [53297.753241] [<ffffffffc078cc6d>]
ath_chanctx_work+0x2d/0x40 [ath9k]
Jun 17 06:57:54 dell6430 kernel: [53297.753242] [<ffffffff81090cdf>]
process_one_work+0x14f/0x420
Jun 17 06:57:54 dell6430 kernel: [53297.753244] [<ffffffff81091498>]
worker_thread+0x118/0x530
Jun 17 06:57:54 dell6430 kernel: [53297.753247] [<ffffffff81091380>]
? rescuer_thread+0x3d0/0x3d0
Jun 17 06:57:54 dell6430 kernel: [53297.753248] [<ffffffff81096a02>]
kthread+0xd2/0xf0
Jun 17 06:57:54 dell6430 kernel: [53297.753251] [<ffffffff81096930>]
? kthread_create_on_node+0x180/0x180
Jun 17 06:57:54 dell6430 kernel: [53297.753253] [<ffffffff817c7662>]
ret_from_fork+0x42/0x70
Jun 17 06:57:54 dell6430 kernel: [53297.753255] [<ffffffff81096930>]
? kthread_create_on_node+0x180/0x180
Jun 17 06:57:54 dell6430 kernel: [53297.753256] ---[ end trace
cd36a6099a6f1ca1 ]---
Jun 17 06:57:54 dell6430 kernel: [53297.753256] xxx ath_calcrxfilter
filter: 0x207
0x000000080 is for probe_req
BR
Janusz
Felix Fietkau wrote:
> The filter is intended to be global in the mac80211 API.
> Treating it as per channel-context inside the driver is a bug.
Agreed.
Sujith
On 2015-06-17 11:39, Sujith Manoharan wrote:
> Janusz Dziedzic wrote:
>> This is what I have with use_chanctx=1
>> Jun 17 06:57:54 dell6430 kernel: [53297.753256] xxx ath_calcrxfilter
>> filter: 0x207
>>
>> 0x000000080 is for probe_req
>
> Ok, so the probe_req filter is set before actually initiating
> RoC - this appears to be the current design in wpa_s.
>
> I am not sure if using a global filter that disregards
> the opmode of a context is the right approach, though...
The filter is intended to be global in the mac80211 API.
Treating it as per channel-context inside the driver is a bug.
- Felix