Message ID | 1557471662-1355-1-git-send-email-yiboz@codeaurora.org (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Johannes Berg |
Headers | show |
Series | mac80211: remove warning message | expand |
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? 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; > } >
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; >> } >>
On Fri, 2019-05-10 at 15:01 +0800, 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. Please share the dump, I don't think this should be happening. I think this warning did what it was supposed to, uncover a bug; rather than remove the warning we should fix the bug. > 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. I don't buy this, it's just a WARN_ON_ONCE(). > 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. I agree, but right now I'm inclined to think it's a bug elsewhere rather than normal operation. johannes
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; }