diff mbox

[1/2] MBO: add option to add MBO query list to ANQP query

Message ID 1488805195-10866-1-git-send-email-andrei.otcheretianski@intel.com
State Superseded
Headers show

Commit Message

Andrei Otcheretianski March 6, 2017, 12:59 p.m. UTC 
From: Avraham Stern <avraham.stern@intel.com>

MBO techspec v0.0_r27 change the MBO ANQP elements format.
The MBO element in ANQP query should now include a MBO query list
element that contains a list of MBO elements to query.

Add API to add the MBO query list to ANQP query. The MBO elements
subtypes should be added as a colon delimited list.

format:
ANQP_GET <bssid> <info_id>,mbo:<MBO element subtype>:...>

example for querying neighbor report with MBO cellular data
connection preference:
ANQP_GET <bssid> 272,mbo:2

Signed-off-by: Avraham Stern <avraham.stern@intel.com>
---
 src/common/ieee802_11_defs.h      |  6 ++++--
 wpa_supplicant/ctrl_iface.c       | 27 ++++++++++++++++++++-------
 wpa_supplicant/interworking.c     |  8 ++++----
 wpa_supplicant/interworking.h     |  4 ++--
 wpa_supplicant/mbo.c              |  9 ++++++---
 wpa_supplicant/wpa_supplicant_i.h |  3 ++-
 6 files changed, 38 insertions(+), 19 deletions(-)

Comments

Jouni Malinen March 7, 2017, 9:59 a.m. UTC | #1 
On Mon, Mar 06, 2017 at 02:59:54PM +0200, Andrei Otcheretianski wrote:
> MBO techspec v0.0_r27 change the MBO ANQP elements format.
> The MBO element in ANQP query should now include a MBO query list
> element that contains a list of MBO elements to query.
> 
> Add API to add the MBO query list to ANQP query. The MBO elements
> subtypes should be added as a colon delimited list.
> 
> format:
> ANQP_GET <bssid> <info_id>,mbo:<MBO element subtype>:...>

This format is not consistent with the existing ANQP_GET cases. Is there
a need for introducing this new syntax and the separate parsing loop for
it in get_anqp() instead of using the existing design and parsing loop?

The current parser uses the following encoding:

ANQP_GET <addr> <info id>[,<info id>]...[,hs20:<subtype>][...,hs20:<subtype>]

For example:
ANQP_GET 00:11:22:33:44:55:66 258,268,hs20:3,hs20:4


Adding MBO to it using the same style (,mbo:2,mbo:3,...) would seem more
consistent and simpler to implement as well and hopefully that would
avoid issues like this:

> diff --git a/wpa_supplicant/ctrl_iface.c b/wpa_supplicant/ctrl_iface.c
> @@ -6712,7 +6712,8 @@ static int get_anqp(struct wpa_supplicant *wpa_s, char *dst)

> +			while (mbo_pos &&
> +			       num_mbo_elems <= MAX_MBO_ANQP_SUBTYPE) {
> +				mbo_elems[num_mbo_elems] = atoi(mbo_pos);
> +				if (mbo_elems[num_mbo_elems] >
> +				    MAX_MBO_ANQP_SUBTYPE) {
> +					wpa_printf(MSG_DEBUG,
> +						   "Invalid MBO ANQP element subtype: %u",
> +						   mbo_elems[num_mbo_elems]);
> +					return -1;
> +				}
> +
> +				num_mbo_elems++;
> +				mbo_pos = os_strchr(mbo_pos, ':');
> +			}

This parser is broken since mbo_pos would be pointing at the separating
colon in the second iteration and atoi(":<int>") returns 0, not <int>..
Andrei Otcheretianski March 7, 2017, 4:37 p.m. UTC | #2 
> 
> This parser is broken since mbo_pos would be pointing at the separating colon in
> the second iteration and atoi(":<int>") returns 0, not <int>..
> 

Oh you are right, sorry about that.
We will send t a fixed version as you suggested.

Thanks,
Andrei
> --
> Jouni Malinen                                            PGP id EFC895FA
diff mbox

Patch

