diff mbox

[1/1] P2P: Prevent pending_action_tx from truncating extended listen

Message ID CAGCGobDs1CYCk3TpSrymge1zjv+aeRP75QbZZx9-YFH5=WkcqQ@mail.gmail.com
State Superseded
Headers show

Commit Message

Jithu Jance Aug. 13, 2014, 2:51 p.m. UTC 
Hi Jouni,

Thanks for your review comments.

>
>>       if (wpa_s->p2p_long_listen > 0)
>>               wpa_s->p2p_long_listen -= wpa_s->max_remain_on_chan;
>> -     if (wpa_s->p2p_long_listen > 0) {
>> -             wpa_printf(MSG_DEBUG, "P2P: Continuing long Listen state");
>> -             wpas_p2p_listen_start(wpa_s, wpa_s->p2p_long_listen);
>> -     } else {
>> +     else {
>
> This looks incorrect.. The previous version decremented
> wpa_s->p2p_long_listen first and only after that compared it to >0 (and
> well, <=0 in case of the else clause).
>
>>               /*
>>                * When listen duration is over, stop listen & update p2p_state
>>                * to IDLE.
>>                */
>>               p2p_stop_listen(wpa_s->global->p2p);
>>       }
>
> So this could potentially not happen for the first time p2p_long_listen
> drops to zero (or below).

Got it. Thanks! I moved this piece of code above
offchannel_pending_listen is to make sure that p2p_long_listen is
updated in
the remain_on_channel_cb context itself and to avoid moving the code
to wpas_p2p_continue_listen_context. So that in case,
p2p_long_listen is accessed in between, it has an updated value
(though it is not done currently).


>
> Unless I'm missing something here, more appropriate way of addressing
> this would be to have the completion of the pending Action frame TX
> schedule a new P2P Listen if wpa_s->p2p_long_listen > 0.
Sounds good!. You mean offchannel_send_action_tx_status function?
Let me know whether below patch is fine. I have moved the p2p_long_listen
above to make sure that it remains updated even if we return from offchannel_tx
or listen_end. Hope it is fine.
---
 wpa_supplicant/offchannel.c     |    6 ++++++
 wpa_supplicant/p2p_supplicant.c |    4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)

        wpas_p2p_listen_start(wpa_s, wpa_s->p2p_long_listen);
--
1.7.9.5
diff mbox

Patch

diff --git a/wpa_supplicant/offchannel.c b/wpa_supplicant/offchannel.c
index 77683b6..891f641 100644
--- a/wpa_supplicant/offchannel.c
+++ b/wpa_supplicant/offchannel.c
@@ -188,6 +188,12 @@  void offchannel_send_action_tx_status(
            wpa_s->pending_action_bssid,
            data, data_len, result);
    }
+
+   if (wpa_s->p2p_long_listen > 0) {
+       /* Continue the listen */
+       wpa_printf(MSG_DEBUG, "P2P: Continuing long Listen state");
+       wpas_p2p_listen_start(wpa_s, wpa_s->p2p_long_listen);
+   }
 }


diff --git a/wpa_supplicant/p2p_supplicant.c
b/wpa_supplicant/p2p_supplicant.c
index d91877c..a1b08f7 100644
--- a/wpa_supplicant/p2p_supplicant.c
+++ b/wpa_supplicant/p2p_supplicant.c
@@ -4954,12 +4954,12 @@  void
wpas_p2p_cancel_remain_on_channel_cb(struct wpa_supplicant *wpa_s,
    wpas_p2p_listen_work_done(wpa_s);
    if (wpa_s->global->p2p_disabled || wpa_s->global->p2p == NULL)
        return;
+   if (wpa_s->p2p_long_listen > 0)
+       wpa_s->p2p_long_listen -= wpa_s->max_remain_on_chan;
    if (p2p_listen_end(wpa_s->global->p2p, freq) > 0)
        return; /* P2P module started a new operation */
    if (offchannel_pending_action_tx(wpa_s))
        return;
-   if (wpa_s->p2p_long_listen > 0)
-       wpa_s->p2p_long_listen -= wpa_s->max_remain_on_chan;
    if (wpa_s->p2p_long_listen > 0) {
        wpa_printf(MSG_DEBUG, "P2P: Continuing long Listen state");