Patchwork p2p_supplicant: probe reporting should be from listen interface

login
register
mail settings
Submitter Arend van Spriel
Date March 28, 2013, 2:39 p.m.
Message ID <1364481543-2636-1-git-send-email-arend@broadcom.com>
Download mbox | patch
Permalink /patch/232052/
State New
Headers show

Comments

Arend van Spriel - March 28, 2013, 2:39 p.m.
P2P listen phase is used to listen for P2P probe request messages
from peers. The probe reporting call should be done on the same
interface as the remain_on_channel.

Cc: Greg Goldman <ggoldman@broadcom.com>
Cc: Jithu Jance <jithu@broadcom.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-hostap: Arend van Spriel <arend@broadcom.com>
---
Hi David,

Found during testing that my device was not found, because we were not
responding to P2P probe requests during the listen phase.

This patch fixes it for my driver and it kind of makes sense to me that
the probe request reporting should be done on the same interface as the
on doing the LISTEN.

Not sure whether the P2P device patches are already applied so sending
this patch to you first.

Regards,
Arend
---
 wpa_supplicant/driver_i.h       |    9 +++++++++
 wpa_supplicant/p2p_supplicant.c |    4 ++--
 2 files changed, 11 insertions(+), 2 deletions(-)

--
1.7.10.4
Spinadel, David - April 4, 2013, 2:23 p.m.
> -----Original Message-----
> From: hostap-bounces@lists.shmoo.com [mailto:hostap-
> bounces@lists.shmoo.com] On Behalf Of Arend van Spriel
> Sent: Thursday, March 28, 2013 16:39
> To: David Spinadel
> Cc: Greg Goldman; hostap@lists.shmoo.com; Johannes Berg
> Subject: [PATCH] p2p_supplicant: probe reporting should be from listen
> interface
> 
> P2P listen phase is used to listen for P2P probe request messages from
> peers. The probe reporting call should be done on the same interface as the
> remain_on_channel.
> 
> Cc: Greg Goldman <ggoldman@broadcom.com>
> Cc: Jithu Jance <jithu@broadcom.com>
> Cc: Johannes Berg <johannes@sipsolutions.net>
> Signed-hostap: Arend van Spriel <arend@broadcom.com>
> ---
> Hi David,
> 
> Found during testing that my device was not found, because we were not
> responding to P2P probe requests during the listen phase.
> 
> This patch fixes it for my driver and it kind of makes sense to me that the
> probe request reporting should be done on the same interface as the on
> doing the LISTEN.
> 
> Not sure whether the P2P device patches are already applied so sending this
> patch to you first.
> 

Hmmm.
Your logic seems to be perfect, but it works fine for me without your fix, and stops to work with it. Didn't debug it yet, but maybe something is wrong with mac80211?

David
Arend van Spriel - April 4, 2013, 6:40 p.m.
On 04/04/13 16:23, Spinadel, David wrote:
>> -----Original Message-----
>> From: hostap-bounces@lists.shmoo.com [mailto:hostap-
>> bounces@lists.shmoo.com] On Behalf Of Arend van Spriel
>> Sent: Thursday, March 28, 2013 16:39
>> To: David Spinadel
>> Cc: Greg Goldman; hostap@lists.shmoo.com; Johannes Berg
>> Subject: [PATCH] p2p_supplicant: probe reporting should be from listen
>> interface
>>
>> P2P listen phase is used to listen for P2P probe request messages from
>> peers. The probe reporting call should be done on the same interface as the
>> remain_on_channel.
>>
>> Cc: Greg Goldman<ggoldman@broadcom.com>
>> Cc: Jithu Jance<jithu@broadcom.com>
>> Cc: Johannes Berg<johannes@sipsolutions.net>
>> Signed-hostap: Arend van Spriel<arend@broadcom.com>
>> ---
>> Hi David,
>>
>> Found during testing that my device was not found, because we were not
>> responding to P2P probe requests during the listen phase.
>>
>> This patch fixes it for my driver and it kind of makes sense to me that the
>> probe request reporting should be done on the same interface as the on
>> doing the LISTEN.
>>
>> Not sure whether the P2P device patches are already applied so sending this
>> patch to you first.
>>
>
> Hmmm.
> Your logic seems to be perfect, but it works fine for me without your fix, and stops to work with it. Didn't debug it yet, but maybe something is wrong with mac80211?

As I am working with a fullmac driver it does not involve mac80211. Are 
you using a mac80211 driver? Otherwise, we have to look for another 
culprit ;-)

