2020-10-20 09:51:05

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH] rt2x00: save survey for every channel visited

On Mon, Oct 19, 2020 at 07:06:47PM +0000, Марков Михаил Александрович wrote:
> rt2800 only gives you survey for current channel.
<snip>
> .watchdog = rt2800_watchdog,
> + .update_survey = rt2800_update_survey,

Since this feature is rt2800 specific, I would do not add new generic
callback. It could be fully done in rt2800* code, i.e ...

> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
> index 8c6d3099b19d..8eff57132154 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
> @@ -1026,6 +1026,12 @@ static int rt2x00lib_probe_hw_modes(struct rt2x00_dev *rt2x00dev,
> if (!rates)
> goto exit_free_channels;
>
> + rt2x00dev->chan_survey =
> + kcalloc(spec->num_channels, sizeof(struct rt2x00_chan_survey),
> + GFP_KERNEL);
> + if (!rt2x00dev->chan_survey)
> + goto exit_free_rates;
... this could be placed in rt2800_probe_hw_mode() just after
channel info array allocation ...

> @@ -316,6 +316,15 @@ int rt2x00mac_config(struct ieee80211_hw *hw, u32 changed)
> if (!test_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags))
> return 0;
>
> + /*
> + * To provide correct survey data for survey-based ACS algorithm
> + * we have to save survey data for current channel before switching.
> + */
> + if (rt2x00dev->ops->lib->update_survey &&
> + (changed & IEEE80211_CONF_CHANGE_CHANNEL)) {
... and this in rt2800_config()

Thanks
Stanislaw


Subject: [PATCH v2] rt2x00: save survey for every channel visited

rt2800 only gives you survey for current channel.

Survey-based ACS algorithms are failing to perform their job when working
with rt2800.

Make rt2800 save survey for every channel visited and be able to give away
that information.

There is a bug registered https://dev.archive.openwrt.org/ticket/19081 and
this patch solves the issue.

Signed-off-by: Markov Mikhail <[email protected]>

---
Changes are now aggregated in rt2800lib.c.
---
.../net/wireless/ralink/rt2x00/rt2800lib.c | 61 ++++++++++++++-----
drivers/net/wireless/ralink/rt2x00/rt2x00.h | 10 +++
2 files changed, 56 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index a779fe771a55..12a709d99e44 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -1228,6 +1228,17 @@ static int rt2800_check_hung(struct data_queue *queue)
return queue->wd_count > 16;
}

