Re: [PATCH] ath10k:add support for multicast rate control

[Kalle Valo]

Pradeep Kumar Chitrapu  wrote:

> Issues wmi command to firmware when multicast rate change is received
> with the new BSS_CHANGED_MCAST_RATE flag.
> 
> Signed-off-by: Pradeep Kumar Chitrapu 

(Cleaning up my backlog, this is from 2018.) I don't understand the
issue from commit log, please clarify and resend if this is still valid.

Patch set to Changes Requested.

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/1523505886-14695-1-git-send-email-prade...@codeaurora.org/

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


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k:add support for multicast rate control

[pradeepc]

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


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k:add support for multicast rate control

[Sven Eckelmann]

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);

[PATCH] ath10k:add support for multicast rate control

[Pradeep Kumar Chitrapu]

Issues wmi command to firmware when multicast rate change is received
with the new BSS_CHANGED_MCAST_RATE flag.

Signed-off-by: Pradeep Kumar Chitrapu 
---
 drivers/net/wireless/ath/ath10k/mac.c | 25 -
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c 
b/drivers/net/wireless/ath/ath10k/mac.c
index bf05a36..63af46f 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -5419,8 +5419,12 @@ static void ath10k_bss_info_changed(struct ieee80211_hw 
*hw,
 {
struct ath10k *ar = hw->priv;
struct ath10k_vif *arvif = (void *)vif->drv_priv;
-   int ret = 0;
+   struct ieee80211_supported_band *sband;
+   struct cfg80211_chan_def def;
u32 vdev_param, pdev_param, slottime, preamble;
+   int rate_index, ret = 0;
+   u8 rate;
+   enum nl80211_band band;
 
mutex_lock(>conf_mutex);
 
@@ -5588,6 +5592,25 @@ static void ath10k_bss_info_changed(struct ieee80211_hw 
*hw,
arvif->vdev_id, ret);
}
 
+   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);
 }
 
-- 
1.9.1


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k