Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
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_UNSPECIFIED1
[]
> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
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 "";
};
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
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 MalinenPGP id EFC895FA
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
Luca Coelho 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
On Fri, 2014-02-07 at 09:46 -0600, Larry Finger wrote: > On 02/07/2014 06:53 AM, Kalle Valo wrote: > > Johannes Berg 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
On 02/07/2014 06:53 AM, Kalle Valo wrote: Johannes Berg 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
Johannes Berg 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
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_UNSPECIFIED1
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ieee80211: Print human-readable disassoc/deauth reason codes
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
---
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->
