* Re: [PATCH] ath10k:add support for multicast rate control
2018-04-12 4:04 [PATCH] ath10k:add support for multicast rate control Pradeep Kumar Chitrapu
@ 2018-04-12 8:00 ` Sven Eckelmann
2020-12-21 17:23 ` Kalle Valo
1 sibling, 0 replies; 5+ messages in thread
From: Sven Eckelmann @ 2018-04-12 8:00 UTC (permalink / raw)
To: ath10k; +Cc: Pradeep Kumar Chitrapu, linux-wireless
[-- Attachment #1.1: Type: text/plain, Size: 2293 bytes --]
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, &def))) {
> + band = def.chan->band;
> + sband = &ar->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(&ar->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,
Sven
[-- Attachment #1.2: 9536-ath10k-Allow-to-configure-bcast-mcast-rate.patch --]
[-- Type: text/x-patch, Size: 9561 bytes --]
From: Sven Eckelmann <sven@narfation.org>
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 <sven@narfation.org>
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 <linux/firmware.h>
#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(&ar->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, &mcast_rate);
+ if (ret)
+ return ret;
+
+ mutex_lock(&ar->conf_mutex);
+
+ arvif->mcast_rate = mcast_rate;
+ ret = ath10k_mac_set_mcast_rate(arvif);
+ if (ret)
+ goto unlock;
+
+ ret = count;
+unlock:
+ mutex_unlock(&ar->conf_mutex);
+
+ return ret;
+}
+
+static ssize_t ath10k_read_mcast_rate(struct file *file, char __user *ubuf,
+ size_t count, loff_t *ppos)
+
+{
+ struct ath10k_vif *arvif = file->private_data;
+ struct ath10k *ar = arvif->ar;
+ char buf[32];
+ int len = 0;
+ u16 mcast_rate;
+
+ mutex_lock(&ar->conf_mutex);
+ mcast_rate = arvif->mcast_rate;
+ mutex_unlock(&ar->conf_mutex);
+
+ len = scnprintf(buf, sizeof(buf) - len, "%u\n", mcast_rate);
+
+ return simple_read_from_buffer(ubuf, count, ppos, buf, len);
+}
+
+static const struct file_operations fops_mcast_rate = {
+ .read = ath10k_read_mcast_rate,
+ .write = ath10k_write_mcast_rate,
+ .open = simple_open
+};
+
+int ath10k_debug_register_netdev(struct ath10k_vif *arvif)
+{
+ struct dentry *debugfs_netdev;
+
+ debugfs_netdev = debugfs_create_dir("ath10k", arvif->vif->debugfs_dir);
+ if (IS_ERR_OR_NULL(debugfs_netdev)) {
+ if (IS_ERR(debugfs_netdev))
+ return PTR_ERR(debugfs_netdev);
+
+ return -ENOMEM;
+ }
+
+ debugfs_create_file("mcast_rate", S_IRUGO | S_IWUSR,
+ debugfs_netdev, arvif,
+ &fops_mcast_rate);
+
+ return 0;
+}
+
+void ath10k_debug_unregister_netdev(struct ath10k_vif *arvif)
+{
+}
+
#endif /* CPTCFG_ATH10K_DEBUGFS */
#ifdef CPTCFG_ATH10K_DEBUG
diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h
index 43dbcdbb3e1bb7bb495377f6a6c1d487453a0a0d..24edf6cf15de1bf22c1126e22051b0aad604d6d7 100644
--- a/drivers/net/wireless/ath/ath10k/debug.h
+++ b/drivers/net/wireless/ath/ath10k/debug.h
@@ -77,6 +77,8 @@ int ath10k_debug_create(struct ath10k *ar);
void ath10k_debug_destroy(struct ath10k *ar);
int ath10k_debug_register(struct ath10k *ar);
void ath10k_debug_unregister(struct ath10k *ar);
+int ath10k_debug_register_netdev(struct ath10k_vif *arvif);
+void ath10k_debug_unregister_netdev(struct ath10k_vif *arvif);
void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb);
void ath10k_debug_tpc_stats_process(struct ath10k *ar,
struct ath10k_tpc_stats *tpc_stats);
@@ -134,6 +136,15 @@ static inline void ath10k_debug_unregister(struct ath10k *ar)
{
}
+static inline int ath10k_debug_register_netdev(struct ath10k_vif *arvif)
+{
+ return 0;
+}
+
+static inline void ath10k_debug_unregister_netdev(struct ath10k_vif *arvif)
+{
+}
+
static inline void ath10k_debug_fw_stats_process(struct ath10k *ar,
struct sk_buff *skb)
{
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 938b8c303312ffaccc14c46cdabbb90e80077359..2bea5e8a6e42cacfa289f0e05052a822c591b05c 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -152,6 +152,101 @@ u8 ath10k_mac_bitrate_to_idx(const struct ieee80211_supported_band *sband,
return 0;
}
+int ath10k_mac_set_mcast_rate(struct ath10k_vif *arvif)
+{
+ const struct ieee80211_supported_band *sband;
+ struct ath10k *ar = arvif->ar;
+ struct cfg80211_chan_def def;
+ enum nl80211_band band;
+ u16 best_bitrate = 0;
+ u16 hw_value;
+ u32 ratemask;
+ u8 rate_code;
+ u8 preamble;
+ int i;
+ int ret;
+
+ lockdep_assert_held(&ar->conf_mutex);
+
+ if (ath10k_mac_vif_chan(arvif->vif, &def))
+ return -EPERM;
+
+ band = def.chan->band;
+ sband = ar->hw->wiphy->bands[band];
+ ratemask = arvif->bitrate_mask.control[band].legacy;
+
+ /* it seems that arvif is lost on every fw crash
+ * read the lowest mcast read of bss
+ */
+ if (!arvif->mcast_rate && arvif->vif->bss_conf.mcast_rate[band])
+ arvif->mcast_rate = sband->bitrates[arvif->vif->bss_conf.mcast_rate[band] - 1].bitrate;
+
+ for (i = 0; i < sband->n_bitrates; i++) {
+ if (!(ratemask & BIT(i)))
+ continue;
+
+ if (best_bitrate &&
+ arvif->mcast_rate != sband->bitrates[i].bitrate)
+ continue;
+
+ best_bitrate = sband->bitrates[i].bitrate;
+ hw_value = sband->bitrates[i].hw_value;
+
+ 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;
+ }
+
+ rate_code = ATH10K_HW_RATECODE(hw_value, 0, preamble);
+ }
+
+ if (!best_bitrate) {
+ ath10k_warn(ar, "failed to select multicast rate\n");
+ return -EINVAL;
+ }
+
+ arvif->mcast_rate = best_bitrate;
+
+ ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
+ ar->wmi.vdev_param->mgmt_rate,
+ rate_code);
+ if (ret)
+ ath10k_warn(ar, "failed to set mgmt fixed rate: %d\n",ret);
+
+ ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
+ ar->wmi.vdev_param->bcast_data_rate,
+ rate_code);
+ if (ret)
+ ath10k_warn(ar, "failed to set bcast fixed rate: %d\n",ret);
+
+ ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
+ ar->wmi.vdev_param->mcast_data_rate,
+ rate_code);
+ if (ret)
+ ath10k_warn(ar, "failed to set mcast fixed rate: %d\n",ret);
+
+ return 0;
+}
+
static int ath10k_mac_get_max_vht_mcs_map(u16 mcs_map, int nss)
{
switch ((mcs_map >> (2 * nss)) & 0x3) {
@@ -5133,6 +5228,9 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
spin_unlock_bh(&ar->htt.tx_lock);
mutex_unlock(&ar->conf_mutex);
+
+ ath10k_debug_register_netdev(arvif);
+
return 0;
err_peer_delete:
@@ -5179,6 +5277,8 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
cancel_work_sync(&arvif->ap_csa_work);
cancel_delayed_work_sync(&arvif->connection_loss_work);
+ ath10k_debug_unregister_netdev(arvif);
+
mutex_lock(&ar->conf_mutex);
spin_lock_bh(&ar->data_lock);
@@ -5474,6 +5574,12 @@ static void ath10k_bss_info_changed(struct ieee80211_hw *hw,
arvif->vdev_id, ret);
}
+ /* TODO when should we actually call that */
+ ret = ath10k_mac_set_mcast_rate(arvif);
+ if (ret)
+ ath10k_warn(ar, "failed to set multicast rate params on vdev %i: %d\n",
+ arvif->vdev_id, ret);
+
mutex_unlock(&ar->conf_mutex);
}
@@ -6958,6 +7064,13 @@ static int ath10k_mac_op_set_bitrate_mask(struct ieee80211_hw *hw,
goto exit;
}
+ ret = ath10k_mac_set_mcast_rate(arvif);
+ if (ret) {
+ ath10k_warn(ar, "failed to set multicast rate params on vdev %i: %d\n",
+ arvif->vdev_id, ret);
+ goto exit;
+ }
+
exit:
mutex_unlock(&ar->conf_mutex);
diff --git a/drivers/net/wireless/ath/ath10k/mac.h b/drivers/net/wireless/ath/ath10k/mac.h
index 1bd29ecfcdcc913ff8d3e447eb0d85c4d3c56ec2..db3966028c629b29bfeea3222597e8ba8d203556 100644
--- a/drivers/net/wireless/ath/ath10k/mac.h
+++ b/drivers/net/wireless/ath/ath10k/mac.h
@@ -69,6 +69,7 @@ u8 ath10k_mac_hw_rate_to_idx(const struct ieee80211_supported_band *sband,
u8 hw_rate, bool cck);
u8 ath10k_mac_bitrate_to_idx(const struct ieee80211_supported_band *sband,
u32 bitrate);
+int ath10k_mac_set_mcast_rate(struct ath10k_vif *arvif);
void ath10k_mac_tx_lock(struct ath10k *ar, int reason);
void ath10k_mac_tx_unlock(struct ath10k *ar, int reason);
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 5+ messages in thread