+static void rt2800_update_survey(struct rt2x00_dev *rt2x00dev)
+{
+ struct ieee80211_channel *chan = rt2x00dev->hw->conf.chandef.chan;
+ struct rt2x00_chan_survey *chan_survey =
+ &rt2x00dev->chan_survey[chan->hw_value];
+
+ chan_survey->time_idle += rt2800_register_read(rt2x00dev, CH_IDLE_STA);
+ chan_survey->time_busy += rt2800_register_read(rt2x00dev, CH_BUSY_STA);
+ chan_survey->time_ext_busy += rt2800_register_read(rt2x00dev, CH_BUSY_STA_SEC);
+}
+
void rt2800_watchdog(struct rt2x00_dev *rt2x00dev)
{
struct data_queue *queue;
@@ -1237,6 +1248,8 @@ void rt2800_watchdog(struct rt2x00_dev *rt2x00dev)
if (test_bit(DEVICE_STATE_SCANNING, &rt2x00dev->flags))
return;

+ rt2800_update_survey(rt2x00dev);
+
queue_for_each(rt2x00dev, queue) {
switch (queue->qid) {
case QID_AC_VO:
@@ -5553,6 +5566,12 @@ void rt2800_config(struct rt2x00_dev *rt2x00dev,
rt2800_config_lna_gain(rt2x00dev, libconf);

if (flags & IEEE80211_CONF_CHANGE_CHANNEL) {
+ /*
+ * To provide correct survey data for survey-based ACS algorithm
+ * we have to save survey data for current channel before switching.
+ */
+ rt2800_update_survey(rt2x00dev);
+
rt2800_config_channel(rt2x00dev, libconf->conf,
&libconf->rf, &libconf->channel);
rt2800_config_txpower(rt2x00dev, libconf->conf->chandef.chan,
@@ -10111,12 +10130,20 @@ static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
}

/*
- * Create channel information array
+ * Create channel information and survey arrays
*/
info = kcalloc(spec->num_channels, sizeof(*info), GFP_KERNEL);
if (!info)
return -ENOMEM;

+ rt2x00dev->chan_survey =
+ kcalloc(spec->num_channels, sizeof(struct rt2x00_chan_survey),
+ GFP_KERNEL);
+ if (!rt2x00dev->chan_survey) {
+ kfree(info);
+ return -ENOMEM;
+ }
+
spec->channels_info = info;

default_power1 = rt2800_eeprom_addr(rt2x00dev, EEPROM_TXPOWER_BG1);
@@ -10504,26 +10531,30 @@ int rt2800_get_survey(struct ieee80211_hw *hw, int idx,
{
struct rt2x00_dev *rt2x00dev = hw->priv;
struct ieee80211_conf *conf = &hw->conf;
- u32 idle, busy, busy_ext;
+ struct rt2x00_chan_survey *chan_survey =
+ &rt2x00dev->chan_survey[idx];
+ enum nl80211_band band = NL80211_BAND_2GHZ;

- if (idx != 0)
+ if (idx >= rt2x00dev->bands[band].n_channels) {
+ idx -= rt2x00dev->bands[band].n_channels;
+ band = NL80211_BAND_5GHZ;
+ }
+
+ if (idx >= rt2x00dev->bands[band].n_channels)
return -ENOENT;

- survey->channel = conf->chandef.chan;
+ if (idx == 0)
+ rt2800_update_survey(rt2x00dev);

- idle = rt2800_register_read(rt2x00dev, CH_IDLE_STA);
- busy = rt2800_register_read(rt2x00dev, CH_BUSY_STA);
- busy_ext = rt2800_register_read(rt2x00dev, CH_BUSY_STA_SEC);
+ survey->channel = &rt2x00dev->bands[band].channels[idx];

- if (idle || busy) {
- survey->filled = SURVEY_INFO_TIME |
- SURVEY_INFO_TIME_BUSY |
- SURVEY_INFO_TIME_EXT_BUSY;
+ survey->filled = SURVEY_INFO_TIME |
+ SURVEY_INFO_TIME_BUSY |
+ SURVEY_INFO_TIME_EXT_BUSY;

- survey->time = (idle + busy) / 1000;
- survey->time_busy = busy / 1000;
- survey->time_ext_busy = busy_ext / 1000;
- }
+ survey->time = div_u64(chan_survey->time_idle + chan_survey->time_busy, 1000);
+ survey->time_busy = div_u64(chan_survey->time_busy, 1000);
+ survey->time_ext_busy = div_u64(chan_survey->time_ext_busy, 1000);

if (!(hw->conf.flags & IEEE80211_CONF_OFFCHANNEL))
survey->filled |= SURVEY_INFO_IN_USE;
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00.h b/drivers/net/wireless/ralink/rt2x00/rt2x00.h
index ecc60d8cbb01..ae0c76067c4a 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00.h
@@ -181,6 +181,15 @@ struct rf_channel {
u32 rf4;
};

+/*
+ * Information structure for channel survey.
+ */
+struct rt2x00_chan_survey {
+ u64 time_idle;
+ u64 time_busy;
+ u64 time_ext_busy;
+};
+
/*
* Channel information structure
*/
@@ -752,6 +761,7 @@ struct rt2x00_dev {
*/
struct ieee80211_hw *hw;
struct ieee80211_supported_band bands[NUM_NL80211_BANDS];
+ struct rt2x00_chan_survey *chan_survey;
enum nl80211_band curr_band;
int curr_freq;

--
2.17.1

________________________________________
??: Stanislaw Gruszka <[email protected]>
??????????: 20 ??????? 2020 ?. 12:12
????: ?????? ?????? ?????????????
?????: [email protected]; Helmut Schaa; Kalle Valo; "David S. Miller"; Jakub Kicinski; [email protected]; [email protected]; [email protected]
????: Re: [PATCH] rt2x00: save survey for every channel visited

On Mon, Oct 19, 2020 at 07:06:47PM +0000, ?????? ?????? ????????????? wrote:
> rt2800 only gives you survey for current channel.
<snip>
> .watchdog = rt2800_watchdog,
> + .update_survey = rt2800_update_survey,

Since this feature is rt2800 specific, I would do not add new generic
callback. It could be fully done in rt2800* code, i.e ...

> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
> index 8c6d3099b19d..8eff57132154 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
> @@ -1026,6 +1026,12 @@ static int rt2x00lib_probe_hw_modes(struct rt2x00_dev *rt2x00dev,
> if (!rates)
> goto exit_free_channels;
>
> + rt2x00dev->chan_survey =
> + kcalloc(spec->num_channels, sizeof(struct rt2x00_chan_survey),
> + GFP_KERNEL);
> + if (!rt2x00dev->chan_survey)
> + goto exit_free_rates;
... this could be placed in rt2800_probe_hw_mode() just after
channel info array allocation ...

> @@ -316,6 +316,15 @@ int rt2x00mac_config(struct ieee80211_hw *hw, u32 changed)
> if (!test_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags))
> return 0;
>
> + /*
> + * To provide correct survey data for survey-based ACS algorithm
> + * we have to save survey data for current channel before switching.
> + */
> + if (rt2x00dev->ops->lib->update_survey &&
> + (changed & IEEE80211_CONF_CHANGE_CHANNEL)) {
... and this in rt2800_config()

Thanks
Stanislaw

2020-11-04 11:51:25

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH v2] rt2x00: save survey for every channel visited

On Mon, Nov 02, 2020 at 06:19:32PM +0200, Kalle Valo wrote:
> Марков Михаил Александрович <[email protected]> writes:
>
> > rt2800 only gives you survey for current channel.
> >
> > Survey-based ACS algorithms are failing to perform their job when working
> > with rt2800.
> >
> > Make rt2800 save survey for every channel visited and be able to give away
> > that information.
> >
> > There is a bug registered https://dev.archive.openwrt.org/ticket/19081 and
> > this patch solves the issue.
> >
> > Signed-off-by: Markov Mikhail <[email protected]>
>
> Content-Type: text/plain; charset="koi8-r"
>
> Just so you know I have no idea how patchwork or my scripts handle
> koi8-r. I recommend using utf-8 when submitting patches, but I can
> always try and see what breaks.

I've downloaded the patch from patchwork (using mbox button) and
it applied cleanly for me by 'git am'. So I think there should be
no problems, but if needed I can repost patch using utf-8.

Thanks
Stanislaw