RE: [PATCH v4] cfg80211: Add support to configure a beacon data rate

[Kushwaha, Purushottam]

> But this is where I got confused - this seems wrong, you should be checking 
> the hweight8() of each of the ht_mcs[i] values?
Thanks for pointing this. I will send new patchset as per your suggestion. 

Regards,
Purushottam

Re: [PATCH v4] cfg80211: Add support to configure a beacon data rate

[Johannes Berg]

Hi,

Thanks for the new version. I was going to apply it but while changing
something small - see below - found what I think is another issue?

> +static int validate_beacon_tx_rate(struct cfg80211_ap_settings
> *params)
> +{
> + u32 rate, count_ht, count_vht, i;
> + enum nl80211_band band;
> +
> + band = params->chandef.chan->band;
> + rate = params->beacon_rate.control[band].legacy;
> +
> + /* Allow only one rate */
> + if (rate && (rate & (rate - 1)))
> + return -EINVAL;

I was going to change that to just
if (hweight32(rate) > 1)
return -EINVAL;

I realize that your code is equivalent, but I doubt that we need to be
really efficient here, and IMHO hweight32() is easier to understand.

> + count_ht = 0;
> + for (i = 0; i < IEEE80211_HT_MCS_MASK_LEN; i++) {
> + if (params->beacon_rate.control[band].ht_mcs[i]) {
> + count_ht++;
> + if (count_ht > 1)
> + return -EINVAL;
> + }
> + }

But this is where I got confused - this seems wrong, you should be
checking the hweight8() of each of the ht_mcs[i] values?

Similarly, the VHT one, but with hweight16() of course, no?

I was going to move a "if (rate) return -EINVAL;" check into this and
the VHT loop, so that we only need the count_ht && count_vht and the
"all empty" portion of this:

> + if ((rate && count_ht) ||
> + (rate && count_vht) ||
> + (count_ht && count_vht) ||
> + (!rate && !count_ht && !count_vht))
> + return -EINVAL;
> +
> + return 0;
> +}


johannes

[PATCH v4] cfg80211: Add support to configure a beacon data rate

[Purushottam Kushwaha]

This allows an option to configure a single beacon tx rate (u8) for an AP.

Signed-off-by: Purushottam Kushwaha 
---
 include/net/cfg80211.h |  25 +--
 net/wireless/nl80211.c | 504 +++--
 2 files changed, 296 insertions(+), 233 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index d5e7f69..c58afc8 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -676,6 +676,18 @@ struct cfg80211_acl_data {
struct mac_address mac_addrs[];
 };
 
