All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
@ 2014-02-06  1:44 Calvin Owens
  2014-02-06  4:44 ` Joe Perches
                   ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Calvin Owens @ 2014-02-06  1:44 UTC (permalink / raw)
  To: Johannes Berg, David S. Miller, John W. Linville
  Cc: linux-wireless, linux-kernel, Calvin Owens

Create a function to return a descriptive string for each reason code,
and print that instead of the numeric value in the kernel log. These
codes are easily found on popular search engines, but one is generally
not able to access the internet when dealing with wireless connectivity
issues.

On x86_64, this patch only enlarges the kernel binary by 489 bytes.

Signed-off-by: Calvin Owens <jcalvinowens@gmail.com>
---
 include/net/mac80211.h | 11 +++++++++
 net/mac80211/main.c    | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/mlme.c    | 12 ++++-----
 3 files changed, 83 insertions(+), 6 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index f4ab2fb..5983b25 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2973,6 +2973,17 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
 					const struct ieee80211_ops *ops);
 
 /**
+ * ieee80211_get_reason_code_string - Get human readable reason code
+ *
+ * This function returns a string describing the @reason_code.
+ *
+ * @reason_code: Reason code we want a human readable string for
+ *
+ * Return: Human readable reason string, or "(INVALID)"
+ */
+const char *ieee80211_get_reason_code_string(u16 reason_code);
+
+/**
  * ieee80211_register_hw - Register hardware device
  *
  * You must call this function before any other functions in
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index d767cfb..a1eb3ab 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -743,6 +743,72 @@ static int ieee80211_init_cipher_suites(struct ieee80211_local *local)
 	return 0;
 }
 
+static const char *reason_code_strings[49] = {
+	"(RESERVED)",
+	"UNSPECIFIED",
+	"PREV_AUTH_NOT_VALID",
+	"DEAUTH_LEAVING",
+	"DISASSOC_DUE_TO_INACTIVITY",
+	"DISASSOC_AP_BUSY",
+	"CLASS2_FRAME_FROM_NONAUTH_STA",
+	"CLASS3_FRAME_FROM_NONASSOC_STA",
+	"DISASSOC_STA_HAS_LEFT",
+	"STA_REQ_ASSOC_WITHOUT_AUTH",
+	"DISASSOC_BAD_POWER",
+	"DISASSOC_BAD_SUPP_CHAN",
+	"(RESERVED)",
+	"INVALID_IE",
+	"MIC_FAILURE",
+	"4WAY_HANDSHAKE_TIMEOUT",
+	"GROUP_KEY_HANDSHAKE_TIMEOUT",
+	"IE_DIFFERENT",
+	"INVALID_GROUP_CIPHER",
+	"INVALID_PAIRWISE_CIPHER",
+	"INVALID_AKMP",
+	"UNSUPP_RSN_VERSION",
+	"INVALID_RSN_IE_CAP",
+	"IEEE8021X_FAILED",
+	"CIPHER_SUITE_REJECTED", /* 24 */
+	"DISASSOC_UNSPECIFIED_QOS", /* 32 (25) */
+	"DISASSOC_QAP_NO_BANDWIDTH",
+	"DISASSOC_LOW_ACK",
+	"DISASSOC_QAP_EXCEED_TXOP",
+	"QSTA_LEAVE_QBSS",
+	"QSTA_NOT_USE",
+	"QSTA_REQUIRE_SETUP",
+	"QSTA_TIMEOUT",
+	"QSTA_CIPHER_NOT_SUPP", /* 45 (33) */
+	"MESH_PEER_CANCELED", /* 52 (34) */
+	"MESH_MAX_PEERS",
+	"MESH_CONFIG",
+	"MESH_CLOSE",
+	"MESH_MAX_RETRIES",
+	"MESH_CONFIRM_TIMEOUT",
+	"MESH_INVALID_GTK",
+	"MESH_INCONSISTENT_PARAM",
+	"MESH_INVALID_SECURITY",
+	"MESH_PATH_ERROR",
+	"MESH_PATH_NOFORWARD",
+	"MESH_PATH_DEST_UNREACHABLE",
+	"MAC_EXISTS_IN_MBSS",
+	"MESH_CHAN_REGULATORY",
+	"MESH_CHAN" /* 66 (48) */
+};
+
+const char *ieee80211_get_reason_code_string(u16 reason_code)
+{
+	if (reason_code <= 24)
+		return reason_code_strings[reason_code];
+	else if (reason_code >= 32 && reason_code <= 39)
+		return reason_code_strings[reason_code - 7];
+	else if (reason_code == 45)
+		return reason_code_strings[33];
+	else if (reason_code >= 52 && reason_code <= 66)
+		return reason_code_strings[reason_code - 18];
+	else
+		return "(INVALID)";
+}
+
 int ieee80211_register_hw(struct ieee80211_hw *hw)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index fc1d824..74e4585 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2231,8 +2231,8 @@ static void ieee80211_rx_mgmt_deauth(struct ieee80211_sub_if_data *sdata,
 
 	reason_code = le16_to_cpu(mgmt->u.deauth.reason_code);
 
-	sdata_info(sdata, "deauthenticated from %pM (Reason: %u)\n",
-		   bssid, reason_code);
+	sdata_info(sdata, "deauthenticated from %pM (reason: %s)\n",
+		   bssid, ieee80211_get_reason_code_string(reason_code));
 
 	ieee80211_set_disassoc(sdata, 0, 0, false, NULL);
 
@@ -4301,8 +4301,8 @@ int ieee80211_mgd_deauth(struct ieee80211_sub_if_data *sdata,
 	bool report_frame = false;
 
 	sdata_info(sdata,
-		   "deauthenticating from %pM by local choice (reason=%d)\n",
-		   req->bssid, req->reason_code);
+		   "deauthenticating from %pM by local choice (reason: %s)\n",
+		   req->bssid, ieee80211_get_reason_code_string(req->reason_code));
 
 	if (ifmgd->auth_data) {
 		drv_mgd_prepare_tx(sdata->local, sdata);
@@ -4348,8 +4348,8 @@ int ieee80211_mgd_disassoc(struct ieee80211_sub_if_data *sdata,
 		return -ENOLINK;
 
 	sdata_info(sdata,
-		   "disassociating from %pM by local choice (reason=%d)\n",
-		   req->bss->bssid, req->reason_code);
+		   "disassociating from %pM by local choice (reason: %s)\n",
+		   req->bss->bssid, ieee80211_get_reason_code_string(req->reason_code));
 
 	memcpy(bssid, req->bss->bssid, ETH_ALEN);
 	ieee80211_set_disassoc(sdata, IEEE80211_STYPE_DISASSOC,
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-06  1:44 [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes Calvin Owens
@ 2014-02-06  4:44 ` Joe Perches
  2014-02-10 11:09   ` Johannes Berg
  2014-02-06  8:37 ` [PATCH] " Johannes Berg
  2014-02-10  8:50 ` Jouni Malinen
  2 siblings, 1 reply; 21+ messages in thread
From: Joe Perches @ 2014-02-06  4:44 UTC (permalink / raw)
  To: Calvin Owens
  Cc: Johannes Berg, David S. Miller, John W. Linville, linux-wireless,
	linux-kernel

On Wed, 2014-02-05 at 19:44 -0600, Calvin Owens wrote:
> Create a function to return a descriptive string for each reason code,
> and print that instead of the numeric value in the kernel log. These
> codes are easily found on popular search engines, but one is generally
> not able to access the internet when dealing with wireless connectivity
> issues.

Hello Calvin.

Some suggestions below...

> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
[]
> @@ -743,6 +743,72 @@ static int ieee80211_init_cipher_suites(struct ieee80211_local *local)
>  	return 0;
>  }
>  
> +static const char *reason_code_strings[49] = {

static const char * const reason_etc...

> +	"(RESERVED)",
> +	"UNSPECIFIED",

etc..., but perhaps this thing below is fragile.

> +const char *ieee80211_get_reason_code_string(u16 reason_code)
> +{
> +	if (reason_code <= 24)
> +		return reason_code_strings[reason_code];
> +	else if (reason_code >= 32 && reason_code <= 39)
> +		return reason_code_strings[reason_code - 7];
> +	else if (reason_code == 45)
> +		return reason_code_strings[33];
> +	else if (reason_code >= 52 && reason_code <= 66)
> +		return reason_code_strings[reason_code - 18];
> +	else
> +		return "(INVALID)";
> +}

Perhaps use a more common kernel style

struct ieee80211_reason_descriptions {
	u16		code;
	const char *	desc;
}

and enumerate the reason codes with #defines and use a
macro to populate the descriptions

#define IEEE80211_REASON_RESERVED		0
#define IEEE80211_REASON_UNSPECIFIED		1

etc.

#define POPULATE_IEEE_REASON(code)			\
	{.code = IEEE80211_REASON_##code, .desc = #code}

static const struct ieee80211_reason_descriptions reasons[] = {
	POPULATE_IEEE_REASON(RESERVED),
	POPULATE_IEEE_REASON(UNSPECIFIED),
	[etc...]
};

So this function becomes something like:

const char *ieee80211_get_reason_code_string(u16 reason_code)
{
	int i;

	for (i = 0; i < ARRAY_SIZE(reasons); i++) {
		if (reasons[i].code == reason_code)
			return reasons[i].desc;
	}

	return "UNKNOWN";
}

This seems a lot less fragile.

> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
[]
> @@ -2231,8 +2231,8 @@ static void ieee80211_rx_mgmt_deauth(struct ieee80211_sub_if_data *sdata,
>  
>  	reason_code = le16_to_cpu(mgmt->u.deauth.reason_code);
>  
> -	sdata_info(sdata, "deauthenticated from %pM (Reason: %u)\n",
> -		   bssid, reason_code);
> +	sdata_info(sdata, "deauthenticated from %pM (reason: %s)\n",
> +		   bssid, ieee80211_get_reason_code_string(reason_code));

Perhaps

	"%u:%s",
	reason_code, ieee80211_get_reason_code_string(reason_code))

might be better here and the other places.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-06  1:44 [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes Calvin Owens
  2014-02-06  4:44 ` Joe Perches
