diff mbox

P2P: Fix setting P2P Client Discoverability bit

Message ID CAFk-A4mfokSYZc9SDMLPryny_kuQuO-_MrhmwtOw7MbGN_P9Sg@mail.gmail.com
State Accepted
Commit 18485b5469c5eeea6a552264fbfaabb089a0a557
Headers show

Commit Message

Masashi Honma June 13, 2012, 1:18 p.m. UTC 
Hello.

In the [1], the P2P Client Discoverability bit is described in "Table 12 Device
Capability Bitmap definition". The table says "Within a P2P Group Info
attribute and a (Re)association request frame the P2P Client Discoverability
field shall be set to 1 when the P2P Device supports P2P Client Discoverability,
and is set to 0 otherwise. This field shall be reserved and set to 0 in all
other frames or uses.". So I made this patch to make it suitable for the spec.

[1] Wi-Fi Peer-to-Peer (P2P) Technical Specification Version 1.1

Signed-hostap: Masashi Honma <masashi.honma@gmail.com>

 		p2p_buf_add_group_id(buf, go->info.p2p_device_addr,

Regards,
Masashi Honma.

Comments

Jouni Malinen June 16, 2012, 5:24 p.m. UTC | #1 
On Wed, Jun 13, 2012 at 10:18:20PM +0900, Masashi Honma wrote:
> In the [1], the P2P Client Discoverability bit is described in "Table 12 Device
> Capability Bitmap definition". The table says "Within a P2P Group Info
> attribute and a (Re)association request frame the P2P Client Discoverability
> field shall be set to 1 when the P2P Device supports P2P Client Discoverability,
> and is set to 0 otherwise. This field shall be reserved and set to 0 in all
> other frames or uses.". So I made this patch to make it suitable for the spec.

Thanks! Applied. Though, this triggers a bug where device
discoverability cannot be used in some cases due to how this dev_capab
bit is learned. I fixed that with commit
f33bc035824a39017a25bedd7017a3ddf6bec866.
Masashi Honma June 18, 2012, 12:02 p.m. UTC | #2 
Thanks for a P2P Client Discoverability bit updation patch. I have a
question for the patch.

http://hostap.epitest.fi/gitweb/gitweb.cgi?p=hostap.git;a=commitdiff;h=f33bc035824a39017a25bedd7017a3ddf6bec866

At p2p_add_group_clients() an else caluse was added for the sentence below.
	if (dev->flags & (P2P_DEV_GROUP_CLIENT_ONLY | P2P_DEV_PROBE_REQ_ONLY))

But I could not make a test case which uses the else clause.

I have thought the P2P_DEV_GROUP_CLIENT_ONLY bit in the dev->flags is
always enabled. Because the device is in the P2P Group Info Attribute.

What case do you expect about the else clause ?


2012/6/17 Jouni Malinen <j@w1.fi>:
> On Wed, Jun 13, 2012 at 10:18:20PM +0900, Masashi Honma wrote:
>> In the [1], the P2P Client Discoverability bit is described in "Table 12 Device
>> Capability Bitmap definition". The table says "Within a P2P Group Info
>> attribute and a (Re)association request frame the P2P Client Discoverability
>> field shall be set to 1 when the P2P Device supports P2P Client Discoverability,
>> and is set to 0 otherwise. This field shall be reserved and set to 0 in all
>> other frames or uses.". So I made this patch to make it suitable for the spec.
>
> Thanks! Applied. Though, this triggers a bug where device
> discoverability cannot be used in some cases due to how this dev_capab
> bit is learned. I fixed that with commit
> f33bc035824a39017a25bedd7017a3ddf6bec866.
>
> --
> Jouni Malinen                                            PGP id EFC895FA
> _______________________________________________
> HostAP mailing list
> HostAP@lists.shmoo.com
> http://lists.shmoo.com/mailman/listinfo/hostap
Masashi Honma June 20, 2012, 1:07 p.m. UTC | #3 
> At p2p_add_group_clients() an else caluse was added for the sentence below.
> 	if (dev->flags & (P2P_DEV_GROUP_CLIENT_ONLY | P2P_DEV_PROBE_REQ_ONLY))
> But I could not make a test case which uses the else clause.
> I have thought the P2P_DEV_GROUP_CLIENT_ONLY bit in the dev->flags is
> always enabled. Because the device is in the P2P Group Info Attribute.
> What case do you expect about the else clause ?

This was solved.
diff mbox

Patch

diff --git a/src/p2p/p2p.c b/src/p2p/p2p.c
index 7046f7b..33afe55 100644
--- a/src/p2p/p2p.c
+++ b/src/p2p/p2p.c
@@ -1815,7 +1815,8 @@  struct wpabuf * p2p_build_probe_resp_ies(struct
p2p_data *p2p)

 	/* P2P IE */
 	len = p2p_buf_add_ie_hdr(buf);
-	p2p_buf_add_capability(buf, p2p->dev_capab, 0);
+	p2p_buf_add_capability(buf,
+		p2p->dev_capab & ~P2P_DEV_CAPAB_CLIENT_DISCOVERABILITY, 0);
 	if (p2p->ext_listen_interval)
 		p2p_buf_add_ext_listen_timing(buf, p2p->ext_listen_period,
 					      p2p->ext_listen_interval);
@@ -2662,7 +2663,8 @@  void p2p_scan_res_handled(struct p2p_data *p2p)
 void p2p_scan_ie(struct p2p_data *p2p, struct wpabuf *ies, const u8 *dev_id)
 {
 	u8 *len = p2p_buf_add_ie_hdr(ies);
-	p2p_buf_add_capability(ies, p2p->dev_capab, 0);
+	p2p_buf_add_capability(ies,
+		p2p->dev_capab & ~P2P_DEV_CAPAB_CLIENT_DISCOVERABILITY, 0);
 	if (dev_id)
 		p2p_buf_add_device_id(ies, dev_id);
 	if (p2p->cfg->reg_class && p2p->cfg->channel)
diff --git a/src/p2p/p2p_go_neg.c b/src/p2p/p2p_go_neg.c
index 766dd6d..cccf650 100644
--- a/src/p2p/p2p_go_neg.c
+++ b/src/p2p/p2p_go_neg.c
@@ -155,7 +155,9 @@  static struct wpabuf * p2p_build_go_neg_req(struct
p2p_data *p2p,
 		group_capab |= P2P_GROUP_CAPAB_CROSS_CONN;
 	if (p2p->cfg->p2p_intra_bss)
 		group_capab |= P2P_GROUP_CAPAB_INTRA_BSS_DIST;
-	p2p_buf_add_capability(buf, p2p->dev_capab, group_capab);
+	p2p_buf_add_capability(buf,
+		p2p->dev_capab & ~P2P_DEV_CAPAB_CLIENT_DISCOVERABILITY,
+		group_capab);
 	p2p_buf_add_go_intent(buf, (p2p->go_intent << 1) |
 			      p2p->next_tie_breaker);
 	p2p->next_tie_breaker = !p2p->next_tie_breaker;
@@ -268,7 +270,9 @@  static struct wpabuf *
p2p_build_go_neg_resp(struct p2p_data *p2p,
 		if (p2p->cfg->p2p_intra_bss)
 			group_capab |= P2P_GROUP_CAPAB_INTRA_BSS_DIST;
 	}
-	p2p_buf_add_capability(buf, p2p->dev_capab, group_capab);
+	p2p_buf_add_capability(buf,
+		p2p->dev_capab & ~P2P_DEV_CAPAB_CLIENT_DISCOVERABILITY,
+		group_capab);
 	p2p_buf_add_go_intent(buf, (p2p->go_intent << 1) | tie_breaker);
 	p2p_buf_add_config_timeout(buf, 100, 20);
 	if (peer && peer->go_state == REMOTE_GO) {
@@ -730,7 +734,9 @@  static struct wpabuf *
p2p_build_go_neg_conf(struct p2p_data *p2p,
 		if (p2p->cfg->p2p_intra_bss)
 			group_capab |= P2P_GROUP_CAPAB_INTRA_BSS_DIST;
 	}
-	p2p_buf_add_capability(buf, p2p->dev_capab, group_capab);
+	p2p_buf_add_capability(buf,
+		p2p->dev_capab & ~P2P_DEV_CAPAB_CLIENT_DISCOVERABILITY,
+		group_capab);
 	if (go || resp_chan == NULL)
 		p2p_buf_add_operating_channel(buf, p2p->cfg->country,
 					      p2p->op_reg_class,
diff --git a/src/p2p/p2p_pd.c b/src/p2p/p2p_pd.c
index 7b47927..45bb4b9 100644
--- a/src/p2p/p2p_pd.c
+++ b/src/p2p/p2p_pd.c
@@ -54,7 +54,8 @@  static struct wpabuf *
p2p_build_prov_disc_req(struct p2p_data *p2p,
 	p2p_buf_add_public_action_hdr(buf, P2P_PROV_DISC_REQ, dialog_token);

 	len = p2p_buf_add_ie_hdr(buf);
-	p2p_buf_add_capability(buf, p2p->dev_capab, 0);
+	p2p_buf_add_capability(buf,
+		p2p->dev_capab & ~P2P_DEV_CAPAB_CLIENT_DISCOVERABILITY, 0);
 	p2p_buf_add_device_info(buf, p2p, NULL);
 	if (go) {