+/*
+ * cfg80211_bitrate_mask - masks for bitrate control
+ */
+struct cfg80211_bitrate_mask {
+   struct {
+   u32 legacy;
+   u8 ht_mcs[IEEE80211_HT_MCS_MASK_LEN];
+   u16 vht_mcs[NL80211_VHT_NSS_MAX];
+   enum nl80211_txrate_gi gi;
+   } control[NUM_NL80211_BANDS];
+};
+
 /**
  * struct cfg80211_ap_settings - AP configuration
  *
@@ -700,6 +712,7 @@ struct cfg80211_acl_data {
  * MAC address based access control
  * @pbss: If set, start as a PCP instead of AP. Relevant for DMG
  * networks.
+ * @beacon_rate: masks for setting user configured beacon tx rate.
  */
 struct cfg80211_ap_settings {
struct cfg80211_chan_def chandef;
@@ -719,6 +732,7 @@ struct cfg80211_ap_settings {
bool p2p_opp_ps;
const struct cfg80211_acl_data *acl;
bool pbss;
+   struct cfg80211_bitrate_mask beacon_rate;
 };
 
 /**
@@ -2001,17 +2015,6 @@ enum wiphy_params_flags {
WIPHY_PARAM_DYN_ACK = 1 << 5,
 };
 
-/*
- * cfg80211_bitrate_mask - masks for bitrate control
- */
-struct cfg80211_bitrate_mask {
-   struct {
-   u32 legacy;
-   u8 ht_mcs[IEEE80211_HT_MCS_MASK_LEN];
-   u16 vht_mcs[NL80211_VHT_NSS_MAX];
-   enum nl80211_txrate_gi gi;
-   } control[NUM_NL80211_BANDS];
-};
 /**
  * struct cfg80211_pmksa - PMK Security Association
  *
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 7ebad35..f49b7df 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3324,6 +3324,274 @@ static int nl80211_set_mac_acl(struct sk_buff *skb, 
struct genl_info *info)
return err;
 }
 
+static u32 rateset_to_mask(struct ieee80211_supported_band *sband,
+  u8 *rates, u8 rates_len)
+{
+   u8 i;
+   u32 mask = 0;
+
+   for (i = 0; i < rates_len; i++) {
+   int rate = (rates[i] & 0x7f) * 5;
+   int ridx;
+
+   for (ridx = 0; ridx < sband->n_bitrates; ridx++) {
+   struct ieee80211_rate *srate =
+   &sband->bitrates[ridx];
+   if (rate == srate->bitrate) {
+   mask |= 1 << ridx;
+   break;
+   }
+   }
+   if (ridx == sband->n_bitrates)
+   return 0; /* rate not found */
+   }
+
+   return mask;
+}
+
+static bool ht_rateset_to_mask(struct ieee80211_supported_band *sband,
+  u8 *rates, u8 rates_len,
+  u8 mcs[IEEE80211_HT_MCS_MASK_LEN])
+{
+   u8 i;
+
+   memset(mcs, 0, IEEE80211_HT_MCS_MASK_LEN);
+
+   for (i = 0; i < rates_len; i++) {
+   int ridx, rbit;
+
+   ridx = rates[i] / 8;
+   rbit = BIT(rates[i] % 8);
+
+   /* check validity */
+   if ((ridx < 0) || (ridx >= IEEE80211_HT_MCS_MASK_LEN))
+   return false;
+
+   /* check availability */
+   if (sband->ht_cap.mcs.rx_mask[ridx] & rbit)
+   mcs[ridx] |= rbit;
+   else
+   return false;
+   }
+
+   return true;
+}
+
+static u16 vht_mcs_map_to_mcs_mask(u8 vht_mcs_map)
+{
+   u16 mcs_mask = 0;
+
+   switch (vht_mcs_map) {
+   case IEEE80211_VHT_MCS_NOT_SUPPORTED:
+   break;
+   case IEEE80211_VHT_MCS_SUPPORT_0_7:
+   mcs_mask = 0x00FF;
+   break;
+   case IEEE80211_VHT_MCS_SUPPORT_0_8:
+   mcs_mask = 0x01FF;
+   break;
+   case IEEE80211_VHT_MCS_SUPPORT_0_9:
+   mcs_mask = 0x03FF;
+   break;
+   default:
+   break;
+   }
+
+   return mcs_mask;
+}
+
+static void vht_build_mcs_mask(u16 vht_mcs_map,
+  u16 vht_mcs_mask[NL80211_VHT_NSS_MAX])
+{
+   u8 nss;
+
+   for (nss = 0; nss < NL80211_VHT_NSS_MAX; nss++) {
+   vht_mcs_mask[nss] = vht_mcs_map_to_mcs_mask(vht_mcs_map & 0x03);
+   vht_mcs_map >>= 2;
+   }
+}
+
+static bool vht_set_mcs_mask(struct ieee80211_supported_band *sband,
+struct nl80211_txrate_vht *txrate,
+u16 mcs[NL80211_VHT_NSS_MAX])
+{
+   u16 tx_mcs_map = le16_to_cpu(sband->vht_cap.vht_mcs.tx_mcs_map);
+   u16 tx_mcs_mas