diff mbox series

[3/3] AP: Consider regulatory limitation when filling WMM IE

Message ID 20190403160753.16682-3-andrei.otcheretianski@intel.com
State Accepted
Headers show
Series [1/3] driver: Add regulatory wmm_limit to hostapd_channel_data | expand

Commit Message

Otcheretianski, Andrei April 3, 2019, 4:07 p.m. UTC 
From: Haim Dreyfuss <haim.dreyfuss@intel.com>

In case the current channel has WMM limitation,
take it into account when filling the WMM IE.
Also check if the new WMM IE is different from the previous one and
if so change the parameter_set_count to imply stations to look into it.

Signed-off-by: Haim Dreyfuss <haim.dreyfuss@intel.com>
---
 src/ap/hostapd.h |  3 +++
 src/ap/wmm.c     | 65 ++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 66 insertions(+), 2 deletions(-)

Comments

Jouni Malinen May 28, 2019, 9:11 p.m. UTC | #1 
On Wed, Apr 03, 2019 at 07:07:53PM +0300, Andrei Otcheretianski wrote:
> In case the current channel has WMM limitation,
> take it into account when filling the WMM IE.
> Also check if the new WMM IE is different from the previous one and
> if so change the parameter_set_count to imply stations to look into it.

Thanks, applied full patch set with some cleanup and fixes. In
particular,