@ 2014-02-06  8:37 ` Johannes Berg
  2014-02-07 12:53   ` Kalle Valo
  2014-02-07 20:50   ` Calvin Owens
  2014-02-10  8:50 ` Jouni Malinen
  2 siblings, 2 replies; 21+ messages in thread
From: Johannes Berg @ 2014-02-06  8:37 UTC (permalink / raw)
  To: Calvin Owens
  Cc: David S. Miller, John W. Linville, linux-wireless, linux-kernel

On Wed, 2014-02-05 at 19:44 -0600, Calvin Owens wrote:
> Create a function to return a descriptive string for each reason code,
> and print that instead of the numeric value in the kernel log. These
> codes are easily found on popular search engines, but one is generally
> not able to access the internet when dealing with wireless connectivity
> issues.

I believe both iw and wpa_supplicant already have the reason code
printout, and if you're diagnosing connectivity issues then you're
probably using those anyway (e.g. iw event -t), so I don't really see
much point in adding this to the kernel?

johannes


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-06  8:37 ` [PATCH] " Johannes Berg
@ 2014-02-07 12:53   ` Kalle Valo
  2014-02-07 15:46     ` Larry Finger
  2014-02-07 20:50   ` Calvin Owens
  1 sibling, 1 reply; 21+ messages in thread
From: Kalle Valo @ 2014-02-07 12:53 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Calvin Owens, David S. Miller, John W. Linville, linux-wireless,
	linux-kernel

Johannes Berg <johannes@sipsolutions.net> writes:

> On Wed, 2014-02-05 at 19:44 -0600, Calvin Owens wrote:
>> Create a function to return a descriptive string for each reason code,
>> and print that instead of the numeric value in the kernel log. These
>> codes are easily found on popular search engines, but one is generally
>> not able to access the internet when dealing with wireless connectivity
>> issues.
>
> I believe both iw and wpa_supplicant already have the reason code
> printout, and if you're diagnosing connectivity issues then you're
> probably using those anyway (e.g. iw event -t), so I don't really see
> much point in adding this to the kernel?

FWIW I find this useful. When I have connection problems I rarely look
at wpasupplicant, mostly I'm so lazy that I just check from the kernel
log what's happening. Just my 0.02 EUR.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-07 12:53   ` Kalle Valo
@ 2014-02-07 15:46     ` Larry Finger
  2014-02-07 22:25       ` Luca Coelho
  0 siblings, 1 reply; 21+ messages in thread
From: Larry Finger @ 2014-02-07 15:46 UTC (permalink / raw)
  To: Kalle Valo, Johannes Berg
  Cc: Calvin Owens, David S. Miller, John W. Linville, linux-wireless,
	linux-kernel

On 02/07/2014 06:53 AM, Kalle Valo wrote:
> Johannes Berg <johannes@sipsolutions.net> writes:
>
>> On Wed, 2014-02-05 at 19:44 -0600, Calvin Owens wrote:
>>> Create a function to return a descriptive string for each reason code,
>>> and print that instead of the numeric value in the kernel log. These
>>> codes are easily found on popular search engines, but one is generally
>>> not able to access the internet when dealing with wireless connectivity
>>> issues.
>>
>> I believe both iw and wpa_supplicant already have the reason code
>> printout, and if you're diagnosing connectivity issues then you're
>> probably using those anyway (e.g. iw event -t), so I don't really see
>> much point in adding this to the kernel?
>
> FWIW I find this useful. When I have connection problems I rarely look
> at wpasupplicant, mostly I'm so lazy that I just check from the kernel
> log what's happening. Just my 0.02 EUR.

I would also find it useful. I assist with wireless problems on the openSUSE 
Forum, and there we are lucky if we get dmesg output. When we do and it contains 
a deauthentication reason, I always need to bring up a web page to interpret the 
output. With this change, one step could be skipped.




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-06  8:37 ` [PATCH] " Johannes Berg
  2014-02-07 12:53   ` Kalle Valo
@ 2014-02-07 20:50   ` Calvin Owens
  1 sibling, 0 replies; 21+ messages in thread
From: Calvin Owens @ 2014-02-07 20:50 UTC (permalink / raw)
  To: Johannes Berg
  Cc: David S. Miller, John W. Linville, linux-wireless, linux-kernel,
	Joe Perches

On Thursday 02/06 at 09:37 +0100, Johannes Berg wrote:
> On Wed, 2014-02-05 at 19:44 -0600, Calvin Owens wrote:
> > Create a function to return a descriptive string for each reason code,
> > and print that instead of the numeric value in the kernel log. These
> > codes are easily found on popular search engines, but one is generally
> > not able to access the internet when dealing with wireless connectivity
> > issues.
> 
> I believe both iw and wpa_supplicant already have the reason code
> printout, and if you're diagnosing connectivity issues then you're
> probably using those anyway (e.g. iw event -t), so I don't really see
> much point in adding this to the kernel?
> 
> johannes

Ah, I didn't realize `iw` would interpret the reason code for you.

There were a couple of other replies expressing interest in this though.
Should I rewrite the patch per Joe Perches' suggestions and resend it?
Or should I just drop it since you can obtain the info from `iw`?

Thanks,
Calvin

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-07 15:46     ` Larry Finger
@ 2014-02-07 22:25       ` Luca Coelho
  2014-02-08  6:38         ` Kalle Valo
  0 siblings, 1 reply; 21+ messages in thread
From: Luca Coelho @ 2014-02-07 22:25 UTC (permalink / raw)
  To: Larry Finger
  Cc: Kalle Valo, Johannes Berg, Calvin Owens, David S. Miller,
	John W. Linville, linux-wireless, linux-kernel

