From patchwork Wed Aug 13 14:51:05 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jithu Jance X-Patchwork-Id: 379667 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from maxx.maxx.shmoo.com (maxx.shmoo.com [205.134.188.171]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 80E5C140085 for ; Thu, 14 Aug 2014 00:51:47 +1000 (EST) Received: from localhost (localhost [127.0.0.1]) by maxx.maxx.shmoo.com (Postfix) with ESMTP id DEE9017C054; Wed, 13 Aug 2014 10:51:42 -0400 (EDT) X-Virus-Scanned: amavisd-new at maxx.shmoo.com Received: from maxx.maxx.shmoo.com ([127.0.0.1]) by localhost (maxx.shmoo.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Qt2yrOPAQSXa; Wed, 13 Aug 2014 10:51:42 -0400 (EDT) Received: from maxx.shmoo.com (localhost [127.0.0.1]) by maxx.maxx.shmoo.com (Postfix) with ESMTP id 0AB3D17C04C; Wed, 13 Aug 2014 10:51:37 -0400 (EDT) X-Original-To: mailman-post+hostap@maxx.shmoo.com Delivered-To: mailman-post+hostap@maxx.shmoo.com Received: from localhost (localhost [127.0.0.1]) by maxx.maxx.shmoo.com (Postfix) with ESMTP id 5E38617C04C for ; Wed, 13 Aug 2014 10:51:35 -0400 (EDT) X-Virus-Scanned: amavisd-new at maxx.shmoo.com Received: from maxx.maxx.shmoo.com ([127.0.0.1]) by localhost (maxx.shmoo.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Nv-q+aecmass for ; Wed, 13 Aug 2014 10:51:28 -0400 (EDT) Received: from mail-lb0-f176.google.com (mail-lb0-f176.google.com [209.85.217.176]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (not verified)) by maxx.maxx.shmoo.com (Postfix) with ESMTPS id 6B72B9C1C7 for ; Wed, 13 Aug 2014 10:51:28 -0400 (EDT) Received: by mail-lb0-f176.google.com with SMTP id u10so8189996lbd.7 for ; Wed, 13 Aug 2014 07:51:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :content-type; bh=4pOHSZoEVJPImxGvADMXrZVysynlLMf18nDVfITfpec=; b=lHCC/eq6EPCkvc+oJu1g43XWKYCmYvXvBLubypV8m4rcw5NvjjEVLCe77AfD+S0Cij eWW4561aRNXsfGVOuUBPkeg755XVX/92GmCTPOudP3y8lzJSNYKQzJ9d1gqMp5qspsX3 SSY6nRLa/+LKQGD8kbBIY9l/RBNky0dLClvEQb7bl4u/auUYUqS1aCwllSyQPBQdXp2e 5CVc7S3pWIcBnrx2oMH+/YqBE0uiGvelUop65heGUy9wE8t/3kQBsuUyb2LLJEojHAql RzG1bermz0xbOLnBYuvOFaf++XeME06BPjr+K5M7VM2yHUz7yupbsCs9BwcPscHKNET+ Xl6g== X-Received: by 10.152.245.171 with SMTP id xp11mr5117486lac.61.1407941486138; Wed, 13 Aug 2014 07:51:26 -0700 (PDT) MIME-Version: 1.0 Received: by 10.112.171.227 with HTTP; Wed, 13 Aug 2014 07:51:05 -0700 (PDT) In-Reply-To: <20140812143010.GC28663@w1.fi> References: <20140812143010.GC28663@w1.fi> From: Jithu Jance Date: Wed, 13 Aug 2014 20:21:05 +0530 Message-ID: Subject: Re: [PATCH 1/1] P2P: Prevent pending_action_tx from truncating extended listen To: "hostap@lists.shmoo.com" X-BeenThere: hostap@lists.shmoo.com X-Mailman-Version: 2.1.11 Precedence: list List-Id: HostAP Project List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: hostap-bounces@lists.shmoo.com Errors-To: hostap-bounces@lists.shmoo.com 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 --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");