diff mbox

[v2] supplicant: Print human readable event names.

Message ID 1320704462-15609-1-git-send-email-greearb@candelatech.com
State Superseded
Headers show

Commit Message

Ben Greear Nov. 7, 2011, 10:21 p.m. UTC 
From: Ben Greear <greearb@candelatech.com>

This makes it easier to understand the event related
logs.

Also, print re-entrancy depth in the event handler.
I'm suspicious that it could be called in a re-entrant
manner if netlink events are received & processed as
part of another event.

Signed-hostap: Ben Greear <greearb@candelatech.com>
---

* Change name to event_to_string as suggested.

:100644 100644 24a61e4... 787d207... M	src/common/wpa_common.c
:100644 100644 06f2db3... f9b4361... M	src/drivers/driver.h
:100644 100644 9a0663b... 3c3d898... M	wpa_supplicant/events.c
 src/common/wpa_common.c |   57 +++++++++++++++++++++++++++++++++++++++++++++++
 src/drivers/driver.h    |    4 +++
 wpa_supplicant/events.c |   13 +++++++---
 3 files changed, 70 insertions(+), 4 deletions(-)

Comments

Jouni Malinen Nov. 9, 2011, 2:26 p.m. UTC | #1 
On Mon, Nov 07, 2011 at 02:21:02PM -0800, greearb@candelatech.com wrote:
> This makes it easier to understand the event related
> logs.

I can understand this part..

> Also, print re-entrancy depth in the event handler.
> I'm suspicious that it could be called in a re-entrant
> manner if netlink events are received & processed as
> part of another event.

This sounds strange. I would recommend splitting this part to a separate
patch if you want to get the human readable part in.. I would need to
hear quite a bit more convincing explanation on how you think this
function can be used in re-entrant manner in a single process
implementation with eloop.


> diff --git a/src/common/wpa_common.c b/src/common/wpa_common.c
> +#include "drivers/driver.h"

This wpa_common.c file should not really use driver interface.

> +const char* event_to_string(enum wpa_event_type event)

And this does not belong here either.. wpa_common.c is a set of common
helper functions for WPA EAPOL-Key processing. Driver events have
nothing to do with it. If you are using this only in
wpa_supplicant/events.c, it would be better to add the function there.
If there is a plan to have this shared with hostapd, a new C file in
src/drivers could be the best approach.
Ben Greear Nov. 9, 2011, 5:11 p.m. UTC | #2 
On 11/09/2011 06:26 AM, Jouni Malinen wrote:
> On Mon, Nov 07, 2011 at 02:21:02PM -0800, greearb@candelatech.com wrote:
>> This makes it easier to understand the event related
>> logs.
>
> I can understand this part..
>
>> Also, print re-entrancy depth in the event handler.
>> I'm suspicious that it could be called in a re-entrant
>> manner if netlink events are received&  processed as
>> part of another event.
>
> This sounds strange. I would recommend splitting this part to a separate
> patch if you want to get the human readable part in.. I would need to
> hear quite a bit more convincing explanation on how you think this
> function can be used in re-entrant manner in a single process
> implementation with eloop.

I was having a paranoid day...and I never did see a recursive call,
so I'm probably wrong about that part.  I'll check that code path
again and remove it from the patch unless I find something
very obvious.

>> diff --git a/src/common/wpa_common.c b/src/common/wpa_common.c
>> +#include "drivers/driver.h"
>
> This wpa_common.c file should not really use driver interface.
>
>> +const char* event_to_string(enum wpa_event_type event)
>
> And this does not belong here either.. wpa_common.c is a set of common
> helper functions for WPA EAPOL-Key processing. Driver events have
> nothing to do with it. If you are using this only in
> wpa_supplicant/events.c, it would be better to add the function there.
> If there is a plan to have this shared with hostapd, a new C file in
> src/drivers could be the best approach.

Seems like it's useful common code.  I'll try adding a new file.

Are you OK with the function header being in driver.h?

Thanks,
Ben
Jouni Malinen Nov. 11, 2011, 12:45 a.m. UTC | #3 
On Wed, Nov 09, 2011 at 09:11:05AM -0800, Ben Greear wrote:
> > If there is a plan to have this shared with hostapd, a new C file in
> > src/drivers could be the best approach.
> 
> Seems like it's useful common code.  I'll try adding a new file.
> 
> Are you OK with the function header being in driver.h?

Yes, that's fine for now. We may end up cleaning driver.h at some point,
but no need to worry about that in this context.
diff mbox

Patch

diff --git a/src/common/wpa_common.c b/src/common/wpa_common.c
index 24a61e4..787d207 100644
--- a/src/common/wpa_common.c
+++ b/src/common/wpa_common.c
@@ -23,8 +23,65 @@ 
 #include "ieee802_11_defs.h"
 #include "defs.h"
 #include "wpa_common.h"