On Fri, 2014-02-07 at 09:46 -0600, Larry Finger wrote:
> On 02/07/2014 06:53 AM, Kalle Valo wrote:
> > Johannes Berg <johannes@sipsolutions.net> writes:
> >
> >> On Wed, 2014-02-05 at 19:44 -0600, Calvin Owens wrote:
> >>> Create a function to return a descriptive string for each reason code,
> >>> and print that instead of the numeric value in the kernel log. These
> >>> codes are easily found on popular search engines, but one is generally
> >>> not able to access the internet when dealing with wireless connectivity
> >>> issues.
> >>
> >> I believe both iw and wpa_supplicant already have the reason code
> >> printout, and if you're diagnosing connectivity issues then you're
> >> probably using those anyway (e.g. iw event -t), so I don't really see
> >> much point in adding this to the kernel?
> >
> > FWIW I find this useful. When I have connection problems I rarely look
> > at wpasupplicant, mostly I'm so lazy that I just check from the kernel
> > log what's happening. Just my 0.02 EUR.
> 
> I would also find it useful. I assist with wireless problems on the openSUSE 
> Forum, and there we are lucky if we get dmesg output. When we do and it contains 
> a deauthentication reason, I always need to bring up a web page to interpret the 
> output. With this change, one step could be skipped.

But is it worth putting this parsing in the *kernel*? I mean, if anyone
is interested enough in the problem, a simple google query is not that
hard, right?

If this happens to you "on-the-fly" and all you want is your connection
to work, you won't start debugging right away.  You'll (maybe) save the
kernel logs, try to reconnect and debug later, at home.

If you think you *can* fix the problem on-the-fly, but you can't google
because the connection doesn't work, you certainly have either the IEEE
802.11 specs or the kernel sources somewhere in your device. :)

--
Luca.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-07 22:25       ` Luca Coelho
@ 2014-02-08  6:38         ` Kalle Valo
  0 siblings, 0 replies; 21+ messages in thread
From: Kalle Valo @ 2014-02-08  6:38 UTC (permalink / raw)
  To: Luca Coelho
  Cc: Larry Finger, Johannes Berg, Calvin Owens, David S. Miller,
	John W. Linville, linux-wireless, linux-kernel

Luca Coelho <luca@coelho.fi> writes:

>> Forum, and there we are lucky if we get dmesg output. When we do and it contains 
>> a deauthentication reason, I always need to bring up a web page to interpret the 
>> output. With this change, one step could be skipped.
>
> But is it worth putting this parsing in the *kernel*? I mean, if anyone
> is interested enough in the problem, a simple google query is not that
> hard, right?

Sure, it's not hard to find it. My and Larry's point is more about
convenience and user friendliness.