> diff --git a/src/ap/wmm.c b/src/ap/wmm.c
> +static void wmm_calc_regulatory_limit(struct hostapd_data *hapd,
> +				      struct hostapd_wmm_ac_params *acp)
> +{
> +	struct hostapd_hw_modes *mode = hapd->iface->current_mode;

mode could be NULL here..

> +	for (c = 0; c < mode->num_channels; c++) {

.. and that would crash the process due to NULL pointer dereference.
This was easy to find with hwsim test cases, so I'd recommend running
those for proposed changes to avoid having to wait for me to hit the
issue when I run the tests before pushing out the commits..
Otcheretianski, Andrei June 5, 2019, 9:07 a.m. UTC | #2 
> 
> Thanks, applied full patch set with some cleanup and fixes. In particular,
> 

Thanks :)

> > diff --git a/src/ap/wmm.c b/src/ap/wmm.c
> > +static void wmm_calc_regulatory_limit(struct hostapd_data *hapd,
> > +				      struct hostapd_wmm_ac_params *acp) {
> > +	struct hostapd_hw_modes *mode = hapd->iface->current_mode;
> 
> mode could be NULL here..
> 
> > +	for (c = 0; c < mode->num_channels; c++) {
> 
> .. and that would crash the process due to NULL pointer dereference.
> This was easy to find with hwsim test cases, so I'd recommend running those for
> proposed changes to avoid having to wait for me to hit the issue when I run the
> tests before pushing out the commits..

Sorry about that, we run the hwsim regularly - still trying to understand how come that I missed that.

Andrei
> 
> --
> Jouni Malinen                                            PGP id EFC895FA
diff mbox series

Patch

diff --git a/src/ap/hostapd.h b/src/ap/hostapd.h
index 790d377548..ff4e247e90 100644
--- a/src/ap/hostapd.h
+++ b/src/ap/hostapd.h
@@ -541,6 +541,9 @@  struct hostapd_iface {
 	unsigned int num_sta_seen;
 
 	u8 dfs_domain;
+
+	/* previous wmm ie */
+	struct hostapd_wmm_ac_params prev_wmm[WMM_AC_NUM];
 };
 
 /* hostapd.c */
diff --git a/src/ap/wmm.c b/src/ap/wmm.c
index 8054c5d2f2..a4a5cab617 100644
--- a/src/ap/wmm.c
+++ b/src/ap/wmm.c
@@ -20,6 +20,12 @@ 
 #include "ap_drv_ops.h"
 #include "wmm.h"
 
+#ifndef MIN
+#define MIN(a, b) (((a) < (b)) ? (a) : (b))
+#endif
+#ifndef MAX
+#define MAX(a, b) (((a) > (b)) ? (a) : (b))
+#endif
 
 static inline u8 wmm_aci_aifsn(int aifsn, int acm, int aci)
 {
@@ -39,6 +45,60 @@  static inline u8 wmm_ecw(int ecwmin, int ecwmax)
 }
 
 
+static void wmm_set_regulatory_limit(struct hostapd_wmm_ac_params *wmm_conf,
+				     struct hostapd_wmm_ac_params *wmm,
+				     struct hostapd_wmm_rule *wmm_reg)
+{
+	int ac;
+
+	for (ac = 0; ac < WMM_AC_NUM; ac++) {
+		wmm[ac].cwmin = MAX(wmm_conf[ac].cwmin, wmm_reg[ac].min_cwmin);
+		wmm[ac].cwmax = MAX(wmm_conf[ac].cwmax, wmm_reg[ac].min_cwmax);
+		wmm[ac].aifs = MAX(wmm_conf[ac].aifs, wmm_reg[ac].min_aifs);
+		wmm[ac].txop_limit =
+			MIN(wmm_conf[ac].txop_limit, wmm_reg[ac].max_txop);
+		wmm[ac].admission_control_mandatory =
+			wmm_conf[ac].admission_control_mandatory;
+	}
+}
+
+/*
+ * Calculate WMM regulatory limit if any.
+ */
+static void wmm_calc_regulatory_limit(struct hostapd_data *hapd,
+				      struct hostapd_wmm_ac_params *acp)
+{
+	struct hostapd_hw_modes *mode = hapd->iface->current_mode;
+	int c;
+
+	for (c = 0; c < mode->num_channels; c++) {
+		struct hostapd_channel_data *chan = &mode->channels[c];
+
+		if (chan->freq != hapd->iface->freq)
+			continue;
+
+		if (chan->wmm_rules_valid)
+			wmm_set_regulatory_limit(hapd->iconf->wmm_ac_params,
+						 acp, chan->wmm_rules);
+		else
+			os_memcpy(acp, hapd->iconf->wmm_ac_params,
+				  sizeof(hapd->iconf->wmm_ac_params));
+		break;
+	}
+
+	/*
+	 * Check if we need to update set count.
+	 * Since both were initialized to zero we can compare
+	 * the whole array in one shot.
+	 */
+	if (os_memcmp(acp, hapd->iface->prev_wmm,
+		      sizeof(hapd->iconf->wmm_ac_params)) != 0) {
+		os_memcpy(hapd->iface->prev_wmm, acp,
+			  sizeof(hapd->iconf->wmm_ac_params));
+		hapd->parameter_set_count++;
+	}
+}
+
 /*
  * Add WMM Parameter Element to Beacon, Probe Response, and (Re)Association
  * Response frames.
@@ -48,10 +108,12 @@  u8 * hostapd_eid_wmm(struct hostapd_data *hapd, u8 *eid)
 	u8 *pos = eid;
 	struct wmm_parameter_element *wmm =
 		(struct wmm_parameter_element *) (pos + 2);
+	struct hostapd_wmm_ac_params wmmp[WMM_AC_NUM] = {0};
 	int e;
 
 	if (!hapd->conf->wmm_enabled)
 		return eid;
+	wmm_calc_regulatory_limit(hapd, wmmp);
 	eid[0] = WLAN_EID_VENDOR_SPECIFIC;
 	wmm->oui[0] = 0x00;
 	wmm->oui[1] = 0x50;
@@ -70,8 +132,7 @@  u8 * hostapd_eid_wmm(struct hostapd_data *hapd, u8 *eid)
 	/* fill in a parameter set record for each AC */
 	for (e = 0; e < 4; e++) {
 		struct wmm_ac_parameter *ac = &wmm->ac[e];
-		struct hostapd_wmm_ac_params *acp =
-			&hapd->iconf->wmm_ac_params[e];
+		struct hostapd_wmm_ac_params *acp = &wmmp[e];
 
 		ac->aci_aifsn = wmm_aci_aifsn(acp->aifs,
 					      acp->admission_control_mandatory,