+#include "drivers/driver.h"
 
 
+const char* event_to_string(enum wpa_event_type event)
+{
+	switch (event) {
+	case EVENT_ASSOC: return "ASSOC";
+	case EVENT_DISASSOC: return "DISASSOC";
+	case EVENT_MICHAEL_MIC_FAILURE: return "MICHAEL_MIC_FAILURE";
+	case EVENT_SCAN_RESULTS: return "SCAN_RESULTS";
+	case EVENT_ASSOCINFO: return "ASSOCINFO";
+	case EVENT_INTERFACE_STATUS: return "INTERFACE_STATUS";
+	case EVENT_PMKID_CANDIDATE: return "PMKID_CANDIDATE";
+	case EVENT_STKSTART: return "STKSTART";
+	case EVENT_TDLS: return "TDLS";
+	case EVENT_FT_RESPONSE: return "FT_RESPONSE";
+	case EVENT_IBSS_RSN_START: return "IBSS_RSN_START";
+	case EVENT_AUTH: return "AUTH";
+	case EVENT_DEAUTH: return "DEAUTH";
+	case EVENT_ASSOC_REJECT: return "ASSOC_REJECT";
+	case EVENT_AUTH_TIMED_OUT: return "AUTH_TIMED_OUT";
+	case EVENT_ASSOC_TIMED_OUT: return "ASSOC_TIMED_OUT";
+	case EVENT_FT_RRB_RX: return "FT_RRB_RX";
+	case EVENT_WPS_BUTTON_PUSHED: return "WPS_BUTTON_PUSHED";
+	case EVENT_TX_STATUS: return "TX_STATUS";
+	case EVENT_RX_FROM_UNKNOWN: return "RX_FROM_UNKNOWN";
+	case EVENT_RX_MGMT: return "RX_MGMT";
+	case EVENT_RX_ACTION: return "RX_ACTION";
+	case EVENT_REMAIN_ON_CHANNEL: return "REMAIN_ON_CHANNEL";
+	case EVENT_CANCEL_REMAIN_ON_CHANNEL: return "CANCEL_ROC";
+	case EVENT_MLME_RX: return "MLME_RX";
+	case EVENT_RX_PROBE_REQ: return "RX_PROBE_REQ";
+	case EVENT_NEW_STA: return "NEW_STA";
+	case EVENT_EAPOL_RX: return "EAPOL_RX";
+	case EVENT_SIGNAL_CHANGE: return "SIGNAL_CHANGE";
+	case EVENT_INTERFACE_ENABLED: return "IFACE_ENABLED";
+	case EVENT_INTERFACE_DISABLED: return "IFACE_DISABLED";
+	case EVENT_CHANNEL_LIST_CHANGED: return "CHANNEL_LIST_CHANGED";
+	case EVENT_INTERFACE_UNAVAILABLE: return "INTERFACE_UNAVAILABLE";
+	case EVENT_BEST_CHANNEL: return "BEST_CHANNEL";
+	case EVENT_UNPROT_DEAUTH: return "UNPROT_DEAUTH";
+	case EVENT_UNPROT_DISASSOC: return "UNPROT_DISASSOC";
+	case EVENT_STATION_LOW_ACK: return "STA_LOW_ACK";
+	case EVENT_P2P_DEV_FOUND: return "P2P_DEV_FOUND";
+	case EVENT_P2P_GO_NEG_REQ_RX: return "P2P_GO_NEG_REQ_RX";
+	case EVENT_P2P_GO_NEG_COMPLETED: return "P2P_GO_NEG_COMPLETED";
+	case EVENT_P2P_PROV_DISC_REQUEST: return "P2P_PROV_DISC_REQUEST";
+	case EVENT_P2P_PROV_DISC_RESPONSE: return "P2P_PROV_DISC_RESPONSE";
+	case EVENT_P2P_SD_REQUEST: return "P2P_SD_REQUEST";
+	case EVENT_P2P_SD_RESPONSE: return "P2P_SD_RESPONSE";
+	case EVENT_IBSS_PEER_LOST: return "IBSS_PEER_LOST";
+	case EVENT_DRIVER_GTK_REKEY: return "DRIVER_GTK_REKEY";
+	case EVENT_SCHED_SCAN_STOPPED: return "SCHED_SCAN_STOPPED";
+	case EVENT_DRIVER_CLIENT_POLL_OK: return "CLIENT_POLL_OK";
+
+	default: return "UNKNOWN";
+	}
+}
+
 /**
  * wpa_eapol_key_mic - Calculate EAPOL-Key MIC
  * @key: EAPOL-Key Key Confirmation Key (KCK)
diff --git a/src/drivers/driver.h b/src/drivers/driver.h
index 06f2db3..f9b4361 100644
--- a/src/drivers/driver.h
+++ b/src/drivers/driver.h
@@ -3456,6 +3456,10 @@  void wpa_supplicant_event(void *ctx, enum wpa_event_type event,
 			  union wpa_event_data *data);
 
 
+/* Convert wpa_event_type to a string for logging. */
+const char* event_to_string(enum wpa_event_type event);
+
+
 /*
  * The following inline functions are provided for convenience to simplify
  * event indication for some of the common events.
diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
index 9a0663b..3c3d898 100644
--- a/wpa_supplicant/events.c
+++ b/wpa_supplicant/events.c
@@ -1941,17 +1941,20 @@  void wpa_supplicant_event(void *ctx, enum wpa_event_type event,
 {
 	struct wpa_supplicant *wpa_s = ctx;
 	u16 reason_code = 0;
+	static int depth = 0;
 
 	if (wpa_s->wpa_state == WPA_INTERFACE_DISABLED &&
 	    event != EVENT_INTERFACE_ENABLED &&
 	    event != EVENT_INTERFACE_STATUS) {
-		wpa_dbg(wpa_s, MSG_DEBUG, "Ignore event %d while interface is "
-			"disabled", event);
+		wpa_dbg(wpa_s, MSG_DEBUG,
+			"Ignore event %s (%d) while interface is disabled",
+			event_to_string(event), event);
 		return;
 	}
 
-	wpa_dbg(wpa_s, MSG_DEBUG, "Event %d received on interface %s",
-		event, wpa_s->ifname);
+	depth++;
+	wpa_dbg(wpa_s, MSG_DEBUG, "Event %s (%d) received, depth: %i",
+		event_to_string(event), event, depth);
 
 	switch (event) {
 	case EVENT_AUTH:
@@ -2458,4 +2461,6 @@  void wpa_supplicant_event(void *ctx, enum wpa_event_type event,
 		wpa_msg(wpa_s, MSG_INFO, "Unknown event %d", event);
 		break;
 	}
+
+	depth--;
 }