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

Reply via email to