diff --git a/src/common/ieee802_11_defs.h b/src/common/ieee802_11_defs.h
index eecabd9..afe2651 100644
--- a/src/common/ieee802_11_defs.h
+++ b/src/common/ieee802_11_defs.h
@@ -1419,9 +1419,11 @@  enum wfa_wnm_notif_subelem_id {
 	WFA_WNM_NOTIF_SUBELEM_CELL_DATA_CAPA = 3,
 };
 
-/* MBO v0.0_r25, 4.3: MBO ANQP-elements */
+/* MBO v0.0_r27, 4.3: MBO ANQP-elements */
 #define MBO_ANQP_OUI_TYPE 0x12
-#define MBO_ANQP_SUBTYPE_CELL_CONN_PREF 1
+#define MBO_ANQP_SUBTYPE_QUERY_LIST 1
+#define MBO_ANQP_SUBTYPE_CELL_CONN_PREF 2
+#define MAX_MBO_ANQP_SUBTYPE MBO_ANQP_SUBTYPE_CELL_CONN_PREF
 
 /* Wi-Fi Direct (P2P) */
 
diff --git a/wpa_supplicant/ctrl_iface.c b/wpa_supplicant/ctrl_iface.c
index 14dcdcd..079ced3 100644
--- a/wpa_supplicant/ctrl_iface.c
+++ b/wpa_supplicant/ctrl_iface.c
@@ -6712,7 +6712,8 @@  static int get_anqp(struct wpa_supplicant *wpa_s, char *dst)
 	u16 id[MAX_ANQP_INFO_ID];
 	size_t num_id = 0;
 	u32 subtypes = 0;
-	int get_cell_pref = 0;
+	u8 mbo_elems[MAX_MBO_ANQP_SUBTYPE];
+	int num_mbo_elems = 0;
 
 	used = hwaddr_aton2(dst, dst_addr);
 	if (used < 0)
@@ -6732,10 +6733,22 @@  static int get_anqp(struct wpa_supplicant *wpa_s, char *dst)
 #endif /* CONFIG_HS20 */
 		} else if (os_strncmp(pos, "mbo:", 4) == 0) {
 #ifdef CONFIG_MBO
-			int num = atoi(pos + 4);
-			if (num != MBO_ANQP_SUBTYPE_CELL_CONN_PREF)
-				return -1;
-			get_cell_pref = 1;
+			const char *mbo_pos = pos + 4;
+
+			while (mbo_pos &&
+			       num_mbo_elems <= MAX_MBO_ANQP_SUBTYPE) {
+				mbo_elems[num_mbo_elems] = atoi(mbo_pos);
+				if (mbo_elems[num_mbo_elems] >
+				    MAX_MBO_ANQP_SUBTYPE) {
+					wpa_printf(MSG_DEBUG,
+						   "Invalid MBO ANQP element subtype: %u",
+						   mbo_elems[num_mbo_elems]);
+					return -1;
+				}
+
+				num_mbo_elems++;
+				mbo_pos = os_strchr(mbo_pos, ':');
+			}
 #else /* CONFIG_MBO */
 			return -1;
 #endif /* CONFIG_MBO */
@@ -6753,8 +6766,8 @@  static int get_anqp(struct wpa_supplicant *wpa_s, char *dst)
 	if (num_id == 0)
 		return -1;
 
-	return anqp_send_req(wpa_s, dst_addr, id, num_id, subtypes,
-			     get_cell_pref);
+	return anqp_send_req(wpa_s, dst_addr, id, num_id, subtypes, mbo_elems,
+			     num_mbo_elems);
 }
 
 
