| Message ID | 1355419763-4012-1-git-send-email-greearb@candelatech.com |
|---|---|
| State | Rejected |
| Headers | show |
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.
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
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).
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++;