On the other hand I do understand that kernel is getting bloated all the
time, for example openwrt is suffering from that. So not really sure
what is the best.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-06  1:44 [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes Calvin Owens
  2014-02-06  4:44 ` Joe Perches
  2014-02-06  8:37 ` [PATCH] " Johannes Berg
@ 2014-02-10  8:50 ` Jouni Malinen
  2 siblings, 0 replies; 21+ messages in thread
From: Jouni Malinen @ 2014-02-10  8:50 UTC (permalink / raw)
  To: Calvin Owens
  Cc: Johannes Berg, David S. Miller, John W. Linville, linux-wireless,
	linux-kernel

On Wed, Feb 05, 2014 at 07:44:48PM -0600, Calvin Owens wrote:
> Create a function to return a descriptive string for each reason code,
> and print that instead of the numeric value in the kernel log. These
> codes are easily found on popular search engines, but one is generally
> not able to access the internet when dealing with wireless connectivity
> issues.

I don't personally see need for this, but if others find it helpful, I'm
not that strongly against either (even though this would really belong
to user space), as long as it does not do this:

> + * ieee80211_get_reason_code_string - Get human readable reason code
> + *
> + * This function returns a string describing the @reason_code.
> + *
> + * @reason_code: Reason code we want a human readable string for
> + *
> + * Return: Human readable reason string, or "(INVALID)"

That "(INVALID)" is not helpful. It is just hiding the "unknown" value.
Printing out the actual reason code is much more valuable than making
this "human readable" in this way.

> +const char *ieee80211_get_reason_code_string(u16 reason_code)
> +{
> +	if (reason_code <= 24)
> +		return reason_code_strings[reason_code];
> +	else if (reason_code >= 32 && reason_code <= 39)
> +		return reason_code_strings[reason_code - 7];
> +	else if (reason_code == 45)
> +		return reason_code_strings[33];
> +	else if (reason_code >= 52 && reason_code <= 66)
> +		return reason_code_strings[reason_code - 18];
> +	else
> +		return "(INVALID)";
> +}

This is hiding a large number of recently added reason codes.. For most
of the "human readable" strings in the reason_code_strings array, I
would end up having to find the full explanation from the standard
anyway, so I would like to be able to find this easily (and seeing the
real value of the reason code field would be the easiest way).

> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> @@ -2231,8 +2231,8 @@ static void ieee80211_rx_mgmt_deauth(struct ieee80211_sub_if_data *sdata,
> -	sdata_info(sdata, "deauthenticated from %pM (Reason: %u)\n",
> -		   bssid, reason_code);
> +	sdata_info(sdata, "deauthenticated from %pM (reason: %s)\n",
> +		   bssid, ieee80211_get_reason_code_string(reason_code));

Please don't do this unless ieee80211_get_reason_code_string() includes
the actual reason code number for every possible case, i.e., just leave
%u print of reason_code here even if the string is added.

-- 
Jouni Malinen                                            PGP id EFC895FA

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-06  4:44 ` Joe Perches
@ 2014-02-10 11:09   ` Johannes Berg
  2014-02-10 16:39     ` Joe Perches
  0 siblings, 1 reply; 21+ messages in thread
From: Johannes Berg @ 2014-02-10 11:09 UTC (permalink / raw)
  To: Joe Perches
  Cc: Calvin Owens, David S. Miller, John W. Linville, linux-wireless,
	linux-kernel

On Wed, 2014-02-05 at 20:44 -0800, Joe Perches wrote:

> Perhaps use a more common kernel style
> 
> struct ieee80211_reason_descriptions {
> 	u16		code;
> 	const char *	desc;
> }
> 
> and enumerate the reason codes with #defines and use a
> macro to populate the descriptions
> 
> #define IEEE80211_REASON_RESERVED		0
> #define IEEE80211_REASON_UNSPECIFIED		1
> 
> etc.
> 
> #define POPULATE_IEEE_REASON(code)			\
> 	{.code = IEEE80211_REASON_##code, .desc = #code}
> 
> static const struct ieee80211_reason_descriptions reasons[] = {
> 	POPULATE_IEEE_REASON(RESERVED),
> 	POPULATE_IEEE_REASON(UNSPECIFIED),
> 	[etc...]
> };
> 
> So this function becomes something like:
> 
> const char *ieee80211_get_reason_code_string(u16 reason_code)
> {
> 	int i;
> 
> 	for (i = 0; i < ARRAY_SIZE(reasons); i++) {
> 		if (reasons[i].code == reason_code)
> 			return reasons[i].desc;
> 	}

Isn't it more efficient to just let the compiler generate it with a big
switch() statement? AFAICT gcc will typically generate code like
Calvin's original hand-rolled code and/or big lookup tables anyway, if
faced with something like

switch (reason) {
case 17: return "asdf";
... // all the others
default: return "<unknown>";
};

In any case, I agree with you and Jouni about leaving the number -
that's certainly needed - but I'm willing to merge a clean patch like
this too.

johannes


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-10 11:09   ` Johannes Berg
@ 2014-02-10 16:39     ` Joe Perches
  2014-02-11  1:25       ` [PATCH v2] " Calvin Owens
  0 siblings, 1 reply; 21+ messages in thread
From: Joe Perches @ 2014-02-10 16:39 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Calvin Owens, David S. Miller, John W. Linville, linux-wireless,
	linux-kernel

On Mon, 2014-02-10 at 12:09 +0100, Johannes Berg wrote:
> On Wed, 2014-02-05 at 20:44 -0800, Joe Perches wrote:
> 
> > Perhaps use a more common kernel style
> > 
> > struct ieee80211_reason_descriptions {
> > 	u16		code;
> > 	const char *	desc;
> > }
> > 
> > and enumerate the reason codes with #defines and use a
> > macro to populate the descriptions
> > 
> > #define IEEE80211_REASON_RESERVED		0
> > #define IEEE80211_REASON_UNSPECIFIED		1
[]
> Isn't it more efficient to just let the compiler generate it with a big
> switch() statement?

That'd be fine too.

The benefit of the #defines is that you typically
get an external .h file for the entries.

Calvin's suggested code looked pretty fragile.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-10 16:39     ` Joe Perches
@ 2014-02-11  1:25       ` Calvin Owens
  2014-02-11  1:39         ` Joe Perches
  0 siblings, 1 reply; 21+ messages in thread
From: Calvin Owens @ 2014-02-11  1:25 UTC (permalink / raw)
  To: Johannes Berg, Joe Perches
  Cc: David S. Miller, John W. Linville, linux-wireless, linux-kernel,
	jcalvinowens

Create a function to return a descriptive string for each reason code,
and print that instead of the numeric value in the kernel log. These
codes are easily found on popular search engines, but one is generally
not able to access the internet when dealing with wireless connectivity
issues.

Changes since v1: Refactored array of strings into switch statement,
print numeric code in addition to string.

Signed-off-by: Calvin Owens <jcalvinowens@gmail.com>
---
 include/net/mac80211.h | 10 +++++++++
 net/mac80211/main.c    | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/mlme.c    | 12 +++++------
 3 files changed, 71 insertions(+), 6 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index f4ab2fb..d18acfe 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2971,6 +2971,16 @@ struct ieee80211_ops {
  */
 struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
 					const struct ieee80211_ops *ops);
+/**
+ * ieee80211_get_reason_code_string - Get human readable reason code
+ *
+ * This function returns a string describing the @reason_code.
+ *
+ * @reason_code: Reason code
+ *
+ * Return: Human readable reason string, or "<INVALID>"
+ */
+const char *ieee80211_get_reason_code_string(u16 reason_code);
 
 /**
  * ieee80211_register_hw - Register hardware device
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index d767cfb..3c52a81 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -743,6 +743,61 @@ static int ieee80211_init_cipher_suites(struct ieee80211_local *local)
 	return 0;
 }
 
+const char *ieee80211_get_reason_code_string(u16 reason_code)
+{
+	enum ieee80211_reasoncode r = reason_code;
+	switch(r) {
+	case WLAN_REASON_UNSPECIFIED: return "UNSPECIFIED";
+	case WLAN_REASON_PREV_AUTH_NOT_VALID: return "PREV_AUTH_NOT_VALID";
+	case WLAN_REASON_DEAUTH_LEAVING: return "DEAUTH_LEAVING";
+	case WLAN_REASON_DISASSOC_DUE_TO_INACTIVITY: return "DISASSOC_DUE_TO_INACTIVITY";
+	case WLAN_REASON_DISASSOC_AP_BUSY: return "DISASSOC_AP_BUSY";
+	case WLAN_REASON_CLASS2_FRAME_FROM_NONAUTH_STA: return "CLASS2_FRAME_FROM_NONAUTH_STA";
+	case WLAN_REASON_CLASS3_FRAME_FROM_NONASSOC_STA: return "CLASS3_FRAME_FROM_NONASSOC_STA";
+	case WLAN_REASON_DISASSOC_STA_HAS_LEFT: return "DISASSOC_STA_HAS_LEFT";
+	case WLAN_REASON_STA_REQ_ASSOC_WITHOUT_AUTH: return "STA_REQ_ASSOC_WITHOUT_AUTH";
+	case WLAN_REASON_DISASSOC_BAD_POWER: return "DISASSOC_BAD_POWER";
+	case WLAN_REASON_DISASSOC_BAD_SUPP_CHAN: return "DISASSOC_BAD_SUPP_CHAN";
+	case WLAN_REASON_INVALID_IE: return "INVALID_IE";
+	case WLAN_REASON_MIC_FAILURE: return "MIC_FAILURE";
+	case WLAN_REASON_4WAY_HANDSHAKE_TIMEOUT: return "4WAY_HANDSHAKE_TIMEOUT";
+	case WLAN_REASON_GROUP_KEY_HANDSHAKE_TIMEOUT: return "GROUP_KEY_HANDSHAKE_TIMEOUT";
+	case WLAN_REASON_IE_DIFFERENT: return "IE_DIFFERENT";
+	case WLAN_REASON_INVALID_GROUP_CIPHER: return "INVALID_GROUP_CIPHER";
+	case WLAN_REASON_INVALID_PAIRWISE_CIPHER: return "INVALID_PAIRWISE_CIPHER";
+	case WLAN_REASON_INVALID_AKMP: return "INVALID_AKMP";
+	case WLAN_REASON_UNSUPP_RSN_VERSION: return "UNSUPP_RSN_VERSION";
+	case WLAN_REASON_INVALID_RSN_IE_CAP: return "INVALID_RSN_IE_CAP";
+	case WLAN_REASON_IEEE8021X_FAILED: return "IEEE8021X_FAILED";
+	case WLAN_REASON_CIPHER_SUITE_REJECTED: return "CIPHER_SUITE_REJECTED";
+	case WLAN_REASON_DISASSOC_UNSPECIFIED_QOS: return "DISASSOC_UNSPECIFIED_QOS";
+	case WLAN_REASON_DISASSOC_QAP_NO_BANDWIDTH: return "DISASSOC_QAP_NO_BANDWIDTH";
+	case WLAN_REASON_DISASSOC_LOW_ACK: return "DISASSOC_LOW_ACK";
+	case WLAN_REASON_DISASSOC_QAP_EXCEED_TXOP: return "DISASSOC_QAP_EXCEED_TXOP";
+	case WLAN_REASON_QSTA_LEAVE_QBSS: return "QSTA_LEAVE_QBSS";
+	case WLAN_REASON_QSTA_NOT_USE: return "QSTA_NOT_USE";
+	case WLAN_REASON_QSTA_REQUIRE_SETUP: return "QSTA_REQUIRE_SETUP";
+	case WLAN_REASON_QSTA_TIMEOUT: return "QSTA_TIMEOUT";
+	case WLAN_REASON_QSTA_CIPHER_NOT_SUPP: return "QSTA_CIPHER_NOT_SUPP";
+	case WLAN_REASON_MESH_PEER_CANCELED: return "MESH_PEER_CANCELED";
+	case WLAN_REASON_MESH_MAX_PEERS: return "MESH_MAX_PEERS";
+	case WLAN_REASON_MESH_CONFIG: return "MESH_CONFIG";
+	case WLAN_REASON_MESH_CLOSE: return "MESH_CLOSE";
+	case WLAN_REASON_MESH_MAX_RETRIES: return "MESH_MAX_RETRIES";
+	case WLAN_REASON_MESH_CONFIRM_TIMEOUT: return "MESH_CONFIRM_TIMEOUT";
+	case WLAN_REASON_MESH_INVALID_GTK: return "MESH_INVALID_GTK";
+	case WLAN_REASON_MESH_INCONSISTENT_PARAM: return "MESH_INCONSISTENT_PARAM";
+	case WLAN_REASON_MESH_INVALID_SECURITY: return "MESH_INVALID_SECURITY";
+	case WLAN_REASON_MESH_PATH_ERROR: return "MESH_PATH_ERROR";
+	case WLAN_REASON_MESH_PATH_NOFORWARD: return "MESH_PATH_NOFORWARD";
+	case WLAN_REASON_MESH_PATH_DEST_UNREACHABLE: return "MESH_PATH_DEST_UNREACHABLE";
+	case WLAN_REASON_MAC_EXISTS_IN_MBSS: return "MAC_EXISTS_IN_MBSS";
+	case WLAN_REASON_MESH_CHAN_REGULATORY: return "MESH_CHAN_REGULATORY";
+	case WLAN_REASON_MESH_CHAN: return "MESH_CHAN";
+	default: return "<INVALID>";
+	}
+}
+
 int ieee80211_register_hw(struct ieee80211_hw *hw)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index fc1d824..5dec202 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2231,8 +2231,8 @@ static void ieee80211_rx_mgmt_deauth(struct ieee80211_sub_if_data *sdata,
 
 	reason_code = le16_to_cpu(mgmt->u.deauth.reason_code);
 
-	sdata_info(sdata, "deauthenticated from %pM (Reason: %u)\n",
-		   bssid, reason_code);
+	sdata_info(sdata, "deauthenticated from %pM (Reason: %u=%s)\n",
+		   bssid, reason_code, ieee80211_get_reason_code_string(reason_code));
 
 	ieee80211_set_disassoc(sdata, 0, 0, false, NULL);
 
@@ -4301,8 +4301,8 @@ int ieee80211_mgd_deauth(struct ieee80211_sub_if_data *sdata,
 	bool report_frame = false;
 
 	sdata_info(sdata,
-		   "deauthenticating from %pM by local choice (reason=%d)\n",
-		   req->bssid, req->reason_code);
+		   "deauthenticating from %pM by local choice (Reason: %u=%s)\n",
+		   req->bssid, req->reason_code, ieee80211_get_reason_code_string(req->reason_code));
 
 	if (ifmgd->auth_data) {
 		drv_mgd_prepare_tx(sdata->local, sdata);
@@ -4348,8 +4348,8 @@ int ieee80211_mgd_disassoc(struct ieee80211_sub_if_data *sdata,
 		return -ENOLINK;
 
 	sdata_info(sdata,
-		   "disassociating from %pM by local choice (reason=%d)\n",
-		   req->bss->bssid, req->reason_code);
+		   "disassociating from %pM by local choice (Reason: %u=%s)\n",
+		   req->bss->bssid, req->reason_code, ieee80211_get_reason_code_string(req->reason_code));
 
 	memcpy(bssid, req->bss->bssid, ETH_ALEN);
 	ieee80211_set_disassoc(sdata, IEEE80211_STYPE_DISASSOC,
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-11  1:25       ` [PATCH v2] " Calvin Owens
@ 2014-02-11  1:39         ` Joe Perches
  2014-02-11 16:37           ` [PATCH v3] " Calvin Owens
  0 siblings, 1 reply; 21+ messages in thread
From: Joe Perches @ 2014-02-11  1:39 UTC (permalink / raw)
  To: Calvin Owens
  Cc: Johannes Berg, David S. Miller, John W. Linville, linux-wireless,
	linux-kernel

On Mon, 2014-02-10 at 19:25 -0600, Calvin Owens wrote:
> Create a function to return a descriptive string for each reason code,
> and print that instead of the numeric value in the kernel log. These
> codes are easily found on popular search engines, but one is generally
> not able to access the internet when dealing with wireless connectivity
> issues.
> 
> Changes since v1: Refactored array of strings into switch statement,
> print numeric code in addition to string.

trivia:

> +const char *ieee80211_get_reason_code_string(u16 reason_code)
> +{
> +	enum ieee80211_reasoncode r = reason_code;

what good does this temporary do?

> +	switch(r) {

space after switch please.

> +	case WLAN_REASON_UNSPECIFIED: return "UNSPECIFIED";
> +	case WLAN_REASON_PREV_AUTH_NOT_VALID: return "PREV_AUTH_NOT_VALID";
> +	case WLAN_REASON_DEAUTH_LEAVING: return "DEAUTH_LEAVING";

This seems a bit too wall of text to me.

Maybe a simplifying macro like:

#define case_WLAN(type)				\
	case WLAN_REASON_##type: return #type

and use:
	switch (reason_code) {
	case_WLAN(UNSPECIFIED);
	case_WLAN(PREV_AUTH_NOT_VALID);

etc...



^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-11  1:39         ` Joe Perches
@ 2014-02-11 16:37           ` Calvin Owens
  2014-02-11 16:48             ` Antonio Quartulli
  2014-02-11 17:13             ` Joe Perches
  0 siblings, 2 replies; 21+ messages in thread
From: Calvin Owens @ 2014-02-11 16:37 UTC (permalink / raw)
  To: Joe Perches, Johannes Berg
  Cc: David S. Miller, John W. Linville, linux-wireless, linux-kernel,
	jcalvinowens

Create a function to return a descriptive string for each reason code,
and print that in addition to the numeric value in the kernel log. These
codes are easily found on popular search engines, but one is generally
not able to access the internet when dealing with wireless connectivity
issues.

Changes in v2: Refactored array of strings into switch statement.
Changes in v3: Fix style problem, use simplifying macro for switch
statement, eliminate temporary enum variable.

Signed-off-by: Calvin Owens <jcalvinowens@gmail.com>
---
 include/net/mac80211.h | 10 +++++++++
 net/mac80211/main.c    | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/mlme.c    | 12 +++++------
 3 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index f4ab2fb..d18acfe 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2971,6 +2971,16 @@ struct ieee80211_ops {
  */
 struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
 					const struct ieee80211_ops *ops);
+/**
+ * ieee80211_get_reason_code_string - Get human readable reason code
+ *
+ * This function returns a string describing the @reason_code.
+ *
+ * @reason_code: Reason code
+ *
+ * Return: Human readable reason string, or "<INVALID>"
+ */
+const char *ieee80211_get_reason_code_string(u16 reason_code);
 
 /**
  * ieee80211_register_hw - Register hardware device
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index d767cfb..307b444 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -743,6 +743,63 @@ static int ieee80211_init_cipher_suites(struct ieee80211_local *local)
 	return 0;
 }
 
+#define case_WLAN(type) \
+	case WLAN_REASON_##type: return #type
+
+const char *ieee80211_get_reason_code_string(u16 reason_code)
+{
+	switch (reason_code) {
+	case_WLAN(UNSPECIFIED);
+	case_WLAN(PREV_AUTH_NOT_VALID);
+	case_WLAN(DEAUTH_LEAVING);
+	case_WLAN(DISASSOC_DUE_TO_INACTIVITY);
+	case_WLAN(DISASSOC_AP_BUSY);
+	case_WLAN(CLASS2_FRAME_FROM_NONAUTH_STA);
+	case_WLAN(CLASS3_FRAME_FROM_NONASSOC_STA);
+	case_WLAN(DISASSOC_STA_HAS_LEFT);
+	case_WLAN(STA_REQ_ASSOC_WITHOUT_AUTH);
+	case_WLAN(DISASSOC_BAD_POWER);
+	case_WLAN(DISASSOC_BAD_SUPP_CHAN);
+	case_WLAN(INVALID_IE);
+	case_WLAN(MIC_FAILURE);
+	case_WLAN(4WAY_HANDSHAKE_TIMEOUT);
+	case_WLAN(GROUP_KEY_HANDSHAKE_TIMEOUT);
+	case_WLAN(IE_DIFFERENT);
+	case_WLAN(INVALID_GROUP_CIPHER);
+	case_WLAN(INVALID_PAIRWISE_CIPHER);
+	case_WLAN(INVALID_AKMP);
+	case_WLAN(UNSUPP_RSN_VERSION);
+	case_WLAN(INVALID_RSN_IE_CAP);
+	case_WLAN(IEEE8021X_FAILED);
+	case_WLAN(CIPHER_SUITE_REJECTED);
+	case_WLAN(DISASSOC_UNSPECIFIED_QOS);
+	case_WLAN(DISASSOC_QAP_NO_BANDWIDTH);
+	case_WLAN(DISASSOC_LOW_ACK);
+	case_WLAN(DISASSOC_QAP_EXCEED_TXOP);
+	case_WLAN(QSTA_LEAVE_QBSS);
+	case_WLAN(QSTA_NOT_USE);
+	case_WLAN(QSTA_REQUIRE_SETUP);
+	case_WLAN(QSTA_TIMEOUT);
+	case_WLAN(QSTA_CIPHER_NOT_SUPP);
+	case_WLAN(MESH_PEER_CANCELED);
+	case_WLAN(MESH_MAX_PEERS);
+	case_WLAN(MESH_CONFIG);
+	case_WLAN(MESH_CLOSE);
+	case_WLAN(MESH_MAX_RETRIES);
+	case_WLAN(MESH_CONFIRM_TIMEOUT);
+	case_WLAN(MESH_INVALID_GTK);
+	case_WLAN(MESH_INCONSISTENT_PARAM);
+	case_WLAN(MESH_INVALID_SECURITY);
+	case_WLAN(MESH_PATH_ERROR);
+	case_WLAN(MESH_PATH_NOFORWARD);
+	case_WLAN(MESH_PATH_DEST_UNREACHABLE);
+	case_WLAN(MAC_EXISTS_IN_MBSS);
+	case_WLAN(MESH_CHAN_REGULATORY);
+	case_WLAN(MESH_CHAN);
+	default: return "<INVALID>";
+	}
+}
+
 int ieee80211_register_hw(struct ieee80211_hw *hw)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index fc1d824..5dec202 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2231,8 +2231,8 @@ static void ieee80211_rx_mgmt_deauth(struct ieee80211_sub_if_data *sdata,
 
 	reason_code = le16_to_cpu(mgmt->u.deauth.reason_code);
 
-	sdata_info(sdata, "deauthenticated from %pM (Reason: %u)\n",
-		   bssid, reason_code);
+	sdata_info(sdata, "deauthenticated from %pM (Reason: %u=%s)\n",
+		   bssid, reason_code, ieee80211_get_reason_code_string(reason_code));
 
 	ieee80211_set_disassoc(sdata, 0, 0, false, NULL);
 
@@ -4301,8 +4301,8 @@ int ieee80211_mgd_deauth(struct ieee80211_sub_if_data *sdata,
 	bool report_frame = false;
 
 	sdata_info(sdata,
-		   "deauthenticating from %pM by local choice (reason=%d)\n",
-		   req->bssid, req->reason_code);
+		   "deauthenticating from %pM by local choice (Reason: %u=%s)\n",
+		   req->bssid, req->reason_code, ieee80211_get_reason_code_string(req->reason_code));
 
 	if (ifmgd->auth_data) {
 		drv_mgd_prepare_tx(sdata->local, sdata);
@@ -4348,8 +4348,8 @@ int ieee80211_mgd_disassoc(struct ieee80211_sub_if_data *sdata,
 		return -ENOLINK;
 
 	sdata_info(sdata,
-		   "disassociating from %pM by local choice (reason=%d)\n",
-		   req->bss->bssid, req->reason_code);
+		   "disassociating from %pM by local choice (Reason: %u=%s)\n",
+		   req->bss->bssid, req->reason_code, ieee80211_get_reason_code_string(req->reason_code));
 
 	memcpy(bssid, req->bss->bssid, ETH_ALEN);
 	ieee80211_set_disassoc(sdata, IEEE80211_STYPE_DISASSOC,
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v3] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-11 16:37           ` [PATCH v3] " Calvin Owens
@ 2014-02-11 16:48             ` Antonio Quartulli
  2014-02-11 17:59               ` Calvin Owens
  2014-02-11 18:19               ` Johannes Berg
  2014-02-11 17:13             ` Joe Perches
  1 sibling, 2 replies; 21+ messages in thread
From: Antonio Quartulli @ 2014-02-11 16:48 UTC (permalink / raw)
  To: Calvin Owens, Joe Perches, Johannes Berg
  Cc: David S. Miller, John W. Linville, linux-wireless, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1663 bytes --]

On 11/02/14 17:37, Calvin Owens wrote:
> Create a function to return a descriptive string for each reason code,
> and print that in addition to the numeric value in the kernel log. These
> codes are easily found on popular search engines, but one is generally
> not able to access the internet when dealing with wireless connectivity
> issues.
> 
> Changes in v2: Refactored array of strings into switch statement.
> Changes in v3: Fix style problem, use simplifying macro for switch
> statement, eliminate temporary enum variable.
> 
> Signed-off-by: Calvin Owens <jcalvinowens@gmail.com>
> ---
>  include/net/mac80211.h | 10 +++++++++
>  net/mac80211/main.c    | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  net/mac80211/mlme.c    | 12 +++++------
>  3 files changed, 73 insertions(+), 6 deletions(-)
> 
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index f4ab2fb..d18acfe 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -2971,6 +2971,16 @@ struct ieee80211_ops {
>   */
>  struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
>  					const struct ieee80211_ops *ops);
> +/**
> + * ieee80211_get_reason_code_string - Get human readable reason code
> + *
> + * This function returns a string describing the @reason_code.
> + *
> + * @reason_code: Reason code

Kerneldoc is not properly formatted here.
The "@argument:" clause should be on the line right after the function
name (as explained in Documentation/kernel-doc-nano-HOWTO.txt), e.g.:

/**
 * function_name - blabla
 * @arg: I am a good arg description
 *


Cheers,

-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-11 16:37           ` [PATCH v3] " Calvin Owens
  2014-02-11 16:48             ` Antonio Quartulli
@ 2014-02-11 17:13             ` Joe Perches
  2014-02-11 17:52               ` Calvin Owens
  1 sibling, 1 reply; 21+ messages in thread
From: Joe Perches @ 2014-02-11 17:13 UTC (permalink / raw)
  To: Calvin Owens
  Cc: Johannes Berg, David S. Miller, John W. Linville, linux-wireless,
	linux-kernel

On Tue, 2014-02-11 at 10:37 -0600, Calvin Owens wrote:
> Create a function to return a descriptive string for each reason code,
> and print that in addition to the numeric value in the kernel log. These
> codes are easily found on popular search engines, but one is generally
> not able to access the internet when dealing with wireless connectivity
> issues.
[]
>  include/net/mac80211.h | 10 +++++++++
>  net/mac80211/main.c    | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  net/mac80211/mlme.c    | 12 +++++------
>  3 files changed, 73 insertions(+), 6 deletions(-)

Is there a reason why all of this this shouldn't
be a static function local to mlme.c?

Is this ever going to be used somewhere else?



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-11 17:13             ` Joe Perches
@ 2014-02-11 17:52               ` Calvin Owens
  2014-02-11 18:36                 ` [PATCH v4] " Calvin Owens
  0 siblings, 1 reply; 21+ messages in thread
From: Calvin Owens @ 2014-02-11 17:52 UTC (permalink / raw)
  To: Joe Perches
  Cc: Johannes Berg, David S. Miller, John W. Linville, linux-wireless,
	linux-kernel

On Tuesday 02/11 at 09:13 -0800, Joe Perches wrote:
> On Tue, 2014-02-11 at 10:37 -0600, Calvin Owens wrote:
> > Create a function to return a descriptive string for each reason code,
> > and print that in addition to the numeric value in the kernel log. These
> > codes are easily found on popular search engines, but one is generally
> > not able to access the internet when dealing with wireless connectivity
> > issues.
> []
> >  include/net/mac80211.h | 10 +++++++++
> >  net/mac80211/main.c    | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  net/mac80211/mlme.c    | 12 +++++------
> >  3 files changed, 73 insertions(+), 6 deletions(-)
> 
> Is there a reason why all of this this shouldn't
> be a static function local to mlme.c?
> 
> Is this ever going to be used somewhere else?

The enum for the reason code is defined in "include/linux/ieee80211.h",
which is #include'd in 61 different files, so I thought it was
conceivable that it might. I also thought the compiler wasn't likely to
inline it even if it were static, since it wasn't tiny and had several
callers.

The switch statement approach produces nice, smaller code though:
(as opposed to the original with the array of strings)

ffffffff81728ac0 <ieee80211_get_reason_code_string>:
ffffffff81728ac0:       83 ef 01                sub    $0x1,%edi
ffffffff81728ac3:       55                      push   %rbp
ffffffff81728ac4:       48 c7 c0 f5 a7 a6 81    mov    $0xffffffff81a6a7f5,%rax
ffffffff81728acb:       66 83 ff 41             cmp    $0x41,%di
ffffffff81728acf:       48 89 e5                mov    %rsp,%rbp
ffffffff81728ad2:       77 0b                   ja     ffffffff81728adf <ieee80211_get_reason_code_string+0x1f>
ffffffff81728ad4:       0f b7 ff                movzwl %di,%edi
ffffffff81728ad7:       48 8b 04 fd 40 98 8e    mov -0x7e7167c0(,%rdi,8),%rax
ffffffff81728ade:       81 
ffffffff81728adf:       5d                      pop    %rbp
ffffffff81728ae0:       c3                      retq   
ffffffff81728ae1:       66 66 66 66 66 66 2e    data32 data32 data32 data32 data32 nopw %cs:0x0(%rax,%rax,1)
ffffffff81728ae8:       0f 1f 84 00 00 00 00 
ffffffff81728aef:       00 

... so it probably would get inlined. I'll make it static and resend.

Thanks,
Calvin

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-11 16:48             ` Antonio Quartulli
@ 2014-02-11 17:59               ` Calvin Owens
  2014-02-11 18:19               ` Johannes Berg
  1 sibling, 0 replies; 21+ messages in thread
From: Calvin Owens @ 2014-02-11 17:59 UTC (permalink / raw)
  To: Antonio Quartulli
  Cc: Joe Perches, Johannes Berg, David S. Miller, John W. Linville,
	linux-wireless, linux-kernel

On Tuesday 02/11 at 17:48 +0100, Antonio Quartulli wrote:
> On 11/02/14 17:37, Calvin Owens wrote:
> > Create a function to return a descriptive string for each reason code,
> > and print that in addition to the numeric value in the kernel log. These
> > codes are easily found on popular search engines, but one is generally
> > not able to access the internet when dealing with wireless connectivity
> > issues.
> > 
> > Changes in v2: Refactored array of strings into switch statement.
> > Changes in v3: Fix style problem, use simplifying macro for switch
> > statement, eliminate temporary enum variable.
> > 
> > Signed-off-by: Calvin Owens <jcalvinowens@gmail.com>
> > ---
> >  include/net/mac80211.h | 10 +++++++++
> >  net/mac80211/main.c    | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  net/mac80211/mlme.c    | 12 +++++------
> >  3 files changed, 73 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> > index f4ab2fb..d18acfe 100644
> > --- a/include/net/mac80211.h
> > +++ b/include/net/mac80211.h
> > @@ -2971,6 +2971,16 @@ struct ieee80211_ops {
> >   */
> >  struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
> >  					const struct ieee80211_ops *ops);
> > +/**
> > + * ieee80211_get_reason_code_string - Get human readable reason code
> > + *
> > + * This function returns a string describing the @reason_code.
> > + *
> > + * @reason_code: Reason code
> 
> Kerneldoc is not properly formatted here.
> The "@argument:" clause should be on the line right after the function
> name (as explained in Documentation/kernel-doc-nano-HOWTO.txt), e.g.:
> 
> /**
>  * function_name - blabla
>  * @arg: I am a good arg description
>  *

I actually just copied the comment format above the functions
surrounding the one I added, several of which also appear to be
incorrectly formatted. I can submit a patch to fix those as well if you
like: is it worth the trouble?

Thanks,
Calvin

> Cheers,
> 
> -- 
> Antonio Quartulli
> 



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-11 16:48             ` Antonio Quartulli
  2014-02-11 17:59               ` Calvin Owens
@ 2014-02-11 18:19               ` Johannes Berg
  1 sibling, 0 replies; 21+ messages in thread
From: Johannes Berg @ 2014-02-11 18:19 UTC (permalink / raw)
  To: Antonio Quartulli
  Cc: Calvin Owens, Joe Perches, David S. Miller, John W. Linville,
	linux-wireless, linux-kernel

On Tue, 2014-02-11 at 17:48 +0100, Antonio Quartulli wrote:
> On 11/02/14 17:37, Calvin Owens wrote:
> > Create a function to return a descriptive string for each reason code,
> > and print that in addition to the numeric value in the kernel log. These
> > codes are easily found on popular search engines, but one is generally
> > not able to access the internet when dealing with wireless connectivity
> > issues.
> > 
> > Changes in v2: Refactored array of strings into switch statement.
> > Changes in v3: Fix style problem, use simplifying macro for switch
> > statement, eliminate temporary enum variable.
> > 
> > Signed-off-by: Calvin Owens <jcalvinowens@gmail.com>
> > ---
> >  include/net/mac80211.h | 10 +++++++++
> >  net/mac80211/main.c    | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  net/mac80211/mlme.c    | 12 +++++------
> >  3 files changed, 73 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> > index f4ab2fb..d18acfe 100644
> > --- a/include/net/mac80211.h
> > +++ b/include/net/mac80211.h
> > @@ -2971,6 +2971,16 @@ struct ieee80211_ops {
> >   */
> >  struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
> >  					const struct ieee80211_ops *ops);
> > +/**
> > + * ieee80211_get_reason_code_string - Get human readable reason code
> > + *
> > + * This function returns a string describing the @reason_code.
> > + *
> > + * @reason_code: Reason code
> 
> Kerneldoc is not properly formatted here.
> The "@argument:" clause should be on the line right after the function
> name (as explained in Documentation/kernel-doc-nano-HOWTO.txt), e.g.:

I'm pretty sure it gets parsed correctly anyway, see e.g.
https://www.kernel.org/doc/htmldocs/80211/API-ieee80211-alloc-hw.html
(which is the function right above with the same style)

johannes


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-11 17:52               ` Calvin Owens
@ 2014-02-11 18:36                 ` Calvin Owens
  2014-02-12 10:46                   ` Johannes Berg
  0 siblings, 1 reply; 21+ messages in thread
From: Calvin Owens @ 2014-02-11 18:36 UTC (permalink / raw)
  To: Joe Perches
  Cc: Johannes Berg, David S. Miller, John W. Linville, linux-wireless,
	linux-kernel, jcalvinowens

Create a function to return a descriptive string for each reason code,
and print that in addition to the numeric value in the kernel log. These
codes are easily found on popular search engines, but one is generally
not able to access the internet when dealing with wireless connectivity
issues.

Changes in v2: Refactored array of strings into switch statement.
Changes in v3: Fix style problem, use simplifying macro for switch
statement, eliminate temporary enum variable.
Changes in v4: Move new function to net/mac80211/mlme.c and make it
static, since all of its callers are there.

Signed-off-by: Calvin Owens <jcalvinowens@gmail.com>
---
 net/mac80211/mlme.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 62 insertions(+), 6 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index fc1d824..0586d12 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2210,6 +2210,62 @@ static void ieee80211_rx_mgmt_auth(struct ieee80211_sub_if_data *sdata,
 	/* ignore frame -- wait for timeout */
 }
 