diff --git a/wpa_supplicant/interworking.c b/wpa_supplicant/interworking.c
index 30b9c88..47e05bc 100644
--- a/wpa_supplicant/interworking.c
+++ b/wpa_supplicant/interworking.c
@@ -2693,8 +2693,8 @@  void interworking_stop_fetch_anqp(struct wpa_supplicant *wpa_s)
 
 
 int anqp_send_req(struct wpa_supplicant *wpa_s, const u8 *dst,
-		  u16 info_ids[], size_t num_ids, u32 subtypes,
-		  int get_cell_pref)
+		  u16 info_ids[], size_t num_ids, u32 subtypes, u8 *mbo_elems,
+		  int num_mbo_elems)
 {
 	struct wpabuf *buf;
 	struct wpabuf *extra_buf = NULL;
@@ -2728,10 +2728,10 @@  int anqp_send_req(struct wpa_supplicant *wpa_s, const u8 *dst,
 #endif /* CONFIG_HS20 */
 
 #ifdef CONFIG_MBO
-	if (get_cell_pref) {
+	if (num_mbo_elems) {
 		struct wpabuf *mbo;
 
-		mbo = mbo_build_anqp_buf(wpa_s, bss);
+		mbo = mbo_build_anqp_buf(wpa_s, bss, mbo_elems, num_mbo_elems);
 		if (mbo) {
 			if (wpabuf_resize(&extra_buf, wpabuf_len(mbo))) {
 				wpabuf_free(extra_buf);
diff --git a/wpa_supplicant/interworking.h b/wpa_supplicant/interworking.h
index 3d22292..0178aec 100644
--- a/wpa_supplicant/interworking.h
+++ b/wpa_supplicant/interworking.h
@@ -12,8 +12,8 @@ 
 enum gas_query_result;
 
 int anqp_send_req(struct wpa_supplicant *wpa_s, const u8 *dst,
-		  u16 info_ids[], size_t num_ids, u32 subtypes,
-		  int get_cell_pref);
+		  u16 info_ids[], size_t num_ids, u32 subtypes, u8 *mbo_elems,
+		  int num_mbo_elems);
 void anqp_resp_cb(void *ctx, const u8 *dst, u8 dialog_token,
 		  enum gas_query_result result,
 		  const struct wpabuf *adv_proto,
diff --git a/wpa_supplicant/mbo.c b/wpa_supplicant/mbo.c
index 3edfd27..e4448d3 100644
--- a/wpa_supplicant/mbo.c
+++ b/wpa_supplicant/mbo.c
@@ -514,7 +514,8 @@  void wpas_mbo_update_cell_capa(struct wpa_supplicant *wpa_s, u8 mbo_cell_capa)
 
 
 struct wpabuf * mbo_build_anqp_buf(struct wpa_supplicant *wpa_s,
-				   struct wpa_bss *bss)
+				   struct wpa_bss *bss, u8 *mbo_elems,
+				   int num_elems)
 {
 	struct wpabuf *anqp_buf;
 	u8 *len_pos;
@@ -526,7 +527,7 @@  struct wpabuf * mbo_build_anqp_buf(struct wpa_supplicant *wpa_s,
 		return NULL;
 	}
 
-	anqp_buf = wpabuf_alloc(10);
+	anqp_buf = wpabuf_alloc(9 + num_elems);
 	if (!anqp_buf)
 		return NULL;
 
@@ -534,7 +535,9 @@  struct wpabuf * mbo_build_anqp_buf(struct wpa_supplicant *wpa_s,
 	wpabuf_put_be24(anqp_buf, OUI_WFA);
 	wpabuf_put_u8(anqp_buf, MBO_ANQP_OUI_TYPE);
 
-	wpabuf_put_u8(anqp_buf, MBO_ANQP_SUBTYPE_CELL_CONN_PREF);
+	wpabuf_put_u8(anqp_buf, MBO_ANQP_SUBTYPE_QUERY_LIST);
+	wpabuf_put_data(anqp_buf, mbo_elems, num_elems);
+
 	gas_anqp_set_element_len(anqp_buf, len_pos);
 
 	return anqp_buf;
diff --git a/wpa_supplicant/wpa_supplicant_i.h b/wpa_supplicant/wpa_supplicant_i.h
index 95e19b8..3e1767c 100644
--- a/wpa_supplicant/wpa_supplicant_i.h
+++ b/wpa_supplicant/wpa_supplicant_i.h
@@ -1290,7 +1290,8 @@  size_t wpas_mbo_ie_bss_trans_reject(struct wpa_supplicant *wpa_s, u8 *pos,
 				    enum mbo_transition_reject_reason reason);
 void wpas_mbo_update_cell_capa(struct wpa_supplicant *wpa_s, u8 mbo_cell_capa);
 struct wpabuf * mbo_build_anqp_buf(struct wpa_supplicant *wpa_s,
-				   struct wpa_bss *bss);
+				   struct wpa_bss *bss, u8 *mbo_elems,
+				   int num_mbo_elems);
 
 /* op_classes.c */
 enum chan_allowed {