diff mbox

[3/3] P2P: Optimize scan frequencies list when re-joining a persistent group

Message ID 1426483203-7325-3-git-send-email-ilan.peer@intel.com
State Changes Requested
Headers show

Commit Message

Ilan Peer March 16, 2015, 5:20 a.m. UTC
From: Avraham Stern <avraham.stern@intel.com>

When starting a P2P client to re-join a persistent group
(P2P_GROUP_ADD persistent=<id>), it is possible that the P2P GO was
already found in previous scans. Try to get the P2P GO operating
frequency from the scan results list so wpa_supplicant will initially
scan only the P2P GO known operating frequency.

Signed-off-by: Avraham Stern <avraham.stern@intel.com>
---
 wpa_supplicant/p2p_supplicant.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

Comments

Jouni Malinen March 22, 2015, 7:17 p.m. UTC | #1
On Mon, Mar 16, 2015 at 01:20:03AM -0400, Ilan Peer wrote:
> When starting a P2P client to re-join a persistent group
> (P2P_GROUP_ADD persistent=<id>), it is possible that the P2P GO was
> already found in previous scans. Try to get the P2P GO operating
> frequency from the scan results list so wpa_supplicant will initially
> scan only the P2P GO known operating frequency.

> diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c
> @@ -5504,18 +5504,26 @@ int wpas_p2p_group_add_persistent(struct wpa_supplicant *wpa_s,
> -	} else {
> -		freq = neg_freq;
> -		if (freq < 0 ||
> -		    (freq > 0 && !freq_included(channels, freq)))
> -			freq = 0;
> -	}
> +	} else if (ssid->mode == WPAS_MODE_INFRA) {
> +		freq = wpas_p2p_select_go_freq(wpa_s, neg_freq);

That looks suspicious.. wpas_p2p_select_go_freq() is used only on the GO
device, so this cannot really be correct.

> +		if (freq <= 0 ||
> +		    (freq > 0 && !freq_included(channels, freq))) {
> +			struct os_reltime now;
> +			struct wpa_bss *bss =
> +				wpa_bss_get_p2p_dev_addr(wpa_s, ssid->bssid);
> +
> +			os_get_reltime(&now);
> +			if (bss &&
> +			    !os_reltime_expired(&now, &bss->last_update, 5))
> +				freq = bss->freq;
> +			else
> +				freq = 0;
> +		}

And the freq > 0 && !freq_included(channels, freq) case should clear
freq to 0 to match the current behavior.
Ilan Peer March 22, 2015, 8:20 p.m. UTC | #2
On א', 2015-03-22 at 21:17 +0200, Jouni Malinen wrote:
> On Mon, Mar 16, 2015 at 01:20:03AM -0400, Ilan Peer wrote:

> > When starting a P2P client to re-join a persistent group

> > (P2P_GROUP_ADD persistent=<id>), it is possible that the P2P GO was

> > already found in previous scans. Try to get the P2P GO operating

> > frequency from the scan results list so wpa_supplicant will initially

> > scan only the P2P GO known operating frequency.

> 

> > diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c

> > @@ -5504,18 +5504,26 @@ int wpas_p2p_group_add_persistent(struct wpa_supplicant *wpa_s,

> > -	} else {

> > -		freq = neg_freq;

> > -		if (freq < 0 ||

> > -		    (freq > 0 && !freq_included(channels, freq)))

> > -			freq = 0;

> > -	}

> > +	} else if (ssid->mode == WPAS_MODE_INFRA) {

> > +		freq = wpas_p2p_select_go_freq(wpa_s, neg_freq);

> 

> That looks suspicious.. wpas_p2p_select_go_freq() is used only on the GO

> device, so this cannot really be correct.


Indeed. But it seems that android (display) is using this API to
re-start a client as well. I'll try to get a log that shows this.

> 

> > +		if (freq <= 0 ||

> > +		    (freq > 0 && !freq_included(channels, freq))) {

> > +			struct os_reltime now;

> > +			struct wpa_bss *bss =

> > +				wpa_bss_get_p2p_dev_addr(wpa_s, ssid->bssid);

> > +

> > +			os_get_reltime(&now);

> > +			if (bss &&

> > +			    !os_reltime_expired(&now, &bss->last_update, 5))

> > +				freq = bss->freq;

> > +			else

> > +				freq = 0;

> > +		}

> 

> And the freq > 0 && !freq_included(channels, freq) case should clear

> freq to 0 to match the current behavior.

> 


It does in case that the time is BSS is not new enough (5 seconds, to
match the fast associate flow).

Regards,

Ilan.
Ilan Peer March 23, 2015, 11:13 a.m. UTC | #3
Hi Jouni,

Below is a snippet of the log. You can also have a look at WifiP2pServiceImpl.java, reinvokePersistentGroup(), that shows how p2pGroupAdd() can be called to restart a P2P Client.

Hope this helps,

Ilan.

01-01 19:25:39.950 D/wpa_supplicant( 2891): P2P: Set timeout (state=SEARCH): 0.327200 sec
01-01 19:25:40.040 I/WifiDisplayController(  654): Connecting to Wifi display: Actiontec A776
01-01 19:25:40.040 D/wpa_supplicant( 2891): p2p0: Control interface command 'P2P_STOP_FIND'
01-01 19:25:40.040 D/wpa_supplicant( 2891): P2P: Stopping find
01-01 19:25:40.040 D/wpa_supplicant( 2891): P2P: Clear timeout (state=SEARCH)
01-01 19:25:40.040 D/wpa_supplicant( 2891): P2P: State SEARCH -> IDLE
01-01 19:25:40.040 I/wpa_supplicant( 2891): P2P-FIND-STOPPED 
01-01 19:25:40.040 D/wpa_supplicant( 2891): CTRL_IFACE monitor sent successfully to /data/misc/wifi/sockets/wpa_ctrl_27362-2\x00
01-01 19:25:40.040 D/wpa_supplicant( 2891): CTRL_IFACE monitor sent successfully to /data/misc/wifi/sockets/wpa_ctrl_654-2\x00
01-01 19:25:40.040 D/wpa_supplicant( 2891): P2P: Clear timeout (state=IDLE)
01-01 19:25:40.040 D/wpa_supplicant( 2891): P2P: Clear drv_in_listen (2412)
01-01 19:25:40.040 D/wpa_supplicant( 2891): nl80211: Cancel remain-on-channel with cookie 0x3dd
01-01 19:25:40.040 D/wpa_supplicant( 2891): nl80211: set_ap_wps_p2p_ie is not supported
01-01 19:25:40.040 D/wpa_supplicant( 2891): nl80211: Disable Probe Request reporting nl_preq=0x7e590c19
01-01 19:25:40.040 D/wpa_supplicant( 2891): p2p0: Skip removing already started radio work 'p2p-listen'@0xf6d186a0
01-01 19:25:40.040 D/wpa_supplicant( 2891): p2p0: Radio work 'p2p-listen'@0xf6d186a0 done in 0.090351 seconds
01-01 19:25:40.040 D/wpa_supplicant( 2891): nl80211: Drv Event 56 (NL80211_CMD_CANCEL_REMAIN_ON_CHANNEL) received for p2p0
01-01 19:25:40.040 D/wpa_supplicant( 2891): nl80211: Remain-on-channel event (cancel=1 freq=2412 channel_type=0 duration=0 cookie=0x3dd (match))
01-01 19:25:40.040 D/wpa_supplicant( 2891): p2p0: Event CANCEL_REMAIN_ON_CHANNEL (21) received
01-01 19:25:40.040 D/wpa_supplicant( 2891): P2P: Cancel remain-on-channel callback (p2p_long_listen=0 ms pending_action_tx=0x0)
01-01 19:25:40.040 D/wpa_supplicant( 2891): P2P: Driver ended Listen state (freq=2412)
01-01 19:25:40.040 D/wpa_supplicant( 2891): P2P: Skip stop_listen since not in listen_only state.
01-01 19:25:40.040 D/wpa_supplicant( 2891): p2p0: Control interface command 'P2P_PEER 10:9f:a9:dd:a7:76'
01-01 19:25:40.040 D/wpa_supplicant( 2891): p2p0: Control interface command 'P2P_PEER 10:9f:a9:dd:a7:76'
01-01 19:25:40.040 D/wpa_supplicant( 2891): p2p0: Control interface command 'P2P_GROUP_ADD persistent=0'
01-01 19:25:40.040 D/wpa_supplicant( 2891): P2P: Stopping find
01-01 19:25:40.040 D/wpa_supplicant( 2891): P2P: Clear timeout (state=IDLE)
01-01 19:25:40.040 D/wpa_supplicant( 2891): P2P: State IDLE -> IDLE
01-01 19:25:40.040 D/wpa_supplicant( 2891): nl80211: set_ap_wps_p2p_ie is not supported
01-01 19:25:40.040 D/wpa_supplicant( 2891): P2P: Create a new interface p2p-p2p0-16 for the group
01-01 19:25:40.040 D/wpa_supplicant( 2891): nl80211: Create interface iftype 8 (P2P_CLIENT)
01-01 19:25:40.040 D/wpa_supplicant( 2891): nl80211: Ignored event (cmd=7) for foreign interface (ifindex 25 wdev 0x0)
01-01 19:25:40.040 I/WAKELOCK_ACQUIRE(  654): TIMESTAMP=6137845336214, TAG=AudioMix, TYPE=PARTIAL_WAKE_LOCK             , COUNT=0, PID=0, UID=1013, FLAGS=
01-01 19:25:40.040 D/wpa_supplicant( 2891): nl80211: Ignored event (cmd=7) for foreign interface (ifindex 25 wdev 0x0)
01-01 19:25:40.040 D/wpa_supplicant( 2891): nl80211: New interface p2p-p2p0-16 created: ifindex=25
01-01 19:25:40.040 D/wpa_supplicant( 2891): nl80211: Interface p2p-p2p0-16 created for P2P - disable 11b rates
01-01 19:25:40.040 D/wpa_supplicant( 2891): nl80211: NL80211_CMD_SET_TX_BITRATE_MASK (ifindex=25 NL80211_TXRATE_LEGACY=OFDM-only)
01-01 19:25:40.040 D/wpa_supplicant( 2891): nl80211: Set TX rates failed: ret=-100 (Network is down)
01-01 19:25:40.040 D/wpa_supplicant( 2891): P2P: Created pending virtual interface p2p-p2p0-16 addr cc:02:ff:aa:00:f0
01-01 19:25:40.040 D/wpa_supplicant( 2891): Override interface parameter: ctrl_interface ('(null)' -> '/data/misc/wifi/sockets')
01-01 19:25:40.040 D/wpa_supplicant( 2891): Initializing interface 'p2p-p2p0-16' conf 'N/A' driver 'nl80211' ctrl_interface '/data/misc/wifi/sockets' bridge 'N/A'
01-01 19:25:40.040 I/wpa_supplicant( 2891): rfkill: Cannot open RFKILL control device
Jouni Malinen March 29, 2015, 7:34 a.m. UTC | #4
On Sun, Mar 22, 2015 at 08:20:35PM +0000, Peer, Ilan wrote:
> On א', 2015-03-22 at 21:17 +0200, Jouni Malinen wrote:
> > On Mon, Mar 16, 2015 at 01:20:03AM -0400, Ilan Peer wrote:
> > > When starting a P2P client to re-join a persistent group
> > > (P2P_GROUP_ADD persistent=<id>), it is possible that the P2P GO was
> > > already found in previous scans. Try to get the P2P GO operating
> > > frequency from the scan results list so wpa_supplicant will initially
> > > scan only the P2P GO known operating frequency.
> > 
> > > diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c
> > > @@ -5504,18 +5504,26 @@ int wpas_p2p_group_add_persistent(struct wpa_supplicant *wpa_s,
> > > +	} else if (ssid->mode == WPAS_MODE_INFRA) {
> > > +		freq = wpas_p2p_select_go_freq(wpa_s, neg_freq);
> > 
> > That looks suspicious.. wpas_p2p_select_go_freq() is used only on the GO
> > device, so this cannot really be correct.
> 
> Indeed. But it seems that android (display) is using this API to
> re-start a client as well. I'll try to get a log that shows this.

"P2P_GROUP_ADD persistent=<id>" can indeed start a P2P Client. I was not
talking about that; I was talking about how wpas_p2p_select_go_freq() is
used. It does not get called if the local device is a P2P Client in the
persistent group that gets restarted with P2P_GROUP_ADD.

wpas_p2p_select_go_freq() figures out the best channel a new GO instance
should be started on. It does not really make any sense to call it in
P2P Client case where you need to figure out what is the operating
channel of a peer device.
Ilan Peer March 29, 2015, 7:52 a.m. UTC | #5
> -----Original Message-----

> From: Jouni Malinen [mailto:j@w1.fi]

> Sent: Sunday, March 29, 2015 10:34

> To: Peer, Ilan

> Cc: hostap@lists.shmoo.com; Stern, Avraham

> Subject: Re: [PATCH 3/3] P2P: Optimize scan frequencies list when re-joining

> a persistent group

> 

> On Sun, Mar 22, 2015 at 08:20:35PM +0000, Peer, Ilan wrote:

> > On א', 2015-03-22 at 21:17 +0200, Jouni Malinen wrote:

> > > On Mon, Mar 16, 2015 at 01:20:03AM -0400, Ilan Peer wrote:

> > > > When starting a P2P client to re-join a persistent group

> > > > (P2P_GROUP_ADD persistent=<id>), it is possible that the P2P GO

> > > > was already found in previous scans. Try to get the P2P GO

> > > > operating frequency from the scan results list so wpa_supplicant

> > > > will initially scan only the P2P GO known operating frequency.

> > >

> > > > diff --git a/wpa_supplicant/p2p_supplicant.c

> > > > b/wpa_supplicant/p2p_supplicant.c @@ -5504,18 +5504,26 @@ int

> > > > wpas_p2p_group_add_persistent(struct wpa_supplicant *wpa_s,

> > > > +	} else if (ssid->mode == WPAS_MODE_INFRA) {

> > > > +		freq = wpas_p2p_select_go_freq(wpa_s, neg_freq);

> > >

> > > That looks suspicious.. wpas_p2p_select_go_freq() is used only on

> > > the GO device, so this cannot really be correct.

> >

> > Indeed. But it seems that android (display) is using this API to

> > re-start a client as well. I'll try to get a log that shows this.

> 

> "P2P_GROUP_ADD persistent=<id>" can indeed start a P2P Client. I was not

> talking about that; I was talking about how wpas_p2p_select_go_freq() is

> used. It does not get called if the local device is a P2P Client in the persistent

> group that gets restarted with P2P_GROUP_ADD.

> 

> wpas_p2p_select_go_freq() figures out the best channel a new GO instance

> should be started on. It does not really make any sense to call it in P2P Client

> case where you need to figure out what is the operating channel of a peer

> device.

> 


Ok. Will fix.

Thanks,

Ilan.
diff mbox

Patch

diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c
index bb61808..8acdbd2 100644
--- a/wpa_supplicant/p2p_supplicant.c
+++ b/wpa_supplicant/p2p_supplicant.c
@@ -5504,18 +5504,26 @@  int wpas_p2p_group_add_persistent(struct wpa_supplicant *wpa_s,
 			    (freq > 0 && !freq_included(channels, freq)))
 				freq = 0;
 		}
-	} else {
-		freq = neg_freq;
-		if (freq < 0 ||
-		    (freq > 0 && !freq_included(channels, freq)))
-			freq = 0;
-	}
+	} else if (ssid->mode == WPAS_MODE_INFRA) {
+		freq = wpas_p2p_select_go_freq(wpa_s, neg_freq);
+		if (freq <= 0 ||
+		    (freq > 0 && !freq_included(channels, freq))) {
+			struct os_reltime now;
+			struct wpa_bss *bss =
+				wpa_bss_get_p2p_dev_addr(wpa_s, ssid->bssid);
+
+			os_get_reltime(&now);
+			if (bss &&
+			    !os_reltime_expired(&now, &bss->last_update, 5))
+				freq = bss->freq;
+			else
+				freq = 0;
+		}
 
-	if (ssid->mode == WPAS_MODE_INFRA)
 		return wpas_start_p2p_client(wpa_s, ssid, addr_allocated, freq);
-
-	if (ssid->mode != WPAS_MODE_P2P_GO)
+	} else {
 		return -1;
+	}
 
 	if (wpas_p2p_init_go_params(wpa_s, &params, freq, ht40, vht, channels))
 		return -1;