+#define case_WLAN(type) \
+	case WLAN_REASON_##type: return #type
+
+static const char *ieee80211_get_reason_code_string(u16 reason_code)
+{
+	switch (reason_code) {
+	case_WLAN(UNSPECIFIED);
+	case_WLAN(PREV_AUTH_NOT_VALID);
+	case_WLAN(DEAUTH_LEAVING);
+	case_WLAN(DISASSOC_DUE_TO_INACTIVITY);
+	case_WLAN(DISASSOC_AP_BUSY);
+	case_WLAN(CLASS2_FRAME_FROM_NONAUTH_STA);
+	case_WLAN(CLASS3_FRAME_FROM_NONASSOC_STA);
+	case_WLAN(DISASSOC_STA_HAS_LEFT);
+	case_WLAN(STA_REQ_ASSOC_WITHOUT_AUTH);
+	case_WLAN(DISASSOC_BAD_POWER);
+	case_WLAN(DISASSOC_BAD_SUPP_CHAN);
+	case_WLAN(INVALID_IE);
+	case_WLAN(MIC_FAILURE);
+	case_WLAN(4WAY_HANDSHAKE_TIMEOUT);
+	case_WLAN(GROUP_KEY_HANDSHAKE_TIMEOUT);
+	case_WLAN(IE_DIFFERENT);
+	case_WLAN(INVALID_GROUP_CIPHER);
+	case_WLAN(INVALID_PAIRWISE_CIPHER);
+	case_WLAN(INVALID_AKMP);
+	case_WLAN(UNSUPP_RSN_VERSION);
+	case_WLAN(INVALID_RSN_IE_CAP);
+	case_WLAN(IEEE8021X_FAILED);
+	case_WLAN(CIPHER_SUITE_REJECTED);
+	case_WLAN(DISASSOC_UNSPECIFIED_QOS);
+	case_WLAN(DISASSOC_QAP_NO_BANDWIDTH);
+	case_WLAN(DISASSOC_LOW_ACK);
+	case_WLAN(DISASSOC_QAP_EXCEED_TXOP);
+	case_WLAN(QSTA_LEAVE_QBSS);
+	case_WLAN(QSTA_NOT_USE);
+	case_WLAN(QSTA_REQUIRE_SETUP);
+	case_WLAN(QSTA_TIMEOUT);
+	case_WLAN(QSTA_CIPHER_NOT_SUPP);
+	case_WLAN(MESH_PEER_CANCELED);
+	case_WLAN(MESH_MAX_PEERS);
+	case_WLAN(MESH_CONFIG);
+	case_WLAN(MESH_CLOSE);
+	case_WLAN(MESH_MAX_RETRIES);
+	case_WLAN(MESH_CONFIRM_TIMEOUT);
+	case_WLAN(MESH_INVALID_GTK);
+	case_WLAN(MESH_INCONSISTENT_PARAM);
+	case_WLAN(MESH_INVALID_SECURITY);
+	case_WLAN(MESH_PATH_ERROR);
+	case_WLAN(MESH_PATH_NOFORWARD);
+	case_WLAN(MESH_PATH_DEST_UNREACHABLE);
+	case_WLAN(MAC_EXISTS_IN_MBSS);
+	case_WLAN(MESH_CHAN_REGULATORY);
+	case_WLAN(MESH_CHAN);
+	default: return "<INVALID>";
+	}
+}
 
 static void ieee80211_rx_mgmt_deauth(struct ieee80211_sub_if_data *sdata,
 				     struct ieee80211_mgmt *mgmt, size_t len)
