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

[Jithu Jance]

Hi Jouni,

I overlooked a compilation failure for the previous patch after
merging to the local repo (compiled without enabling CONFIG_P2P).
Please find the corrected patch attached inline as well as attached. I
am facing white space issue in copy-pasting the patch in-line. Is
there any work around for the white space issue? I am using gmail in
plain-text mode.

Signed-off-by: Jithu Jance <jithu at broadcom.com>
---
 wpa_supplicant/offchannel.c     | 7 +++++++
 wpa_supplicant/p2p_supplicant.c | 6 +++---
 wpa_supplicant/p2p_supplicant.h | 2 ++
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/wpa_supplicant/offchannel.c b/wpa_supplicant/offchannel.c
index 77683b6..17689c5 100644
--- a/wpa_supplicant/offchannel.c
+++ b/wpa_supplicant/offchannel.c
@@ -12,6 +12,7 @@
 #include "common.h"
 #include "utils/eloop.h"
 #include "wpa_supplicant_i.h"
+#include "p2p_supplicant.h"
 #include "driver_i.h"
 #include "offchannel.h"

@@ -188,6 +189,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..3a30b1c 100644
--- a/wpa_supplicant/p2p_supplicant.c
+++ b/wpa_supplicant/p2p_supplicant.c
@@ -4925,7 +4925,7 @@ void wpas_p2p_remain_on_channel_cb(struct
wpa_supplicant *wpa_s,
 }


-static int wpas_p2p_listen_start(struct wpa_supplicant *wpa_s,
+int wpas_p2p_listen_start(struct wpa_supplicant *wpa_s,
  unsigned int timeout)
 {
  /* Limit maximum Listen state time based on driver limitation. */
@@ -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");
  wpas_p2p_listen_start(wpa_s, wpa_s->p2p_long_listen);
diff --git a/wpa_supplicant/p2p_supplicant.h b/wpa_supplicant/p2p_supplicant.h
index 841d6df..a75c467 100644
--- a/wpa_supplicant/p2p_supplicant.h
+++ b/wpa_supplicant/p2p_supplicant.h
@@ -58,6 +58,8 @@ int wpas_p2p_find(struct wpa_supplicant *wpa_s,
unsigned int timeout,
   const u8 *dev_id, unsigned int search_delay);
 void wpas_p2p_stop_find(struct wpa_supplicant *wpa_s);
 int wpas_p2p_listen(struct wpa_supplicant *wpa_s, unsigned int timeout);
+int wpas_p2p_listen_start(struct wpa_supplicant *wpa_s,
+ unsigned int timeout);
 int wpas_p2p_assoc_req_ie(struct wpa_supplicant *wpa_s, struct wpa_bss *bss,
   u8 *buf, size_t len, int p2p_group);
 void wpas_p2p_scan_ie(struct wpa_supplicant *wpa_s, struct wpabuf *ies);
-- 
1.8.1.4



On Wed, Aug 13, 2014 at 8:21 PM, Jithu Jance <jithujance at gmail.com> wrote:
> 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.
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-P2P-Avoid-trunction-of-extended-listen-due-to-offcha.patch
Type: application/octet-stream
Size: 3301 bytes
Desc: not available
URL: <http://lists.shmoo.com/pipermail/hostap/attachments/20140820/a50f66e7/attachment.obj>