FW: [PATCH] P2P: Fix inconsistency in displaying wps pin

[Jithu Jance]

Hi Jouni,


> I implemented this a bit differently in commit
> c19316354ed46e688704c1ebcf1c7efe816ddf31 by skipping the > PD
Response notifications in PD-before-join case regardless of
> what config method was requested. This looks like the most
> consistent and cleanest approach here.
> This will also require another commit for some cases where the
> PD Response could have been processed after having already
> started the scan for the join operation. Commit
> f63b8542156496ba88ef9f161e5931122d7319b9
> makes wpa_supplicant wait for the PD Response prior to
> continuing the join with the scan-for-connection step.

Your commits looks good to me and addresses my requirements. Thanks!! :)




- Jithu Jance


On Mon, Feb 6, 2012 at 12:32 AM, Jouni Malinen <j at w1.fi> wrote:

> On Mon, Dec 26, 2011 at 09:23:54PM -0800, Jithu Jance wrote:
> > Sounds like for "p2p_connect <devaddr> pin display join" command,
> suppressing Prov Disc SHOW-PIN would be a better
> > approach.
>
> Agreed.
>
> > I would also like to confirm about the handling of
> wpas_notify_p2p_provision_discovery
> > in this particular scenario. Is it okay to invoke the function with
> generated_pin = 0 value (the below patch uses this approach)
> >  or should I suppress the invocation of
> wpas_notify_p2p_provision_discovery when wpas->p2p_pin is populated??
>
> I think it is better to skip this call completely. Actually, these
> provision discovery response notifications should really be skipped
> completely in the automatic PD-before-join case since those were not
> explicitly requested by any external program and they do not really
> provide any additional information.
>
> > diff --git a/wpa_supplicant/p2p_supplicant.c
> b/wpa_supplicant/p2p_supplicant.c
> > @@ -1785,8 +1785,7 @@ void wpas_prov_disc_req(void *ctx, const u8 *peer,
> u16 config_methods,
> > -
> > -     if (config_methods & WPS_CONFIG_DISPLAY) {
> > +     if ((config_methods & WPS_CONFIG_DISPLAY) && (wpa_s->p2p_pin[0] ==
> '\0')) {
> >               generated_pin = wps_generate_pin();
>
> Why would we need to modify prov_disc_req callback? This is not used on
> the P2P device that will be joining a group as a P2P client.
>
> > @@ -1809,7 +1808,7 @@ void wpas_prov_disc_resp(void *ctx, const u8
> *peer, u16 config_methods)
> >       if (config_methods & WPS_CONFIG_DISPLAY)
> >               wpas_prov_disc_local_keypad(wpa_s, peer, "");
> > -     else if (config_methods & WPS_CONFIG_KEYPAD) {
> > +     else if ((config_methods & WPS_CONFIG_KEYPAD) &&
> (wpa_s->p2p_pin[0] == '\0')) {
> >               generated_pin = wps_generate_pin();
> >               wpas_prov_disc_local_display(wpa_s, peer, "",
> generated_pin);
> >       } else if (config_methods & WPS_CONFIG_PUSHBUTTON)
>
> While this could be fine for most cases, I don't really want to depend
> on wpa_s->p2p_pin getting cleared in all cases. I guess this could
> potentially skip an event message for Provision Discovery that is done
> in other contexts.
>
> I implemented this a bit differently in commit
> c19316354ed46e688704c1ebcf1c7efe816ddf31 by skipping the PD Response
> notifications in PD-before-join case regardless of what config method
> was requested. This looks like the most consistent and cleanest approach
> here.
>
> This will also require another commit for some cases where the PD
> Response could have been processed after having already started the scan
> for the join operation. Commit f63b8542156496ba88ef9f161e5931122d7319b9
> makes wpa_supplicant wait for the PD Response prior to continuing the
> join with the scan-for-connection step. This is also cleaning up the
> sequence of driver operations in the join step and can make it somewhat
> easier for some drivers to handle the remain-on-channel/offchannel TX
> and scan.
>
> --
> Jouni Malinen                                            PGP id EFC895FA
> _______________________________________________
> HostAP mailing list
> HostAP at lists.shmoo.com
> http://lists.shmoo.com/mailman/listinfo/hostap
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.shmoo.com/pipermail/hostap/attachments/20120207/70009444/attachment.htm