Patchwork nl80211: Increase verbosity for some auth failures.

login
register
mail settings
Submitter Ben Greear
Date Dec. 13, 2012, 5:29 p.m.
Message ID <1355419763-4012-1-git-send-email-greearb@candelatech.com>
Download mbox | patch
Permalink /patch/206200/
State Rejected
Headers show

Comments

Ben Greear - Dec. 13, 2012, 5:29 p.m.
From: Ben Greear <greearb@candelatech.com>

Makes the logs a bit more useful when running in default
message level (INFO).

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 src/drivers/driver_nl80211.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Jouni Malinen - Dec. 23, 2012, 9:51 a.m.
On Thu, Dec 13, 2012 at 09:29:23AM -0800, greearb@candelatech.com wrote:
> Makes the logs a bit more useful when running in default
> message level (INFO).

Since these are wpa_dbg() and not wpa_printf() calls, these would show
up in wpa_cli and especially the (auth) one shows up in common cases due
to cfg80211 behavior ("Operation already in progress"). As such, these
would result in undesired entries in wpa_cli and other programs using
control interface. Furthermore, these should be wpa_msg() instead of
wpa_dbg() after the change to MSG_INFO. Anyway, I don't think I would
apply this because of the way too common entries these would show in
external programs for cases that are expected behavior with at least
older versions of cfg80211.
Ben Greear - Jan. 2, 2013, 6:21 p.m.
On 12/23/2012 01:51 AM, Jouni Malinen wrote:
> On Thu, Dec 13, 2012 at 09:29:23AM -0800, greearb@candelatech.com wrote:
>> Makes the logs a bit more useful when running in default
>> message level (INFO).
>
> Since these are wpa_dbg() and not wpa_printf() calls, these would show
> up in wpa_cli and especially the (auth) one shows up in common cases due
> to cfg80211 behavior ("Operation already in progress"). As such, these
> would result in undesired entries in wpa_cli and other programs using
> control interface. Furthermore, these should be wpa_msg() instead of
> wpa_dbg() after the change to MSG_INFO. Anyway, I don't think I would
> apply this because of the way too common entries these would show in
> external programs for cases that are expected behavior with at least
> older versions of cfg80211.

Just to make sure I understand properly:  The patch should use
wpa_printf() instead, but even if I changed it as you suggested, you don't
want to apply the change upstream?

If so, I'll just leave it as is and try to remember to
not re-send this patch anymore :)

Thanks,
Ben
Jouni Malinen - Jan. 6, 2013, 6:25 p.m.
On Wed, Jan 02, 2013 at 10:21:34AM -0800, Ben Greear wrote:
> Just to make sure I understand properly:  The patch should use
> wpa_printf() instead, but even if I changed it as you suggested, you don't
> want to apply the change upstream?

wpa_msg() with MSG_INFO is not fine for an event that happens frequently
during normal operations (i.e., without any kind of real error
condition). I don't think I would add wpa_printf(MSG_INFO) for this
either for the same reason. So yes, using MSG_INFO for these would sound
excessive unless the real error cases can be somehow identified (e.g.,
check that errno is not that EALREADY or whatever the common non-error
case is).

Patch

diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index 297f677..c0e37f2 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -4548,7 +4548,7 @@  static int wpa_driver_nl80211_mlme(struct wpa_driver_nl80211_data *drv,
 	ret = send_and_recv_msgs(drv, msg, NULL, NULL);
 	msg = NULL;
 	if (ret) {
-		wpa_dbg(drv->ctx, MSG_DEBUG,
+		wpa_dbg(drv->ctx, MSG_INFO,
 			"nl80211: MLME command failed: reason=%u ret=%d (%s)",
 			reason_code, ret, strerror(-ret));
 		goto nla_put_failure;
@@ -4734,7 +4734,7 @@  retry:
 	ret = send_and_recv_msgs(drv, msg, NULL, NULL);
 	msg = NULL;
 	if (ret) {
-		wpa_dbg(drv->ctx, MSG_DEBUG,
+		wpa_dbg(drv->ctx, MSG_INFO,
 			"nl80211: MLME command failed (auth): ret=%d (%s)",
 			ret, strerror(-ret));
 		count++;