2018-04-12 04:04:51

by Pradeep Kumar Chitrapu

[permalink] [raw]
Subject: [PATCH] ath10k:add support for multicast rate control

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 <[email protected]>
---
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(&ar->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, &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);
}

--
1.9.1


2018-04-17 06:33:18

by Sven Eckelmann

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

On Montag, 16. April 2018 22:10:50 CEST [email protected] 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


Attachments:
signature.asc (833.00 B)
This is a digitally signed message part.

2018-04-17 05:10:51

by Pradeep Kumar Chitrapu

[permalink] [raw]
Subject: 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, &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;
> + }
>

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

2018-04-12 08:00:06

by Sven Eckelmann

[permalink] [raw]
Subject: 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, &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


Attachments:
9536-ath10k-Allow-to-configure-bcast-mcast-rate.patch (9.34 kB)
signature.asc (833.00 B)
This is a digitally signed message part.
Download all attachments

2020-12-21 17:26:59

by Kalle Valo

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

Pradeep Kumar Chitrapu <[email protected]> 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 <[email protected]>

(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/[email protected]/

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