| Message ID | 65d5f9e18f252cc5cc2075828ebef9abb77c5b06.1407407258.git.jithu@broadcom.com |
|---|---|
| State | Rejected |
| Headers | show |
On Thu, Aug 07, 2014 at 03:58:58PM +0530, Jithu Jance wrote: > After sending out GO Neg response with status=1, clear the p2p > discovery state. This will help in allowing the legacy STA scan's > to proceed (concurrent case). Also the P2P-FIND-STOPPED event will > help to sync the userspace. Another option is to resume the SEARCH > or LISTEN, but doesn't make much sense since we are waiting for the > user input (p2p_connect command). Hmm.. This sounds problematic. Wouldn't the change allow anyone to stop P2P discovery simply by sending a GO Neg Req frame to this device? p2p_connect command would be used only if the user is interested in connecting with the specific peer; if not, any other peer should be allowed to request group formation (or other P2P operations).
Hi Jouni, > Hmm.. This sounds problematic. Wouldn't the change allow anyone to stop > P2P discovery simply by sending a GO Neg Req frame to this device? That's true. But I am assuming that since there is a connection attempt from the peer, there would be a user input from the current device side (whether to accept the connection or deny incoming connection). In the denial context, the discovery state can be restarted by a fresh p2p_find. But yes, this will block other incoming connect request till upper layer/user finishes the action for the previous request. This is just a thought. I am not sure whether this is the right approach. Please advise. Actually the p2p_continue_find was not getting invoked in this scenario and p2p_state was stuck in P2P_SEARCH. so either we need to do p2p_continue_find or come out of discovery till we get a user input. If you think that we need to continue the discovery state, (P2P_SEARCH or LISTEN_ONLY state), I will send a patch accordingly.
On Tue, Aug 12, 2014 at 11:35:19PM +0530, Jithu Jance wrote: > > Hmm.. This sounds problematic. Wouldn't the change allow anyone to stop > > P2P discovery simply by sending a GO Neg Req frame to this device? > > That's true. But I am assuming that since there is a connection > attempt from the peer, there > would be a user input from the current device side (whether to accept > the connection or deny > incoming connection). In the denial context, the discovery state can > be restarted by a fresh p2p_find. > But yes, this will block other incoming connect request till upper > layer/user finishes the action for the previous request. This is just > a thought. I am not sure whether this is the right approach. Please > advise. I don't think this correct behavior. When the operation is initiated by a peer (e.g., GO Neg Req from unknown peer in this specific case) instead of some user action on the local device, I'd continue to previously requested local operation (p2p_listen/p2p_find in this case). > Actually the p2p_continue_find was not getting invoked in this > scenario and p2p_state was stuck in > P2P_SEARCH. so either we need to do p2p_continue_find or come out of > discovery till we get a user input. > If you think that we need to continue the discovery state, (P2P_SEARCH > or LISTEN_ONLY state), I will send > a patch accordingly. I'm not sure what the case was in August, but at least now that I added a new hwsim test case for this both for p2p_listen and p2p_find, the operation does continue after having transmitted the GO Negotiation Response (status=1). As such, I'm not sure what else would need to be changed. Anyway, if you can still see a case where the P2P listen/search gets stopped unexpectedly, I'd like to see a debug log showing the sequence that can hit that case. As far as this patch to clear state on GO Neg Resp failure callback is concerned, I'm dropping it now since I don't think it is correct thing to do in that case.
Hi Jouni, > the >operation does continue after having transmitted the GO Negotiation >Response (status=1). I think this is what I wanted to achieve via the earlier patch. Thanks a lot for the update. - Jithu Jance On Sun, Mar 1, 2015 at 8:05 PM, Jouni Malinen <j@w1.fi> wrote: > On Tue, Aug 12, 2014 at 11:35:19PM +0530, Jithu Jance wrote: > > > Hmm.. This sounds problematic. Wouldn't the change allow anyone to stop > > > P2P discovery simply by sending a GO Neg Req frame to this device? > > > > That's true. But I am assuming that since there is a connection > > attempt from the peer, there > > would be a user input from the current device side (whether to accept > > the connection or deny > > incoming connection). In the denial context, the discovery state can > > be restarted by a fresh p2p_find. > > But yes, this will block other incoming connect request till upper > > layer/user finishes the action for the previous request. This is just > > a thought. I am not sure whether this is the right approach. Please > > advise. > > I don't think this correct behavior. When the operation is initiated by > a peer (e.g., GO Neg Req from unknown peer in this specific case) > instead of some user action on the local device, I'd continue to > previously requested local operation (p2p_listen/p2p_find in this case). > > > Actually the p2p_continue_find was not getting invoked in this > > scenario and p2p_state was stuck in > > P2P_SEARCH. so either we need to do p2p_continue_find or come out of > > discovery till we get a user input. > > If you think that we need to continue the discovery state, (P2P_SEARCH > > or LISTEN_ONLY state), I will send > > a patch accordingly. > > I'm not sure what the case was in August, but at least now that I added > a new hwsim test case for this both for p2p_listen and p2p_find, the > operation does continue after having transmitted the GO Negotiation > Response (status=1). As such, I'm not sure what else would need to be > changed. Anyway, if you can still see a case where the P2P listen/search > gets stopped unexpectedly, I'd like to see a debug log showing the > sequence that can hit that case. > > As far as this patch to clear state on GO Neg Resp failure callback is > concerned, I'm dropping it now since I don't think it is correct thing > to do in that case. > > -- > Jouni Malinen PGP id EFC895FA > _______________________________________________ > HostAP mailing list > HostAP@lists.shmoo.com > http://lists.shmoo.com/mailman/listinfo/hostap >
diff --git a/src/p2p/p2p.c b/src/p2p/p2p.c index 565cbdb..22ec6cf 100644 --- a/src/p2p/p2p.c +++ b/src/p2p/p2p.c @@ -3034,8 +3034,16 @@ static void p2p_go_neg_resp_failure_cb(struct p2p_data *p2p, int success, struct p2p_device *dev; dev = p2p_get_device(p2p, addr); if (dev && - dev->status == P2P_SC_FAIL_INFO_CURRENTLY_UNAVAILABLE) + dev->status == P2P_SC_FAIL_INFO_CURRENTLY_UNAVAILABLE) { dev->flags |= P2P_DEV_PEER_WAITING_RESPONSE; + if ((p2p->state == P2P_SEARCH) || (p2p->state == P2P_LISTEN_ONLY)) { + /* Clear our search state or Listen state since now the peer is + * awaiting response from our side. + */ + p2p_dbg(p2p, "Clear the P2P discovery state"); + p2p_stop_find(p2p); + } + } } }
After sending out GO Neg response with status=1, clear the p2p discovery state. This will help in allowing the legacy STA scan's to proceed (concurrent case). Also the P2P-FIND-STOPPED event will help to sync the userspace. Another option is to resume the SEARCH or LISTEN, but doesn't make much sense since we are waiting for the user input (p2p_connect command). Signed-off-by: Jithu Jance <jithu@broadcom.com> --- src/p2p/p2p.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) -- 1.7.9.5