Re: [PATCH] ath10k:add support for multicast rate control
On Montag, 16. April 2018 22:10:50 CEST prade...@codeaurora.org wrote: [...] > > I see two major problems here without checking the actual > > implementation > > details: > > > > * hw_value is incorrect for a couple of devices. Some devices use a > > different > > mapping when they receive rates inforamtion (hw_value) then the ones > > you use > > for the mcast/bcast/beacon rate setting. I've handled in my POC patch > > like [...] > Isn't this already fixed in https://patchwork.kernel.org/patch/9150145/ > ? No, this is exactly the patch which causes the problem for you. The parameter for these settings stayed the same but this patch changes the order in ieee80211_rate to make it easier for the driver to parse the (reordered) rx status information for CCK rates (maybe also tx - not sure about that right now). > > * bcast + mcast (+ mgmt) have to be set separately > > Can you please let me know why this would be necessary? > Although I see minstrel rate control is currently seems using the > mcast_rate > setting to set MGMT/BCAST/MCAST rates, will this not be misleading to > user > passed value with 'iw set mcast_rate' for MGMT traffic as well? What about broadcast? MGMT was only mentioned here because it is the third item which can be manipulated the same way. But yes, it would be good to get the mgmt rates in sync with the non-hw-rate-control drivers. Kind regards, Sven signature.asc Description: This is a digitally signed message part.
Re: [PATCH] ath10k:add support for multicast rate control
On 2018-04-12 01:00, Sven Eckelmann wrote:
On Mittwoch, 11. April 2018 21:04:46 CEST Pradeep Kumar Chitrapu wrote:
Issues wmi command to firmware when multicast rate change is received
with the new BSS_CHANGED_MCAST_RATE flag.
[...]
+ if (changed & BSS_CHANGED_MCAST_RATE &&
+ !WARN_ON(ath10k_mac_vif_chan(arvif->vif, ))) {
+ band = def.chan->band;
+ sband = >mac.sbands[band];
+ vdev_param = ar->wmi.vdev_param->mcast_data_rate;
+ rate_index = vif->bss_conf.mcast_rate[band] - 1;
+ rate = ATH10K_HW_MCS_RATE(sband->bitrates[rate_index].hw_value);
+ ath10k_dbg(ar, ATH10K_DBG_MAC,
+ "mac vdev %d mcast_rate %d\n",
+ arvif->vdev_id, rate);
+
+ ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
+ vdev_param, rate);
+ if (ret)
+ ath10k_warn(ar,
+ "failed to set mcast rate on vdev"
+ " %i: %d\n", arvif->vdev_id, ret);
+ }
+
mutex_unlock(>conf_mutex);
}
I see two major problems here without checking the actual
implementation
details:
* hw_value is incorrect for a couple of devices. Some devices use a
different
mapping when they receive rates inforamtion (hw_value) then the ones
you use
for the mcast/bcast/beacon rate setting. I've handled in my POC patch
like
this:
+ if (ath10k_mac_bitrate_is_cck(sband->bitrates[i].bitrate)) {
+ preamble = WMI_RATE_PREAMBLE_CCK;
+
+ /* QCA didn't use the correct rate values for CA99x0
+* and above (ath10k_g_rates_rev2)
+*/
+ switch (sband->bitrates[i].bitrate) {
+ case 10:
+ hw_value = ATH10K_HW_RATE_CCK_LP_1M;
+ break;
+ case 20:
+ hw_value = ATH10K_HW_RATE_CCK_LP_2M;
+ break;
+ case 55:
+ hw_value = ATH10K_HW_RATE_CCK_LP_5_5M;
+ break;
+ case 110:
+ hw_value = ATH10K_HW_RATE_CCK_LP_11M;
+ break;
+ }
+ } else {
+ preamble = WMI_RATE_PREAMBLE_OFDM;
+ }
Isn't this already fixed in https://patchwork.kernel.org/patch/9150145/
?
* bcast + mcast (+ mgmt) have to be set separately
Can you please let me know why this would be necessary?
Although I see minstrel rate control is currently seems using the
mcast_rate
setting to set MGMT/BCAST/MCAST rates, will this not be misleading to
user
passed value with 'iw set mcast_rate' for MGMT traffic as well?
I have attached my POC patch (which I was using for packet loss based
mesh
metrics) and to work around (using debugfs) the silly mgmt vs.
mcast/bcast
settings of the QCA fw for APs.
Many of the information came from Ben Greears ath10k-ct driver
https://github.com/greearb/ath10k-ct
Kind regards,
Sven
Re: [PATCH] ath10k:add support for multicast rate control
On Mittwoch, 11. April 2018 21:04:46 CEST Pradeep Kumar Chitrapu wrote:
> Issues wmi command to firmware when multicast rate change is received
> with the new BSS_CHANGED_MCAST_RATE flag.
[...]
>
> + if (changed & BSS_CHANGED_MCAST_RATE &&
> + !WARN_ON(ath10k_mac_vif_chan(arvif->vif, ))) {
> + band = def.chan->band;
> + sband = >mac.sbands[band];
> + vdev_param = ar->wmi.vdev_param->mcast_data_rate;
> + rate_index = vif->bss_conf.mcast_rate[band] - 1;
> + rate = ATH10K_HW_MCS_RATE(sband->bitrates[rate_index].hw_value);
> + ath10k_dbg(ar, ATH10K_DBG_MAC,
> +"mac vdev %d mcast_rate %d\n",
> +arvif->vdev_id, rate);
> +
> + ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
> + vdev_param, rate);
> + if (ret)
> + ath10k_warn(ar,
> + "failed to set mcast rate on vdev"
> + " %i: %d\n", arvif->vdev_id, ret);
> + }
> +
> mutex_unlock(>conf_mutex);
> }
>
>
I see two major problems here without checking the actual implementation
details:
* hw_value is incorrect for a couple of devices. Some devices use a different
mapping when they receive rates inforamtion (hw_value) then the ones you use
for the mcast/bcast/beacon rate setting. I've handled in my POC patch like
this:
+ if (ath10k_mac_bitrate_is_cck(sband->bitrates[i].bitrate)) {
+ preamble = WMI_RATE_PREAMBLE_CCK;
+
+ /* QCA didn't use the correct rate values for CA99x0
+* and above (ath10k_g_rates_rev2)
+*/
+ switch (sband->bitrates[i].bitrate) {
+ case 10:
+ hw_value = ATH10K_HW_RATE_CCK_LP_1M;
+ break;
+ case 20:
+ hw_value = ATH10K_HW_RATE_CCK_LP_2M;
+ break;
+ case 55:
+ hw_value = ATH10K_HW_RATE_CCK_LP_5_5M;
+ break;
+ case 110:
+ hw_value = ATH10K_HW_RATE_CCK_LP_11M;
+ break;
+ }
+ } else {
+ preamble = WMI_RATE_PREAMBLE_OFDM;
+ }
* bcast + mcast (+ mgmt) have to be set separately
I have attached my POC patch (which I was using for packet loss based mesh
metrics) and to work around (using debugfs) the silly mgmt vs. mcast/bcast
settings of the QCA fw for APs.
Many of the information came from Ben Greears ath10k-ct driver
https://github.com/greearb/ath10k-ct
Kind regards,
SvenFrom: Sven Eckelmann
Date: Fri, 16 Feb 2018 13:49:51 +0100
Subject: [PATCH] ath10k: Allow to configure bcast/mcast rate
TODO:
* find better way to get mcast_rate
* better get the lowest configured basic rate for APs
* remove netifd WAR
Signed-off-by: Sven Eckelmann
Forwarded: no
not yet in the correct shape
---
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/debug.c | 78 ++
drivers/net/wireless/ath/ath10k/debug.h | 11
drivers/net/wireless/ath/ath10k/mac.c | 113
drivers/net/wireless/ath/ath10k/mac.h | 1 +
5 files changed, 204 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 3ff4a423c4d873d322deebd3c498ef0688e84e05..a53f7b2042f4a80bbd358b00d4d26d6fabe5df6e 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -423,6 +423,7 @@ struct ath10k_vif {
bool nohwcrypt;
int num_legacy_stations;
int txpower;
+ u16 mcast_rate;
struct wmi_wmm_params_all_arg wmm_params;
struct work_struct ap_csa_work;
struct delayed_work connection_loss_work;
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index fa72ef54605e74f501ab655a6afd0a50b89b6b7e..fc59f214d2a5288f2ac41824e584def787b4799c 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -23,6 +23,7 @@
#include
#include "core.h"
+#include "mac.h"
#include "debug.h"
#include "hif.h"
#include "wmi-ops.h"
@@ -2454,6 +2455,83 @@ void ath10k_debug_unregister(struct ath10k *ar)
cancel_delayed_work_sync(>debug.htt_stats_dwork);
}
+
+
+static ssize_t ath10k_write_mcast_rate(struct file *file,
+ const char __user *ubuf,
+ size_t count, loff_t *ppos)
+{
+ struct ath10k_vif *arvif = file->private_data;
+ struct ath10k *ar = arvif->ar;
+ ssize_t ret = 0;
+ u32 mcast_rate;
+
+ ret = kstrtou32_from_user(ubuf, count, 0, _rate);
