diff mbox

[V4] hostapd: Add Operating Mode Notification support

Message ID 1392036185-4650-1-git-send-email-marek.kwaczynski@tieto.com
State Accepted
Headers show

Commit Message

Marek Kwaczynski Feb. 10, 2014, 12:43 p.m. UTC 
Handling Operating Mode Notification received in Assoc Request.

Signed-hostap: Marek Kwaczynski <marek.kwaczynski@tieto.com>
---
 src/ap/ap_drv_ops.c            |    4 +++-
 src/ap/ap_drv_ops.h            |    2 +-
 src/ap/ieee802_11.c            |    7 ++++++-
 src/ap/ieee802_11.h            |    2 ++
 src/ap/ieee802_11_vht.c        |   27 +++++++++++++++++++++++++++
 src/ap/sta_info.h              |    2 ++
 src/common/ieee802_11_common.c |    5 +++++
 src/common/ieee802_11_common.h |    1 +
 src/common/ieee802_11_defs.h   |    7 +++++++
 src/drivers/driver.h           |    2 ++
 src/drivers/driver_nl80211.c   |    6 ++++++
 11 files changed, 62 insertions(+), 3 deletions(-)

Comments

Jouni Malinen Feb. 14, 2014, 8 p.m. UTC | #1 
On Mon, Feb 10, 2014 at 01:43:05PM +0100, Marek Kwaczynski wrote:
> Handling Operating Mode Notification received in Assoc Request.

Thanks, applied.
Jouni Malinen June 13, 2014, 9:54 p.m. UTC | #2 
On Mon, Feb 10, 2014 at 01:43:05PM +0100, Marek Kwaczynski wrote:
> Handling Operating Mode Notification received in Assoc Request.

