2012-12-12 21:55:43

by Ben Greear

[permalink] [raw]
Subject: [RFC 1/6] ath9k: Report txerr-filtered errors in debugfs.

From: Ben Greear <[email protected]>

Signed-off-by: Ben Greear <[email protected]>
---
drivers/net/wireless/ath/ath9k/debug.c | 3 +++
drivers/net/wireless/ath/ath9k/debug.h | 2 ++
2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index 13ff9ed..429be51 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -536,6 +536,7 @@ static ssize_t read_file_xmit(struct file *file, char __user *user_buf,
PR("AMPDUs Completed:", a_completed);
PR("AMPDUs Retried: ", a_retries);
PR("AMPDUs XRetried: ", a_xretries);
+ PR("TXERR Filtered: ", txerr_filtered);
PR("FIFO Underrun: ", fifo_underrun);
PR("TXOP Exceeded: ", xtxop);
PR("TXTIMER Expiry: ", timer_exp);
@@ -755,6 +756,8 @@ void ath_debug_stat_tx(struct ath_softc *sc, struct ath_buf *bf,
TX_STAT_INC(qnum, completed);
}

+ if (ts->ts_status & ATH9K_TXERR_FILT)
+ TX_STAT_INC(qnum, txerr_filtered);
if (ts->ts_status & ATH9K_TXERR_FIFO)
TX_STAT_INC(qnum, fifo_underrun);
if (ts->ts_status & ATH9K_TXERR_XTXOP)
diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
index 375c3b4..5c2db3e 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -141,6 +141,7 @@ struct ath_interrupt_stats {
* @a_completed: Total AMPDUs completed
* @a_retries: No. of AMPDUs retried (SW)
* @a_xretries: No. of AMPDUs dropped due to xretries
+ * @txerr_filtered: No. of frames with TXERR_FILT flag set.
* @fifo_underrun: FIFO underrun occurrences
Valid only for:
- non-aggregate condition.
@@ -167,6 +168,7 @@ struct ath_tx_stats {
u32 a_completed;
u32 a_retries;
u32 a_xretries;
+ u32 txerr_filtered;
u32 fifo_underrun;
u32 xtxop;
u32 timer_exp;
--
1.7.3.4



2012-12-12 21:56:47

by Ben Greear

[permalink] [raw]
Subject: [RFC 6/6] ath9k: Allow using ath9k-rate-control without forcing it.

From: Ben Greear <[email protected]>

Let user still pick the rate control method by setting the
mac80211 module option, ie:

# cat /etc/modprobe.d/mac80211.conf
options mac80211 ieee80211_default_rc_algo=ath9k_rate_control

Signed-off-by: Ben Greear <[email protected]>
---
drivers/net/wireless/ath/ath9k/init.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index f69ef5d..772b69f 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -818,7 +818,7 @@ void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
sc->ant_tx = hw->wiphy->available_antennas_tx;

#ifdef CONFIG_ATH9K_RATE_CONTROL
- hw->rate_control_algorithm = "ath9k_rate_control";
+ /* hw->rate_control_algorithm = "ath9k_rate_control"; */
#endif

if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_2GHZ)
--
1.7.3.4


2012-12-12 22:24:08

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC 2/6] wifi: Don't spam logs with 'Found new beacon' messages.

On Wed, 2012-12-12 at 23:20 +0100, Johannes Berg wrote:
> On Wed, 2012-12-12 at 23:17 +0100, Johannes Berg wrote:
> > On Wed, 2012-12-12 at 14:15 -0800, Ben Greear wrote:
> > > On 12/12/2012 02:08 PM, Johannes Berg wrote:
> > > > On Wed, 2012-12-12 at 13:54 -0800, [email protected] wrote:
> > > >> From: Ben Greear <[email protected]>
> > > >>
> > > >> We saw logs fill with this (at very high speeds):
> > > >>
> > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > >>
> > > >> Signed-off-by: Ben Greear <[email protected]>
> > > >> ---
> > > >> net/wireless/reg.c | 11 ++++++-----
> > > >> 1 files changed, 6 insertions(+), 5 deletions(-)
> > > >>
> > > >> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> > > >> index 6e53089..8aa7867 100644
> > > >> --- a/net/wireless/reg.c
> > > >> +++ b/net/wireless/reg.c
> > > >> @@ -2067,11 +2067,12 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy,
> > > >> if (!reg_beacon)
> > > >> return -ENOMEM;
> > > >>
> > > >> - REG_DBG_PRINT("Found new beacon on "
> > > >
> > > > Why not just disable the config option for it if you don't care?
> > >
> > > I don't mind seeing it, just don't want thousands of them in my logs
> > > in a very short amount of time. Maybe I was just hitting some
> > > other bug, and there is no valid reason for this to ever spam?
> >
> > Quite likely, since I think it's supposed to happen only once for each
> > channel unless you reset the regulatory (but that would cause more
> > messages? I think?)
>
> Ah, no, I get it. It is related to your many interfaces :-)
>
> The thing is that when a beacon comes in it schedules a work item, and
> that work item only sets the beacon_found to true. Now since you have a
> ton of interfaces each beacon can be processed many times, and you get a
> ton of work items, and ... ouch.

Or if you're not world roaming.. this is really bad, we alloc all those
beacon hint structs just to not do anything with them, I wonder if they
even get leaked or added to reg_beacon_list (but that's about the same)

This is pretty much a mess. Luis?

johannes


2012-12-12 21:55:37

by Ben Greear

[permalink] [raw]
Subject: [RFC 5/6] ath9k: Report rx-crc-errors in ethtool stats.

From: Ben Greear <[email protected]>

Signed-off-by: Ben Greear <[email protected]>
---
drivers/net/wireless/ath/ath9k/debug.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index 429be51..fe66be2 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -1627,6 +1627,7 @@ static const char ath9k_gstrings_stats[][ETH_GSTRING_LEN] = {
AMKSTR(d_tx_desc_cfg_err),
AMKSTR(d_tx_data_underrun),
AMKSTR(d_tx_delim_underrun),
+ "d_rx_crc_err",
"d_rx_decrypt_crc_err",
"d_rx_phy_err",
"d_rx_mic_err",
@@ -1707,6 +1708,7 @@ void ath9k_get_et_stats(struct ieee80211_hw *hw,
AWDATA(data_underrun);
AWDATA(delim_underrun);

+ AWDATA_RX(crc_err);
AWDATA_RX(decrypt_crc_err);
AWDATA_RX(phy_err);
AWDATA_RX(mic_err);
--
1.7.3.4


2012-12-12 22:12:00

by Ben Greear

[permalink] [raw]
Subject: Re: [RFC 3/6] mac80211: Check for NULL in get-ethtool-stats logic.

On 12/12/2012 02:02 PM, Johannes Berg wrote:
> On Wed, 2012-12-12 at 13:54 -0800, [email protected] wrote:
>> From: Ben Greear <[email protected]>
>>
>> In an earlier version of this code, testing found that sometimes
>> either sta wasn't found or sta->sdata was NULL. This caused
>> crashes of course. So, add checks with WARN_ON_ONCE to
>> catch these.
>
> I don't know what you tested, but there's absolutely no way that
> sta->sdata is NULL... *Especially* not after sta_info_get_bss(), I mean,
> look at that function. But once it's on the list it can't be NULL, so
> I'm not going to apply this patch.
>
> In the future, I'd appreciate if you could avoid avoid sending patches
> for drivers and mac80211 etc. in one series, it gets confusing with who
> applies what etc.

Ok, it was quite a while ago..maybe I was hitting other upstream bugs or
bugs of my own.

For what it's worth, I haven't noticed my WARN_ON hitting.

I'll drop this patch from my tree.

Thanks,
Ben

>
> johannes
>


--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com


2012-12-12 22:19:38

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC 2/6] wifi: Don't spam logs with 'Found new beacon' messages.

On Wed, 2012-12-12 at 23:17 +0100, Johannes Berg wrote:
> On Wed, 2012-12-12 at 14:15 -0800, Ben Greear wrote:
> > On 12/12/2012 02:08 PM, Johannes Berg wrote:
> > > On Wed, 2012-12-12 at 13:54 -0800, [email protected] wrote:
> > >> From: Ben Greear <[email protected]>
> > >>
> > >> We saw logs fill with this (at very high speeds):
> > >>
> > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > >>
> > >> Signed-off-by: Ben Greear <[email protected]>
> > >> ---
> > >> net/wireless/reg.c | 11 ++++++-----
> > >> 1 files changed, 6 insertions(+), 5 deletions(-)
> > >>
> > >> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> > >> index 6e53089..8aa7867 100644
> > >> --- a/net/wireless/reg.c
> > >> +++ b/net/wireless/reg.c
> > >> @@ -2067,11 +2067,12 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy,
> > >> if (!reg_beacon)
> > >> return -ENOMEM;
> > >>
> > >> - REG_DBG_PRINT("Found new beacon on "
> > >
> > > Why not just disable the config option for it if you don't care?
> >
> > I don't mind seeing it, just don't want thousands of them in my logs
> > in a very short amount of time. Maybe I was just hitting some
> > other bug, and there is no valid reason for this to ever spam?
>
> Quite likely, since I think it's supposed to happen only once for each
> channel unless you reset the regulatory (but that would cause more
> messages? I think?)

Ah, no, I get it. It is related to your many interfaces :-)

The thing is that when a beacon comes in it schedules a work item, and
that work item only sets the beacon_found to true. Now since you have a
ton of interfaces each beacon can be processed many times, and you get a
ton of work items, and ... ouch.

Not sure how the locking works here so can't really suggest a fix right
now.

johannes


2012-12-13 00:58:58

by Ben Greear

[permalink] [raw]
Subject: Re: [RFC 4/6] mac80211: Allow disabling SGI-20.

On 12/12/2012 02:06 PM, Johannes Berg wrote:
> On Wed, 2012-12-12 at 13:54 -0800, [email protected] wrote:
>> From: Ben Greear <[email protected]>
>>
>> This allows user-space (wpa_supplicant) to disable
>> short guard interval (SGI) for 20Mhz. The SGI-40
>> disable option is already handled.
>>
>> Signed-off-by: Ben Greear <[email protected]>
>> ---
>> net/mac80211/ht.c | 3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
>> index a71d891..8a6d68f 100644
>> --- a/net/mac80211/ht.c
>> +++ b/net/mac80211/ht.c
>> @@ -62,6 +62,9 @@ void ieee80211_apply_htcap_overrides(struct ieee80211_sub_if_data *sdata,
>> __check_htcap_disable(sdata, ht_cap, IEEE80211_HT_CAP_SUP_WIDTH_20_40);
>> __check_htcap_disable(sdata, ht_cap, IEEE80211_HT_CAP_SGI_40);
>>
>> + /* Allow user to disable SGI-20 (SGI-40 is handled above) */
>> + __check_htcap_disable(sdata, ht_cap, IEEE80211_HT_CAP_SGI_20);
>
> And you tested this? And it actually did something, despite
> mac80211_ht_capa_mod_mask not including it??

It was busted...new patch sent, and the new one was freshly tested and
appears to work as desired.

Thanks,
Ben

>
> johannes
>


--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com


2012-12-12 22:05:50

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC 4/6] mac80211: Allow disabling SGI-20.

On Wed, 2012-12-12 at 13:54 -0800, [email protected] wrote:
> From: Ben Greear <[email protected]>
>
> This allows user-space (wpa_supplicant) to disable
> short guard interval (SGI) for 20Mhz. The SGI-40
> disable option is already handled.
>
> Signed-off-by: Ben Greear <[email protected]>
> ---
> net/mac80211/ht.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
> index a71d891..8a6d68f 100644
> --- a/net/mac80211/ht.c
> +++ b/net/mac80211/ht.c
> @@ -62,6 +62,9 @@ void ieee80211_apply_htcap_overrides(struct ieee80211_sub_if_data *sdata,
> __check_htcap_disable(sdata, ht_cap, IEEE80211_HT_CAP_SUP_WIDTH_20_40);
> __check_htcap_disable(sdata, ht_cap, IEEE80211_HT_CAP_SGI_40);
>
> + /* Allow user to disable SGI-20 (SGI-40 is handled above) */
> + __check_htcap_disable(sdata, ht_cap, IEEE80211_HT_CAP_SGI_20);

And you tested this? And it actually did something, despite
mac80211_ht_capa_mod_mask not including it??

johannes


2012-12-13 20:28:24

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [RFC 2/6] wifi: Don't spam logs with 'Found new beacon' messages.

On Wed, Dec 12, 2012 at 11:24:27PM +0100, Johannes Berg wrote:
> On Wed, 2012-12-12 at 23:20 +0100, Johannes Berg wrote:
> > On Wed, 2012-12-12 at 23:17 +0100, Johannes Berg wrote:
> > > On Wed, 2012-12-12 at 14:15 -0800, Ben Greear wrote:
> > > > On 12/12/2012 02:08 PM, Johannes Berg wrote:
> > > > > On Wed, 2012-12-12 at 13:54 -0800, [email protected] wrote:
> > > > >> From: Ben Greear <[email protected]>
> > > > >>
> > > > >> We saw logs fill with this (at very high speeds):
> > > > >>
> > > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > > >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> > > > >>
> > > > >> Signed-off-by: Ben Greear <[email protected]>
> > > > >> ---
> > > > >> net/wireless/reg.c | 11 ++++++-----
> > > > >> 1 files changed, 6 insertions(+), 5 deletions(-)
> > > > >>
> > > > >> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> > > > >> index 6e53089..8aa7867 100644
> > > > >> --- a/net/wireless/reg.c
> > > > >> +++ b/net/wireless/reg.c
> > > > >> @@ -2067,11 +2067,12 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy,
> > > > >> if (!reg_beacon)
> > > > >> return -ENOMEM;
> > > > >>
> > > > >> - REG_DBG_PRINT("Found new beacon on "
> > > > >
> > > > > Why not just disable the config option for it if you don't care?
> > > >
> > > > I don't mind seeing it, just don't want thousands of them in my logs
> > > > in a very short amount of time. Maybe I was just hitting some
> > > > other bug, and there is no valid reason for this to ever spam?
> > >
> > > Quite likely, since I think it's supposed to happen only once for each
> > > channel unless you reset the regulatory (but that would cause more
> > > messages? I think?)
> >
> > Ah, no, I get it. It is related to your many interfaces :-)
> >
> > The thing is that when a beacon comes in it schedules a work item, and
> > that work item only sets the beacon_found to true. Now since you have a
> > ton of interfaces each beacon can be processed many times, and you get a
> > ton of work items, and ... ouch.

As explained below indeed you should only see this once for one channel
even if your box has a billion interfaces, but Johannes is right that we
won't be able to reap benefits of the check that annotates this was an
already established beacon hint until the work item gets processed and
since its a scheduled task we can potentially alloc quite a lot of beacon
hints for the same channel if you have a lot of interfaces.

I'll post a patch for this, please test.

> Or if you're not world roaming.. this is really bad, we alloc all those
> beacon hint structs just to not do anything with them,

Indeed, we don't annotate the beacon hint as found if we are not world
roaming. I'll look to annotate this, patch to follow after the first one.

> I wonder if they even get leaked or added to reg_beacon_list (but that's
> about the same)

We get beacon hints added to the reg_beacon_list if the channel fits
the area where we allow it given that we allow for future devices to
pop up and we want them to learn from the exisitng device's information.
We can certainly change this if we want.

Luis

2012-12-12 21:56:06

by Ben Greear

[permalink] [raw]
Subject: [RFC 3/6] mac80211: Check for NULL in get-ethtool-stats logic.

From: Ben Greear <[email protected]>

In an earlier version of this code, testing found that sometimes
either sta wasn't found or sta->sdata was NULL. This caused
crashes of course. So, add checks with WARN_ON_ONCE to
catch these.

Signed-off-by: Ben Greear <[email protected]>
---
net/mac80211/cfg.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 5c61677..1e6dcc3 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -599,7 +599,8 @@ static void ieee80211_get_et_stats(struct wiphy *wiphy,
if (sdata->vif.type == NL80211_IFTYPE_STATION) {
sta = sta_info_get_bss(sdata, sdata->u.mgd.bssid);

- if (!(sta && !WARN_ON(sta->sdata->dev != dev)))
+ if (sta == NULL || WARN_ON_ONCE(sta->sdata == NULL)
+ || WARN_ON(sta->sdata->dev != dev))
goto do_survey;

i = 0;
@@ -625,7 +626,8 @@ static void ieee80211_get_et_stats(struct wiphy *wiphy,
} else {
list_for_each_entry(sta, &local->sta_list, list) {
/* Make sure this station belongs to the proper dev */
- if (sta->sdata->dev != dev)
+ if (WARN_ON_ONCE(sta->sdata == NULL) ||
+ sta->sdata->dev != dev)
continue;

i = 0;
--
1.7.3.4


2012-12-12 22:08:11

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC 2/6] wifi: Don't spam logs with 'Found new beacon' messages.

On Wed, 2012-12-12 at 13:54 -0800, [email protected] wrote:
> From: Ben Greear <[email protected]>
>
> We saw logs fill with this (at very high speeds):
>
> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
>
> Signed-off-by: Ben Greear <[email protected]>
> ---
> net/wireless/reg.c | 11 ++++++-----
> 1 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index 6e53089..8aa7867 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -2067,11 +2067,12 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy,
> if (!reg_beacon)
> return -ENOMEM;
>
> - REG_DBG_PRINT("Found new beacon on "

Why not just disable the config option for it if you don't care?

johannes


2012-12-12 22:16:52

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC 2/6] wifi: Don't spam logs with 'Found new beacon' messages.

On Wed, 2012-12-12 at 14:15 -0800, Ben Greear wrote:
> On 12/12/2012 02:08 PM, Johannes Berg wrote:
> > On Wed, 2012-12-12 at 13:54 -0800, [email protected] wrote:
> >> From: Ben Greear <[email protected]>
> >>
> >> We saw logs fill with this (at very high speeds):
> >>
> >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> >> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
> >>
> >> Signed-off-by: Ben Greear <[email protected]>
> >> ---
> >> net/wireless/reg.c | 11 ++++++-----
> >> 1 files changed, 6 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> >> index 6e53089..8aa7867 100644
> >> --- a/net/wireless/reg.c
> >> +++ b/net/wireless/reg.c
> >> @@ -2067,11 +2067,12 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy,
> >> if (!reg_beacon)
> >> return -ENOMEM;
> >>
> >> - REG_DBG_PRINT("Found new beacon on "
> >
> > Why not just disable the config option for it if you don't care?
>
> I don't mind seeing it, just don't want thousands of them in my logs
> in a very short amount of time. Maybe I was just hitting some
> other bug, and there is no valid reason for this to ever spam?

Quite likely, since I think it's supposed to happen only once for each
channel unless you reset the regulatory (but that would cause more
messages? I think?)

johannes


2012-12-12 22:15:29

by Ben Greear

[permalink] [raw]
Subject: Re: [RFC 2/6] wifi: Don't spam logs with 'Found new beacon' messages.

On 12/12/2012 02:08 PM, Johannes Berg wrote:
> On Wed, 2012-12-12 at 13:54 -0800, [email protected] wrote:
>> From: Ben Greear <[email protected]>
>>
>> We saw logs fill with this (at very high speeds):
>>
>> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
>> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
>> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
>> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
>> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
>> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
>> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
>> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
>> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
>> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
>> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
>> cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
>>
>> Signed-off-by: Ben Greear <[email protected]>
>> ---
>> net/wireless/reg.c | 11 ++++++-----
>> 1 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>> index 6e53089..8aa7867 100644
>> --- a/net/wireless/reg.c
>> +++ b/net/wireless/reg.c
>> @@ -2067,11 +2067,12 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy,
>> if (!reg_beacon)
>> return -ENOMEM;
>>
>> - REG_DBG_PRINT("Found new beacon on "
>
> Why not just disable the config option for it if you don't care?

I don't mind seeing it, just don't want thousands of them in my logs
in a very short amount of time. Maybe I was just hitting some
other bug, and there is no valid reason for this to ever spam?

Thanks,
Ben

>
> johannes
>


--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com


2012-12-12 21:56:39

by Ben Greear

[permalink] [raw]
Subject: [RFC 4/6] mac80211: Allow disabling SGI-20.

From: Ben Greear <[email protected]>

This allows user-space (wpa_supplicant) to disable
short guard interval (SGI) for 20Mhz. The SGI-40
disable option is already handled.

Signed-off-by: Ben Greear <[email protected]>
---
net/mac80211/ht.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
index a71d891..8a6d68f 100644
--- a/net/mac80211/ht.c
+++ b/net/mac80211/ht.c
@@ -62,6 +62,9 @@ void ieee80211_apply_htcap_overrides(struct ieee80211_sub_if_data *sdata,
__check_htcap_disable(sdata, ht_cap, IEEE80211_HT_CAP_SUP_WIDTH_20_40);
__check_htcap_disable(sdata, ht_cap, IEEE80211_HT_CAP_SGI_40);

+ /* Allow user to disable SGI-20 (SGI-40 is handled above) */
+ __check_htcap_disable(sdata, ht_cap, IEEE80211_HT_CAP_SGI_20);
+
/* Allow user to disable the max-AMSDU bit. */
__check_htcap_disable(sdata, ht_cap, IEEE80211_HT_CAP_MAX_AMSDU);

--
1.7.3.4


2012-12-12 22:02:30

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC 3/6] mac80211: Check for NULL in get-ethtool-stats logic.

On Wed, 2012-12-12 at 13:54 -0800, [email protected] wrote:
> From: Ben Greear <[email protected]>
>
> In an earlier version of this code, testing found that sometimes
> either sta wasn't found or sta->sdata was NULL. This caused
> crashes of course. So, add checks with WARN_ON_ONCE to
> catch these.

I don't know what you tested, but there's absolutely no way that
sta->sdata is NULL... *Especially* not after sta_info_get_bss(), I mean,
look at that function. But once it's on the list it can't be NULL, so
I'm not going to apply this patch.

In the future, I'd appreciate if you could avoid avoid sending patches
for drivers and mac80211 etc. in one series, it gets confusing with who
applies what etc.

johannes


2012-12-12 21:55:31

by Ben Greear

[permalink] [raw]
Subject: [RFC 2/6] wifi: Don't spam logs with 'Found new beacon' messages.

From: Ben Greear <[email protected]>

We saw logs fill with this (at very high speeds):

cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0
cfg80211: Found new beacon on frequency: 5745 MHz (Ch 149) on wiphy0

Signed-off-by: Ben Greear <[email protected]>
---
net/wireless/reg.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 6e53089..8aa7867 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2067,11 +2067,12 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy,
if (!reg_beacon)
return -ENOMEM;

- REG_DBG_PRINT("Found new beacon on "
- "frequency: %d MHz (Ch %d) on %s\n",
- beacon_chan->center_freq,
- ieee80211_frequency_to_channel(beacon_chan->center_freq),
- wiphy_name(wiphy));
+ if (printk_ratelimit())
+ REG_DBG_PRINT("Found new beacon on "
+ "frequency: %d MHz (Ch %d) on %s\n",
+ beacon_chan->center_freq,
+ ieee80211_frequency_to_channel(beacon_chan->center_freq),
+ wiphy_name(wiphy));

memcpy(&reg_beacon->chan, beacon_chan,
sizeof(struct ieee80211_channel));
--
1.7.3.4


2012-12-12 22:08:32

by Ben Greear

[permalink] [raw]
Subject: Re: [RFC 4/6] mac80211: Allow disabling SGI-20.

On 12/12/2012 02:06 PM, Johannes Berg wrote:
> On Wed, 2012-12-12 at 13:54 -0800, [email protected] wrote:
>> From: Ben Greear <[email protected]>
>>
>> This allows user-space (wpa_supplicant) to disable
>> short guard interval (SGI) for 20Mhz. The SGI-40
>> disable option is already handled.
>>
>> Signed-off-by: Ben Greear <[email protected]>
>> ---
>> net/mac80211/ht.c | 3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
>> index a71d891..8a6d68f 100644
>> --- a/net/mac80211/ht.c
>> +++ b/net/mac80211/ht.c
>> @@ -62,6 +62,9 @@ void ieee80211_apply_htcap_overrides(struct ieee80211_sub_if_data *sdata,
>> __check_htcap_disable(sdata, ht_cap, IEEE80211_HT_CAP_SUP_WIDTH_20_40);
>> __check_htcap_disable(sdata, ht_cap, IEEE80211_HT_CAP_SGI_40);
>>
>> + /* Allow user to disable SGI-20 (SGI-40 is handled above) */
>> + __check_htcap_disable(sdata, ht_cap, IEEE80211_HT_CAP_SGI_20);
>
> And you tested this? And it actually did something, despite
> mac80211_ht_capa_mod_mask not including it??

I thought it was working, but perhaps I was wrong. I will re-test.

Thanks,
Ben

>
> johannes
>


--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com