Gr. AvS
Arend van Spriel - April 5, 2013, 8:12 a.m.
On 04/04/2013 08:40 PM, Arend van Spriel wrote:
> On 04/04/13 16:23, Spinadel, David wrote:
>>> -----Original Message-----
>>> From: hostap-bounces@lists.shmoo.com [mailto:hostap-
>>> bounces@lists.shmoo.com] On Behalf Of Arend van Spriel
>>> Sent: Thursday, March 28, 2013 16:39
>>> To: David Spinadel
>>> Cc: Greg Goldman; hostap@lists.shmoo.com; Johannes Berg
>>> Subject: [PATCH] p2p_supplicant: probe reporting should be from listen
>>> interface
>>>
>>> P2P listen phase is used to listen for P2P probe request messages from
>>> peers. The probe reporting call should be done on the same interface
>>> as the
>>> remain_on_channel.
>>>
>>> Cc: Greg Goldman<ggoldman@broadcom.com>
>>> Cc: Jithu Jance<jithu@broadcom.com>
>>> Cc: Johannes Berg<johannes@sipsolutions.net>
>>> Signed-hostap: Arend van Spriel<arend@broadcom.com>
>>> ---
>>> Hi David,
>>>
>>> Found during testing that my device was not found, because we were not
>>> responding to P2P probe requests during the listen phase.
>>>
>>> This patch fixes it for my driver and it kind of makes sense to me
>>> that the
>>> probe request reporting should be done on the same interface as the on
>>> doing the LISTEN.
>>>
>>> Not sure whether the P2P device patches are already applied so
>>> sending this
>>> patch to you first.
>>>
>>
>> Hmmm.
>> Your logic seems to be perfect, but it works fine for me without your
>> fix, and stops to work with it. Didn't debug it yet, but maybe
>> something is wrong with mac80211?
>
> As I am working with a fullmac driver it does not involve mac80211. Are
> you using a mac80211 driver? Otherwise, we have to look for another
> culprit ;-)

Hi Johannes,

It seems we are having an interworking issue here. During the P2P LISTEN 
state, should probe requests be reported on the P2P_DEVICE interface or 
on the primary interface? The probe response from wpa_supplicant should 
be sent on the P2P_DEVICE interface for sure, right? We need a consensus 
here on what behaviour is required.

Regards,
Arend
Johannes Berg - April 5, 2013, 9:51 a.m.
On Fri, 2013-04-05 at 10:12 +0200, Arend van Spriel wrote:

> >>> Found during testing that my device was not found, because we were not
> >>> responding to P2P probe requests during the listen phase.
> >>>
> >>> This patch fixes it for my driver and it kind of makes sense to me
> >>> that the
> >>> probe request reporting should be done on the same interface as the on
> >>> doing the LISTEN.
> >>>
> >>> Not sure whether the P2P device patches are already applied so
> >>> sending this
> >>> patch to you first.
> >>>
> >>
> >> Hmmm.
> >> Your logic seems to be perfect, but it works fine for me without your
> >> fix, and stops to work with it. Didn't debug it yet, but maybe
> >> something is wrong with mac80211?
> >
> > As I am working with a fullmac driver it does not involve mac80211. Are
> > you using a mac80211 driver? Otherwise, we have to look for another
> > culprit ;-)

> It seems we are having an interworking issue here. During the P2P LISTEN 
> state, should probe requests be reported on the P2P_DEVICE interface or 
> on the primary interface? The probe response from wpa_supplicant should 
> be sent on the P2P_DEVICE interface for sure, right? We need a consensus 
> here on what behaviour is required.

I think they should be reported on the P2P_DEVICE. I believe that's
actually the case for our driver anyway since it just reports the
frames, and mac80211 will sort them according to the virtual interface
address.

OTOH, if this change broke our driver and it worked before, maybe it's
not the case? I don't really know.

David, can you send me (privately, and probably better to my @intel
address) a trace capture for this problem?

johannes
Arend van Spriel - April 5, 2013, 10:51 a.m.
On 04/05/2013 11:51 AM, Johannes Berg wrote:
> On Fri, 2013-04-05 at 10:12 +0200, Arend van Spriel wrote:
>
>>>>> Found during testing that my device was not found, because we were not
>>>>> responding to P2P probe requests during the listen phase.
>>>>>
>>>>> This patch fixes it for my driver and it kind of makes sense to me
>>>>> that the
>>>>> probe request reporting should be done on the same interface as the on
>>>>> doing the LISTEN.
>>>>>
>>>>> Not sure whether the P2P device patches are already applied so
>>>>> sending this
>>>>> patch to you first.
>>>>>
>>>>
>>>> Hmmm.
>>>> Your logic seems to be perfect, but it works fine for me without your
>>>> fix, and stops to work with it. Didn't debug it yet, but maybe
>>>> something is wrong with mac80211?
>>>
>>> As I am working with a fullmac driver it does not involve mac80211. Are
>>> you using a mac80211 driver? Otherwise, we have to look for another
>>> culprit ;-)
>
>> It seems we are having an interworking issue here. During the P2P LISTEN
>> state, should probe requests be reported on the P2P_DEVICE interface or
>> on the primary interface? The probe response from wpa_supplicant should
>> be sent on the P2P_DEVICE interface for sure, right? We need a consensus
>> here on what behaviour is required.
>
> I think they should be reported on the P2P_DEVICE. I believe that's
> actually the case for our driver anyway since it just reports the
> frames, and mac80211 will sort them according to the virtual interface
> address.

Good to hear we are on the same page on this.

> OTOH, if this change broke our driver and it worked before, maybe it's
> not the case? I don't really know.

That made me decide to ping you.

