[prev in list] [next in list] [prev in thread] [next in thread]
List: hostap
Subject: Re: [PATCH 1/1] P2P: Prevent pending_action_tx from truncating extended listen
From: Jithu Jance <jithujance () gmail ! com>
Date: 2014-08-20 16:58:49
Message-ID: CAGCGobA5UPQhPQVHE4ObSfDGH+nZhhPAVoq4sSwJqk=5C67c5w () mail ! gmail ! com
[Download RAW message or body]
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@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@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.
>
["0001-P2P-Avoid-trunction-of-extended-listen-due-to-offcha.patch" (application/octet-stream)]
From ff8487b643bffef4dd64be2976b6f79114868063 Mon Sep 17 00:00:00 2001
From: Jithu Jance <jithu@broadcom.com>
Date: Wed, 20 Aug 2014 21:55:06 +0530
Subject: [PATCH] P2P: Avoid trunction of extended listen due to offchan tx
On receiving the cancel remain on channel event, the pending_tx
is scheduled immediately and returned. This was preventing
the wpas_p2p_listen_start function from execution thereby resulting
in termination of the extended listen.
Signed-off-by: Jithu Jance <jithu@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
_______________________________________________
HostAP mailing list
HostAP@lists.shmoo.com
http://lists.shmoo.com/mailman/listinfo/hostap
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic