Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Mon, 13 May 2019 14:20:26 +0800
From:   Yibo Zhao <yiboz@codeaurora.org>
To:     Ben Greear <greearb@candelatech.com>
Cc:     linux-wireless@vger.kernel.org, ath10k@lists.infradead.org,
        Zhi Chen <zhichen@codeaurora.org>
Subject: Re: [PATCH] mac80211: remove warning message
In-Reply-To: <7119f24f-5b88-629a-d507-73776b841f65@candelatech.com>
References: <1557471662-1355-1-git-send-email-yiboz@codeaurora.org>
 <7119f24f-5b88-629a-d507-73776b841f65@candelatech.com>
Message-ID: <0247de90c551b76aed1f9647f9b274c0@codeaurora.org>
User-Agent: Roundcube Webmail/1.2.5
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On 2019-05-10 22:04, Ben Greear wrote:
> On 05/10/2019 12:01 AM, Yibo Zhao wrote:
>> In multiple SSID cases, it takes time to prepare every AP interface
>> to be ready in initializing phase. If a sta already knows everything 
>> it
>> needs to join one of the APs and sends authentication to the AP which
>> is not fully prepared at this point of time, AP's channel context
>> could be NULL. As a result, warning message occurs.
>> 
>> Even worse, if the AP is under attack via tools such as MDK3 and 
>> massive
>> authentication requests are received in a very short time, console 
>> will
>> be hung due to kernel warning messages.
> 
> Since it is a WARN_ON_ONCE, how it the console hang due to warnings?  
> You should
> get no more than once per boot?
> 
Hi Ben,

I was planning to use WARN_ON_ONCE() in the first place to replace 
WARN_ON() then after some discussion, we think removing it could be 
better. So the patch was based on my first version. Sorry for the 
confusing. Will raise another one.

> I have no problem with removing it though.  Seems a harmless splat and 
> I removed
> it from my tree some time back as well.
> 
> Thanks,
> Ben
> 
>> 
>> If this case can be hit during normal functionality, there should be 
>> no
>> WARN_ON(). Those should be reserved to cases that are not supposed to 
>> be
>> hit at all or some other more specific cases like indicating obsolete
>> interface.
>> 
>> Signed-off-by: Zhi Chen <zhichen@codeaurora.org>
>> Signed-off-by: Yibo Zhao <yiboz@codeaurora.org>
>> ---
>>  net/mac80211/ieee80211_i.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
>> index 2ae0364..f39c289 100644
>> --- a/net/mac80211/ieee80211_i.h
>> +++ b/net/mac80211/ieee80211_i.h
>> @@ -1435,7 +1435,7 @@ struct ieee80211_local {
>>  	rcu_read_lock();
>>  	chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>> 
>> -	if (WARN_ON_ONCE(!chanctx_conf)) {
>> +	if (!chanctx_conf) {
>>  		rcu_read_unlock();
>>  		return NULL;
>>  	}
>> 

-- 
Yibo