Recent patch somewhat breaking p2p

[Arik Nemtsov]

On Sat, Aug 11, 2012 at 9:25 PM, Jouni Malinen <j at w1.fi> wrote:
>
> > A cap is attached (courtesy of Eyal, also CCed). Note the delay
> > between GO Neg Resp (Packet #132) to GO Neg Confirm (Packet #139).
> > About 200ms.
>
> Thanks! I was able to reproduce this both with and without a concurrent
> AP connection. This is not exactly good as far as the P2P specification
> if concerned and certainly not helpful with the commit
> 579a80982ad07619373876c09cdd60e2afcba5b5 in hostap.git. This resulted in
> 100% reproducible failure to complete GO Negotiation for the case where
> Nexus was the device sending GO Negotiation Confirmation.

Thanks. That's indeed a step in the right direction.

>
> > > I don't really want to revert this, but the mechanism needs to be made
> > > to take into account the ack frame. If it is also clear that some
> > > common deployed devices take more than 100 ms to send to GO Neg Conf,
> > > the timeout value may need to be adjusted.
> >
> > I don't think this will be good enough. We've seen situations where a
> > peer switches the channels just after getting a GO Neg Resp. It
> > receives the frame, but doesn't send an ack.
> > Said peer will now send a GO Neg Confirm, and since the current code
> > ignores the Tx status of the GO Neg Resp, everything will be fine. If
> > we start considering the ack, and only giving the negotiation a single
> > shot, the connection will fail.
>
> I'm not sure I follow. Not getting an ack would mean that we would not
> stop at the 100 ms timeout.

I'm referring to p2p_go_neg_resp_cb() - the "success" argument
(indicating the Tx ack) is ignored. I was simply stating this is good
in some situations, where we don't get the ack, but the peer still got
the GO-Resp packet and will respond (with a GO-Confirm).

>
> > Of course there are situations where the peer doesn't send an ack
> > since it didn't get the GO Neg Resp packet. So we should still give it
> > another try in that case. No amount of delay can address this one.
>
> If the peer does not receive GO Negotiation Response, I would assume it
> will likely start new GO Negotiation attempt. What do you mean with
> another try? Sending GO Negotiation Response again? It was already
> retried at lower layer for multiple times..

The peer that didn't get the GO Negotiation response will indeed start
a new negotiation attempt. But there's a problem with the peer that
sent the GO Negotiation Response (that wasn't received).
This peer will mark P2P_DEV_WAIT_GO_NEG_CONFIRM and that will cause
the negotiation to fail in p2p_timeout_connect().

If the p2p_timeout_connect() check is removed, the sending peer will
simply start a new negotiation attempt, which is good.

Further, I think checking "success" in p2p_go_neg_resp_cb() is not a
good solution for the problem, because of what I stated above.

Arik