2021-04-22 03:05:56

by Pkshih

[permalink] [raw]
Subject: [PATCH v2 2/2] rtw88: add debugfs to force lowest basic rate

From: Yu-Yen Ting <[email protected]>

The management frame with high rate e.g. 24M may not be transmitted
smoothly in long range environment.
Add a debugfs to force to use the lowest basic rate
in order to debug the reachability of transmitting management frame.

obtain current setting
cat /sys/kernel/debug/ieee80211/phyX/rtw88/basic_rates

force lowest rate:
echo 1 > /sys/kernel/debug/ieee80211/phyX/rtw88/basic_rates

Signed-off-by: Yu-Yen Ting <[email protected]>
Signed-off-by: Ping-Ke Shih <[email protected]>
---
drivers/net/wireless/realtek/rtw88/debug.c | 39 ++++++++++++++++++++++
drivers/net/wireless/realtek/rtw88/main.h | 1 +
drivers/net/wireless/realtek/rtw88/tx.c | 3 +-
3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtw88/debug.c b/drivers/net/wireless/realtek/rtw88/debug.c
index 18ab472ea46c..a99a3dc6f9f9 100644
--- a/drivers/net/wireless/realtek/rtw88/debug.c
+++ b/drivers/net/wireless/realtek/rtw88/debug.c
@@ -861,6 +861,39 @@ static int rtw_debugfs_get_fw_crash(struct seq_file *m, void *v)
return 0;
}

+static ssize_t rtw_debugfs_set_basic_rates(struct file *filp,
+ const char __user *buffer,
+ size_t count, loff_t *loff)
+{
+ struct seq_file *seqpriv = (struct seq_file *)filp->private_data;
+ struct rtw_debugfs_priv *debugfs_priv = seqpriv->private;
+ struct rtw_dev *rtwdev = debugfs_priv->rtwdev;
+ bool input;
+ int err;
+
+ err = kstrtobool_from_user(buffer, count, &input);
+ if (err)
+ return err;
+
+ if (input)
+ set_bit(RTW_FLAG_USE_LOWEST_RATE, rtwdev->flags);
+ else
+ clear_bit(RTW_FLAG_USE_LOWEST_RATE, rtwdev->flags);
+
+ return count;
+}
+
+static int rtw_debugfs_get_basic_rates(struct seq_file *m, void *v)
+{
+ struct rtw_debugfs_priv *debugfs_priv = m->private;
+ struct rtw_dev *rtwdev = debugfs_priv->rtwdev;
+
+ seq_printf(m, "use lowest: %d\n",
+ test_bit(RTW_FLAG_USE_LOWEST_RATE, rtwdev->flags));
+
+ return 0;
+}
+
static ssize_t rtw_debugfs_set_dm_cap(struct file *filp,
const char __user *buffer,
size_t count, loff_t *loff)
@@ -1046,6 +1079,11 @@ static struct rtw_debugfs_priv rtw_debug_priv_fw_crash = {
.cb_read = rtw_debugfs_get_fw_crash,
};

+static struct rtw_debugfs_priv rtw_debug_priv_basic_rates = {
+ .cb_write = rtw_debugfs_set_basic_rates,
+ .cb_read = rtw_debugfs_get_basic_rates,
+};
+
static struct rtw_debugfs_priv rtw_debug_priv_dm_cap = {
.cb_write = rtw_debugfs_set_dm_cap,
.cb_read = rtw_debugfs_get_dm_cap,
@@ -1125,6 +1163,7 @@ void rtw_debugfs_init(struct rtw_dev *rtwdev)
rtw_debugfs_add_r(rf_dump);
rtw_debugfs_add_r(tx_pwr_tbl);
rtw_debugfs_add_rw(fw_crash);
+ rtw_debugfs_add_rw(basic_rates);
rtw_debugfs_add_rw(dm_cap);
}

diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index dc3744847ba9..0df5df3a62ab 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -362,6 +362,7 @@ enum rtw_flags {
RTW_FLAG_BUSY_TRAFFIC,
RTW_FLAG_WOWLAN,
RTW_FLAG_RESTARTING,
+ RTW_FLAG_USE_LOWEST_RATE,

NUM_OF_RTW_FLAGS,
};
diff --git a/drivers/net/wireless/realtek/rtw88/tx.c b/drivers/net/wireless/realtek/rtw88/tx.c
index e5949a775283..0aeed15736c8 100644
--- a/drivers/net/wireless/realtek/rtw88/tx.c
+++ b/drivers/net/wireless/realtek/rtw88/tx.c
@@ -238,8 +238,9 @@ static u8 rtw_get_mgmt_rate(struct rtw_dev *rtwdev, struct sk_buff *skb,
{
struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
struct ieee80211_vif *vif = tx_info->control.vif;
+ bool use_lowest = test_bit(RTW_FLAG_USE_LOWEST_RATE, rtwdev->flags);

- if (!vif || !vif->bss_conf.basic_rates || ignore_rate)
+ if (!vif || !vif->bss_conf.basic_rates || ignore_rate || use_lowest)
return lowest_rate;

return __ffs(vif->bss_conf.basic_rates) + lowest_rate;
--
2.21.0


2021-06-11 23:01:50

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] rtw88: add debugfs to force lowest basic rate

On Thu, Apr 22, 2021 at 11:04:13AM +0800, Ping-Ke Shih wrote:
> From: Yu-Yen Ting <[email protected]>
>
> The management frame with high rate e.g. 24M may not be transmitted
> smoothly in long range environment.
> Add a debugfs to force to use the lowest basic rate
> in order to debug the reachability of transmitting management frame.
>
> obtain current setting
> cat /sys/kernel/debug/ieee80211/phyX/rtw88/basic_rates
>
> force lowest rate:
> echo 1 > /sys/kernel/debug/ieee80211/phyX/rtw88/basic_rates
>
> Signed-off-by: Yu-Yen Ting <[email protected]>
> Signed-off-by: Ping-Ke Shih <[email protected]>

I believe some initial objection to this was because it was unclear if
this is for "production" use (e.g., recommending distros to play with
this) or for debugging. I'll admit, I requested the feature for patch 1,
because I've seen that for those networks where people *do* configure
odd Basic Rates, they intend for stations to follow those, and not use
the lowest (and most airtime-hogging) rates.

And I can say, I don't see why distributions should be turning that back
off. If the Basic Rates setting is wrong, then the that's up to the
network admin to fix.

All that is to say: I agree that this patch is purely for debugging, as
stated, and that it belongs in debugfs. I also maintain a distribution,
and I don't plan on using this beyond debugging.

Therefore:

Reviewed-by: Brian Norris <[email protected]>

BTW, if we have clear guidelines on debugfs, module parameters, etc.,
maybe those should be going on the wiki? I know this came up before:

https://lore.kernel.org/linux-wireless/[email protected]/

At this point, I'm willing to write such guidelines, if I get an ack
from the relevant folks (I guess that's just Kalle?). It probably
belongs somewhere in this tree:

https://wireless.wiki.kernel.org/en/developers/documentation

similar to this:
https://wireless.wiki.kernel.org/en/developers/documentation/nl80211#vendor-specific_api
except it's not really an nl80211 thing. Suggestions welcome.

Side note: it could really use some cleanup -- like this page:
https://wireless.wiki.kernel.org/en/developers/process

Brian