> diff --git a/src/ap/ieee802_11_vht.c b/src/ap/ieee802_11_vht.c
> @@ -108,6 +108,33 @@ u16 copy_sta_vht_capab(struct hostapd_data *hapd, struct sta_info *sta,
> +u16 set_sta_vht_opmode(struct hostapd_data *hapd, struct sta_info *sta,
> +		       const u8 *vht_oper_notif)

> +	channel_width = *vht_oper_notif & VHT_OPMODE_CHANNEL_WIDTH_MASK;
> +
> +	if (channel_width != VHT_CHANWIDTH_USE_HT &&
> +	    channel_width != VHT_CHANWIDTH_80MHZ &&
> +	    channel_width != VHT_CHANWIDTH_160MHZ &&
> +	    channel_width != VHT_CHANWIDTH_80P80MHZ &&
> +	    ((*vht_oper_notif & VHT_OPMODE_CHANNEL_RxNSS_MASK) >>
> +	     VHT_OPMODE_NOTIF_RX_NSS_SHIFT) > VHT_RX_NSS_MAX_STREAMS - 1) {
> +		sta->flags &= ~WLAN_STA_VHT_OPMODE_ENABLED;
> +		return WLAN_STATUS_UNSPECIFIED_FAILURE;
> +	}

That last item results in a static analyzer warning due to it being
impossible for it to trigger. With all these conditions being ANDed
together, that failure case cannot be hit.

> diff --git a/src/common/ieee802_11_defs.h b/src/common/ieee802_11_defs.h
> +#define VHT_OPMODE_CHANNEL_RxNSS_MASK		    ((u8) BIT(4) | BIT(5) | BIT(6))
> +#define VHT_OPMODE_NOTIF_RX_NSS_SHIFT		    4

> +#define VHT_RX_NSS_MAX_STREAMS			    8

That results in:

((u8_val & (BIT(4)|BIT(5)|BIT(6))) >> 4) > 8 - 1

The value on the left can be at most: (0xff & 0x70) >> 4 = 0x70 >> 4 = 7
which is obviously never going to be larger than 8 - 1 = 7.

I'm not sure what this is trying to do, but in practice, it does not do
anything. Was that last item supposed to be ORed with the channel_width
checks? If so, it would still not do anything with RxNSS mask, but at
least the channel_width checks would work. Or was this supposed to check
something else?
Jouni Malinen June 17, 2014, 9:44 p.m. UTC | #3 
On Sat, Jun 14, 2014 at 12:54:43AM +0300, Jouni Malinen wrote:
> On Mon, Feb 10, 2014 at 01:43:05PM +0100, Marek Kwaczynski wrote:
> > Handling Operating Mode Notification received in Assoc Request.
> 
> > diff --git a/src/ap/ieee802_11_vht.c b/src/ap/ieee802_11_vht.c
> > @@ -108,6 +108,33 @@ u16 copy_sta_vht_capab(struct hostapd_data *hapd, struct sta_info *sta,
> > +u16 set_sta_vht_opmode(struct hostapd_data *hapd, struct sta_info *sta,
> > +		       const u8 *vht_oper_notif)
> 
> > +	channel_width = *vht_oper_notif & VHT_OPMODE_CHANNEL_WIDTH_MASK;
> > +
> > +	if (channel_width != VHT_CHANWIDTH_USE_HT &&
> > +	    channel_width != VHT_CHANWIDTH_80MHZ &&
> > +	    channel_width != VHT_CHANWIDTH_160MHZ &&
> > +	    channel_width != VHT_CHANWIDTH_80P80MHZ &&
> > +	    ((*vht_oper_notif & VHT_OPMODE_CHANNEL_RxNSS_MASK) >>
> > +	     VHT_OPMODE_NOTIF_RX_NSS_SHIFT) > VHT_RX_NSS_MAX_STREAMS - 1) {
> > +		sta->flags &= ~WLAN_STA_VHT_OPMODE_ENABLED;
> > +		return WLAN_STATUS_UNSPECIFIED_FAILURE;
> > +	}

> I'm not sure what this is trying to do, but in practice, it does not do
> anything. Was that last item supposed to be ORed with the channel_width
> checks? If so, it would still not do anything with RxNSS mask, but at
> least the channel_width checks would work. Or was this supposed to check
> something else?

I cannot make any sense from this. Even that channel width part cannot
be invalid since it is a two-bit field without any way of encoding
something else than the values 0..3. Furthermore, those VHT_CHANWIDTH_*
defines do not match the bits defined for the Channel Width subfield in
the Operating Mode field.

Since this doesn't do anything now and I cannot think of what this could
be trying to do, I'll just delete these lines. If someone can come up
with an explanation of what should be checked here, a new patch to add
such validation can be considered.
Marek Kwaczynski July 4, 2014, 8:15 a.m. UTC | #4 
Hi Jouni,

You are right this condition checking is not necessary:
+   if (channel_width != VHT_CHANWIDTH_USE_HT &&
> > +       channel_width != VHT_CHANWIDTH_80MHZ &&
> > +       channel_width != VHT_CHANWIDTH_160MHZ &&
> > +       channel_width != VHT_CHANWIDTH_80P80MHZ &&
> > +       ((*vht_oper_notif & VHT_OPMODE_CHANNEL_RxNSS_MASK) >>
> > +        VHT_OPMODE_NOTIF_RX_NSS_SHIFT) > VHT_RX_NSS_MAX_STREAMS - 1) {
> > +           sta->flags &= ~WLAN_STA_VHT_OPMODE_ENABLED;
> > +           return WLAN_STATUS_UNSPECIFIED_
FAILURE;
> > +   }


it will be enough:
channel_width = *vht_oper_notif & VHT_OPMODE_CHANNEL_WIDTH_MASK;

I'm sorry I missed it.
I will send a new patch.

Best regards,
Marek Kwaczyński

On 17 June 2014 23:44, Jouni Malinen <j@w1.fi> wrote:
> On Sat, Jun 14, 2014 at 12:54:43AM +0300, Jouni Malinen wrote:
>> On Mon, Feb 10, 2014 at 01:43:05PM +0100, Marek Kwaczynski wrote:
>> > Handling Operating Mode Notification received in Assoc Request.
>>
>> > diff --git a/src/ap/ieee802_11_vht.c b/src/ap/ieee802_11_vht.c
>> > @@ -108,6 +108,33 @@ u16 copy_sta_vht_capab(struct hostapd_data *hapd, struct sta_info *sta,
>> > +u16 set_sta_vht_opmode(struct hostapd_data *hapd, struct sta_info *sta,
>> > +                  const u8 *vht_oper_notif)
>>
>> > +   channel_width = *vht_oper_notif & VHT_OPMODE_CHANNEL_WIDTH_MASK;
>> > +
>> > +   if (channel_width != VHT_CHANWIDTH_USE_HT &&
>> > +       channel_width != VHT_CHANWIDTH_80MHZ &&
>> > +       channel_width != VHT_CHANWIDTH_160MHZ &&
>> > +       channel_width != VHT_CHANWIDTH_80P80MHZ &&
>> > +       ((*vht_oper_notif & VHT_OPMODE_CHANNEL_RxNSS_MASK) >>
>> > +        VHT_OPMODE_NOTIF_RX_NSS_SHIFT) > VHT_RX_NSS_MAX_STREAMS - 1) {
>> > +           sta->flags &= ~WLAN_STA_VHT_OPMODE_ENABLED;
>> > +           return WLAN_STATUS_UNSPECIFIED_FAILURE;
>> > +   }
>
>> I'm not sure what this is trying to do, but in practice, it does not do
>> anything. Was that last item supposed to be ORed with the channel_width
>> checks? If so, it would still not do anything with RxNSS mask, but at
>> least the channel_width checks would work. Or was this supposed to check
>> something else?
>
> I cannot make any sense from this. Even that channel width part cannot
> be invalid since it is a two-bit field without any way of encoding
> something else than the values 0..3. Furthermore, those VHT_CHANWIDTH_*
> defines do not match the bits defined for the Channel Width subfield in
> the Operating Mode field.
>
> Since this doesn't do anything now and I cannot think of what this could
> be trying to do, I'll just delete these lines. If someone can come up
> with an explanation of what should be checked here, a new patch to add
> such validation can be considered.
>
> --
> Jouni Malinen                                            PGP id EFC895FA
diff mbox

Patch

diff --git a/src/ap/ap_drv_ops.c b/src/ap/ap_drv_ops.c
index 893e6d9..e998fc6 100644
--- a/src/ap/ap_drv_ops.c
+++ b/src/ap/ap_drv_ops.c
@@ -346,7 +346,7 @@  int hostapd_sta_add(struct hostapd_data *hapd,
 		    u16 listen_interval,
 		    const struct ieee80211_ht_capabilities *ht_capab,
 		    const struct ieee80211_vht_capabilities *vht_capab,
-		    u32 flags, u8 qosinfo)
+		    u32 flags, u8 qosinfo, u8 vht_opmode)
 {
 	struct hostapd_sta_add_params params;
 
@@ -364,6 +364,8 @@  int hostapd_sta_add(struct hostapd_data *hapd,
 	params.listen_interval = listen_interval;
 	params.ht_capabilities = ht_capab;
 	params.vht_capabilities = vht_capab;
+	params.vht_opmode_enabled = !!(flags & WLAN_STA_VHT_OPMODE_ENABLED);
+	params.vht_opmode = vht_opmode;
 	params.flags = hostapd_sta_flags_to_drv(flags);
 	params.qosinfo = qosinfo;
 	return hapd->driver->sta_add(hapd->drv_priv, &params);
diff --git a/src/ap/ap_drv_ops.h b/src/ap/ap_drv_ops.h
index 15a4b26..9edaf7d 100644
--- a/src/ap/ap_drv_ops.h
+++ b/src/ap/ap_drv_ops.h
@@ -40,7 +40,7 @@  int hostapd_sta_add(struct hostapd_data *hapd,
 		    u16 listen_interval,
 		    const struct ieee80211_ht_capabilities *ht_capab,
 		    const struct ieee80211_vht_capabilities *vht_capab,
-		    u32 flags, u8 qosinfo);
+		    u32 flags, u8 qosinfo, u8 vht_opmode);
 int hostapd_set_privacy(struct hostapd_data *hapd, int enabled);
 int hostapd_set_generic_elem(struct hostapd_data *hapd, const u8 *elem,
 			     size_t elem_len);
diff --git a/src/ap/ieee802_11.c b/src/ap/ieee802_11.c
index dee3c7a..d64af8e 100644
--- a/src/ap/ieee802_11.c
+++ b/src/ap/ieee802_11.c
@@ -910,6 +910,11 @@  static u16 check_assoc_ies(struct hostapd_data *hapd, struct sta_info *sta,
 				  elems.vht_capabilities_len);
 	if (resp != WLAN_STATUS_SUCCESS)
 		return resp;
+
+	resp = set_sta_vht_opmode(hapd, sta, elems.vht_opmode_notif);
+	if (resp != WLAN_STATUS_SUCCESS)
+		return resp;
+
 	if (hapd->iconf->ieee80211ac && hapd->iconf->require_vht &&
 	    !(sta->flags & WLAN_STA_VHT)) {
 		hostapd_logger(hapd, sta->addr, HOSTAPD_MODULE_IEEE80211,
@@ -1953,7 +1958,7 @@  static void handle_assoc_cb(struct hostapd_data *hapd,
 			    sta->listen_interval,
 			    sta->flags & WLAN_STA_HT ? &ht_cap : NULL,
 			    sta->flags & WLAN_STA_VHT ? &vht_cap : NULL,
-			    sta->flags, sta->qosinfo)) {
+			    sta->flags, sta->qosinfo, sta->vht_opmode)) {
 		hostapd_logger(hapd, sta->addr, HOSTAPD_MODULE_IEEE80211,
 			       HOSTAPD_LEVEL_NOTICE,
 			       "Could not add STA to kernel driver");
diff --git a/src/ap/ieee802_11.h b/src/ap/ieee802_11.h
index 5edeb71..6ad35f1 100644
--- a/src/ap/ieee802_11.h
+++ b/src/ap/ieee802_11.h
@@ -62,6 +62,8 @@  u16 copy_sta_ht_capab(struct hostapd_data *hapd, struct sta_info *sta,
 void update_ht_state(struct hostapd_data *hapd, struct sta_info *sta);
 u16 copy_sta_vht_capab(struct hostapd_data *hapd, struct sta_info *sta,
 		       const u8 *vht_capab, size_t vht_capab_len);
+u16 set_sta_vht_opmode(struct hostapd_data *hapd, struct sta_info *sta,
+                       const u8 *vht_opmode);
 void hostapd_tx_status(struct hostapd_data *hapd, const u8 *addr,
 		       const u8 *buf, size_t len, int ack);
 void hostapd_eapol_tx_status(struct hostapd_data *hapd, const u8 *dst,
diff --git a/src/ap/ieee802_11_vht.c b/src/ap/ieee802_11_vht.c
index f2ab182..66fc01f 100644
--- a/src/ap/ieee802_11_vht.c
+++ b/src/ap/ieee802_11_vht.c
@@ -108,6 +108,33 @@  u16 copy_sta_vht_capab(struct hostapd_data *hapd, struct sta_info *sta,
 	return WLAN_STATUS_SUCCESS;
 }
 
+u16 set_sta_vht_opmode(struct hostapd_data *hapd, struct sta_info *sta,
+		       const u8 *vht_oper_notif)
+{
+	u8 channel_width;
+
+	if (!vht_oper_notif) {
+		sta->flags &= ~WLAN_STA_VHT_OPMODE_ENABLED;
+		return WLAN_STATUS_SUCCESS;
+	}
+
+	channel_width = *vht_oper_notif & VHT_OPMODE_CHANNEL_WIDTH_MASK;
+
+	if (channel_width != VHT_CHANWIDTH_USE_HT &&
+	    channel_width != VHT_CHANWIDTH_80MHZ &&
+	    channel_width != VHT_CHANWIDTH_160MHZ &&
+	    channel_width != VHT_CHANWIDTH_80P80MHZ &&
+	    ((*vht_oper_notif & VHT_OPMODE_CHANNEL_RxNSS_MASK) >>
+	     VHT_OPMODE_NOTIF_RX_NSS_SHIFT) > VHT_RX_NSS_MAX_STREAMS - 1) {
+		sta->flags &= ~WLAN_STA_VHT_OPMODE_ENABLED;
+		return WLAN_STATUS_UNSPECIFIED_FAILURE;
+	}
+
+	sta->flags |= WLAN_STA_VHT_OPMODE_ENABLED;
+	sta->vht_opmode = *vht_oper_notif;
+	return WLAN_STATUS_SUCCESS;
+}
+
 void hostapd_get_vht_capab(struct hostapd_data *hapd,
 			   struct ieee80211_vht_capabilities *vht_cap,
 			   struct ieee80211_vht_capabilities *neg_vht_cap)
diff --git a/src/ap/sta_info.h b/src/ap/sta_info.h
index 9b77e06..1973af6 100644
--- a/src/ap/sta_info.h
+++ b/src/ap/sta_info.h
@@ -27,6 +27,7 @@ 
 #define WLAN_STA_GAS BIT(17)
 #define WLAN_STA_VHT BIT(18)
 #define WLAN_STA_WNM_SLEEP_MODE BIT(19)
+#define WLAN_STA_VHT_OPMODE_ENABLED BIT(20)
 #define WLAN_STA_PENDING_DISASSOC_CB BIT(29)
 #define WLAN_STA_PENDING_DEAUTH_CB BIT(30)
 #define WLAN_STA_NONERP BIT(31)
@@ -104,6 +105,7 @@  struct sta_info {
 	struct ieee80211_ht_capabilities *ht_capabilities;
 	struct ieee80211_vht_capabilities *vht_capabilities;
 
+	u8 vht_opmode;
 #ifdef CONFIG_IEEE80211W
 	int sa_query_count; /* number of pending SA Query requests;
 			     * 0 = no SA Query in progress */
diff --git a/src/common/ieee802_11_common.c b/src/common/ieee802_11_common.c
index 809089f..50bdc01 100644
--- a/src/common/ieee802_11_common.c
+++ b/src/common/ieee802_11_common.c
@@ -252,6 +252,11 @@  ParseRes ieee802_11_parse_elems(const u8 *start, size_t len,
 			elems->vht_operation = pos;
 			elems->vht_operation_len = elen;
 			break;
+		case WLAN_EID_VHT_OPERATING_MODE_NOTIFICATION:
+			if (elen != 1)
+				break;
+			elems->vht_opmode_notif = pos;
+			break;
 		case WLAN_EID_LINK_ID:
 			if (elen < 18)
 				break;
diff --git a/src/common/ieee802_11_common.h b/src/common/ieee802_11_common.h
index b84dd9e..4fb2e84 100644
--- a/src/common/ieee802_11_common.h
+++ b/src/common/ieee802_11_common.h
@@ -30,6 +30,7 @@  struct ieee802_11_elems {
 	const u8 *ht_operation;
 	const u8 *vht_capabilities;
 	const u8 *vht_operation;
+	const u8 *vht_opmode_notif;
 	const u8 *vendor_ht_cap;
 	const u8 *p2p;
 	const u8 *wfd;
diff --git a/src/common/ieee802_11_defs.h b/src/common/ieee802_11_defs.h
index 6f7f777..c567017 100644
--- a/src/common/ieee802_11_defs.h
+++ b/src/common/ieee802_11_defs.h
@@ -764,6 +764,13 @@  struct ieee80211_vht_operation {
 #define VHT_CAP_RX_ANTENNA_PATTERN                  ((u32) BIT(28))
 #define VHT_CAP_TX_ANTENNA_PATTERN                  ((u32) BIT(29))
 
+#define VHT_OPMODE_CHANNEL_WIDTH_MASK		    ((u8) BIT(0) | BIT(1))
+#define VHT_OPMODE_CHANNEL_RxNSS_MASK		    ((u8) BIT(4) | BIT(5) | BIT(6))
+#define VHT_OPMODE_NOTIF_RX_NSS_SHIFT		    4
+#define VHT_OPMODE_LEN				    1
+
+#define VHT_RX_NSS_MAX_STREAMS			    8
+
 /* VHT channel widths */
 #define VHT_CHANWIDTH_USE_HT	0
 #define VHT_CHANWIDTH_80MHZ	1
diff --git a/src/drivers/driver.h b/src/drivers/driver.h
index 2f4536e..f4b4672 100644
--- a/src/drivers/driver.h
+++ b/src/drivers/driver.h
@@ -1007,6 +1007,8 @@  struct hostapd_sta_add_params {
 	u16 listen_interval;
 	const struct ieee80211_ht_capabilities *ht_capabilities;
 	const struct ieee80211_vht_capabilities *vht_capabilities;
+	int vht_opmode_enabled;
+	u8 vht_opmode;
 	u32 flags; /* bitmask of WPA_STA_* flags */
 	int set; /* Set STA parameters instead of add */
 	u8 qosinfo;
diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index 81f20b3..20642a1 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -7347,6 +7347,12 @@  static int wpa_driver_nl80211_sta_add(void *priv,
 			params->vht_capabilities);
 	}
 
+	if (params->vht_opmode_enabled) {
+		wpa_printf(MSG_DEBUG, "  * opmode=%u", params->vht_opmode);
+		NLA_PUT_U8(msg, NL80211_ATTR_OPMODE_NOTIF,
+			   params->vht_opmode);
+	}
+
 	wpa_printf(MSG_DEBUG, "  * capability=0x%x", params->capability);
 	NLA_PUT_U16(msg, NL80211_ATTR_STA_CAPABILITY, params->capability);