Regards,
Arend

> David, can you send me (privately, and probably better to my @intel
> address) a trace capture for this problem?
>
> johannes
>
>
Johannes Berg - April 18, 2013, 5:44 p.m.
On Fri, 2013-04-05 at 12:51 +0200, Arend van Spriel wrote:

> > I think they should be reported on the P2P_DEVICE. I believe that's
> > actually the case for our driver anyway since it just reports the
> > frames, and mac80211 will sort them according to the virtual interface
> > address.

I was looking at this code for another reason and noticed that the
reporting is done using the bss->nl_mgmt socket -- maybe it should be
another socket?

johannes
Arend van Spriel - April 18, 2013, 7:39 p.m.
On 04/18/2013 07:44 PM, Johannes Berg wrote:
> On Fri, 2013-04-05 at 12:51 +0200, Arend van Spriel wrote:
>
>>> I think they should be reported on the P2P_DEVICE. I believe that's
>>> actually the case for our driver anyway since it just reports the
>>> frames, and mac80211 will sort them according to the virtual interface
>>> address.
>
> I was looking at this code for another reason and noticed that the
> reporting is done using the bss->nl_mgmt socket -- maybe it should be
> another socket?

Still not very proficient pn wpa_s, but the action frames are done in 
similar way.

nl80211_init_p2p_dev():
         /* Register P2P Public Action frame on the device interface */
         type = (WLAN_FC_TYPE_MGMT << 2) | (WLAN_FC_STYPE_ACTION << 4);
         nl80211_register_frame(p2p_dev, bss->nl_mgmt, type,
                                (u8 *) "\x04\x09\x50\x6f\x9a\x09", 6);

Gr. AvS
Johannes Berg - April 18, 2013, 7:41 p.m.
On Thu, 2013-04-18 at 21:39 +0200, Arend van Spriel wrote:
> On 04/18/2013 07:44 PM, Johannes Berg wrote:
> > On Fri, 2013-04-05 at 12:51 +0200, Arend van Spriel wrote:
> >
> >>> I think they should be reported on the P2P_DEVICE. I believe that's
> >>> actually the case for our driver anyway since it just reports the
> >>> frames, and mac80211 will sort them according to the virtual interface
> >>> address.
> >
> > I was looking at this code for another reason and noticed that the
> > reporting is done using the bss->nl_mgmt socket -- maybe it should be
> > another socket?
> 
> Still not very proficient pn wpa_s, but the action frames are done in 
> similar way.
> 
> nl80211_init_p2p_dev():
>          /* Register P2P Public Action frame on the device interface */
>          type = (WLAN_FC_TYPE_MGMT << 2) | (WLAN_FC_STYPE_ACTION << 4);
>          nl80211_register_frame(p2p_dev, bss->nl_mgmt, type,
>                                 (u8 *) "\x04\x09\x50\x6f\x9a\x09", 6);

Yeah, I'm not convinced that's right either :-)

Also your patch breaks operating with hwsim.

I'm looking into it ...

johannes

Patch

diff --git a/wpa_supplicant/driver_i.h b/wpa_supplicant/driver_i.h
index d70da3d..c7de723 100644
--- a/wpa_supplicant/driver_i.h
+++ b/wpa_supplicant/driver_i.h
@@ -734,6 +734,15 @@  wpa_drv_p2p_send_action_cancel_wait(struct wpa_supplicant *wpa_s)
 		wpa_s->driver->send_action_cancel_wait(wpa_p2p_get_priv(wpa_s));
 }

+static inline int wpa_drv_p2p_probe_req_report(struct wpa_supplicant *wpa_s,
+					       int report)
+{
+	if (wpa_s->driver->probe_req_report)
+		return wpa_s->driver->probe_req_report(wpa_p2p_get_priv(wpa_s),
+						       report);
+	return -1;
+}
+
 static inline int wpa_drv_p2p_remain_on_channel(struct wpa_supplicant *wpa_s,
 						unsigned int freq,
 						unsigned int duration)
diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c
index a6f7614..fefa86f 100644
--- a/wpa_supplicant/p2p_supplicant.c
+++ b/wpa_supplicant/p2p_supplicant.c
@@ -1229,7 +1229,7 @@  static int wpas_start_listen(void *ctx, unsigned int freq,

 	wpa_drv_set_ap_wps_ie(wpa_s, NULL, probe_resp_ie, NULL);

-	if (wpa_drv_probe_req_report(wpa_s, 1) < 0) {
+	if (wpa_drv_p2p_probe_req_report(wpa_s, 1) < 0) {
 		wpa_printf(MSG_DEBUG, "P2P: Failed to request the driver to "
 			   "report received Probe Request frames");
 		return -1;
@@ -1261,7 +1261,7 @@  static void wpas_stop_listen(void *ctx)
 		wpa_s->roc_waiting_drv_freq = 0;
 	}
 	wpa_drv_set_ap_wps_ie(wpa_s, NULL, NULL, NULL);
-	wpa_drv_probe_req_report(wpa_s, 0);
+	wpa_drv_p2p_probe_req_report(wpa_s, 0);
 }