@@ -2231,8 +2287,8 @@ static void ieee80211_rx_mgmt_deauth(struct ieee80211_sub_if_data *sdata,
 
 	reason_code = le16_to_cpu(mgmt->u.deauth.reason_code);
 
-	sdata_info(sdata, "deauthenticated from %pM (Reason: %u)\n",
-		   bssid, reason_code);
+	sdata_info(sdata, "deauthenticated from %pM (Reason: %u=%s)\n",
+		   bssid, reason_code, ieee80211_get_reason_code_string(reason_code));
 
 	ieee80211_set_disassoc(sdata, 0, 0, false, NULL);
 
@@ -4301,8 +4357,8 @@ int ieee80211_mgd_deauth(struct ieee80211_sub_if_data *sdata,
 	bool report_frame = false;
 
 	sdata_info(sdata,
-		   "deauthenticating from %pM by local choice (reason=%d)\n",
-		   req->bssid, req->reason_code);
+		   "deauthenticating from %pM by local choice (Reason: %u=%s)\n",
+		   req->bssid, req->reason_code, ieee80211_get_reason_code_string(req->reason_code));
 
 	if (ifmgd->auth_data) {
 		drv_mgd_prepare_tx(sdata->local, sdata);
@@ -4348,8 +4404,8 @@ int ieee80211_mgd_disassoc(struct ieee80211_sub_if_data *sdata,
 		return -ENOLINK;
 
 	sdata_info(sdata,
-		   "disassociating from %pM by local choice (reason=%d)\n",
-		   req->bss->bssid, req->reason_code);
+		   "disassociating from %pM by local choice (Reason: %u=%s)\n",
+		   req->bss->bssid, req->reason_code, ieee80211_get_reason_code_string(req->reason_code));
 
 	memcpy(bssid, req->bss->bssid, ETH_ALEN);
 	ieee80211_set_disassoc(sdata, IEEE80211_STYPE_DISASSOC,
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v4] ieee80211: Print human-readable disassoc/deauth reason codes
  2014-02-11 18:36                 ` [PATCH v4] " Calvin Owens
@ 2014-02-12 10:46                   ` Johannes Berg
  0 siblings, 0 replies; 21+ messages in thread
From: Johannes Berg @ 2014-02-12 10:46 UTC (permalink / raw)
  To: Calvin Owens
  Cc: Joe Perches, David S. Miller, John W. Linville, linux-wireless,
	linux-kernel

On Tue, 2014-02-11 at 12:36 -0600, Calvin Owens wrote:
> Create a function to return a descriptive string for each reason code,
> and print that in addition to the numeric value in the kernel log. These
> codes are easily found on popular search engines, but one is generally
> not able to access the internet when dealing with wireless connectivity
> issues.

Applied.

> Changes in v2: Refactored array of strings into switch statement.
> Changes in v3: Fix style problem, use simplifying macro for switch
> statement, eliminate temporary enum variable.
> Changes in v4: Move new function to net/mac80211/mlme.c and make it
> static, since all of its callers are there.

I removed all this, it belongs after the --- imho.

> +	default: return "<INVALID>";

There are many more valid reason codes that this function doesn't
understand, so <invalid> is misleading, I replaced that by "<unknown>". 

johannes


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2014-02-12 10:46 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-06  1:44 [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes Calvin Owens
2014-02-06  4:44 ` Joe Perches
2014-02-10 11:09   ` Johannes Berg
2014-02-10 16:39     ` Joe Perches
2014-02-11  1:25       ` [PATCH v2] " Calvin Owens
2014-02-11  1:39         ` Joe Perches
2014-02-11 16:37           ` [PATCH v3] " Calvin Owens
2014-02-11 16:48             ` Antonio Quartulli
2014-02-11 17:59               ` Calvin Owens
2014-02-11 18:19               ` Johannes Berg
2014-02-11 17:13             ` Joe Perches
2014-02-11 17:52               ` Calvin Owens
2014-02-11 18:36                 ` [PATCH v4] " Calvin Owens
2014-02-12 10:46                   ` Johannes Berg
2014-02-06  8:37 ` [PATCH] " Johannes Berg
2014-02-07 12:53   ` Kalle Valo
2014-02-07 15:46     ` Larry Finger
2014-02-07 22:25       ` Luca Coelho
2014-02-08  6:38         ` Kalle Valo
2014-02-07 20:50   ` Calvin Owens
2014-02-10  8:50 ` Jouni Malinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.