2018-04-19 09:18:14

by Toke Høiland-Jørgensen

[permalink] [raw]
Subject: [PATCH] reg: Rename confusing 'country IE' in log output

The 'country IE' messages in the log can be confusing and make people think
that the country code has been set to Ireland. Fix this by changing the
log messages to use 'country element' instead (as they are no longer called
'information element' in the spec anyway).

Reported-by: Bernhard Gabler <[email protected]>
Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
---
net/wireless/reg.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 16c7e4ef5820..ecfee5f06c76 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1652,7 +1652,7 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
case NL80211_REGDOM_SET_BY_DRIVER:
return "driver";
case NL80211_REGDOM_SET_BY_COUNTRY_IE:
- return "country IE";
+ return "country element";
default:
WARN_ON(1);
return "bug";
@@ -2618,7 +2618,7 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
* This doesn't happen yet, not sure we
* ever want to support it for this case.
*/
- WARN_ONCE(1, "Unexpected intersection for country IEs");
+ WARN_ONCE(1, "Unexpected intersection for country elements");
return REG_REQ_IGNORE;
}

--
2.16.3


2018-04-20 09:12:11

by Toke Høiland-Jørgensen

[permalink] [raw]
Subject: Re: [PATCH] reg: Rename confusing 'country IE' in log output

Johannes Berg <[email protected]> writes:

> On Fri, 2018-04-20 at 00:32 +0200, Bernhard Gabler wrote:
>
>> looking at the patch, I see more occurences of the outdated "IE" in a
>> constant name and function name:
>>
>> case NL80211_REGDOM_SET_BY_COUNTRY_IE:
>> or
>> reg_process_hint_country_ie()
>
>
> Right. I searched for further strings that would show up to the user,
> but didn't see any, but the code still refers to it in many places.
>
>> Would it make sense to likewise fix these (and possibly more)
>> occurrences too, to match the spec from 2016?
>> For the sake of readability and consistency of code ...
>
> I dunno. Country IE isn't the only place where "IE" is used, it's even
> used in struct names (struct ieee80211_tim_ie, and many others), so I
> would think that people who actually work on the code will have to be
> aware of the previous wording of "Information Element" (and the
> abbreviation IE) anyway. Changing *everything* here to be just _element
> instead of _ie or then probably the shorter _elem would IMHO be too much
> code churn.

Also, no one is going to think that reg_process_hint_country_ie() has
anything to do with Ireland. So it's just a matter of outdated
terminology...

-Toke

2018-04-19 22:32:55

by Bernhard Gabler

[permalink] [raw]
Subject: Re: [PATCH] reg: Rename confusing 'country IE' in log output

Hello Toke & all,

looking at the patch, I see more occurences of the outdated "IE" in a
constant name and function name:

        case NL80211_REGDOM_SET_BY_COUNTRY_IE:
or
        reg_process_hint_country_ie()

Would it make sense to likewise fix these (and possibly more)
occurrences too, to match the spec from 2016?
For the sake of readability and consistency of code ...

Bernhard


Am 19.04.2018 um 11:17 schrieb Toke Høiland-Jørgensen:

> The 'country IE' messages in the log can be confusing and make people think
> that the country code has been set to Ireland. Fix this by changing the
> log messages to use 'country element' instead (as they are no longer called
> 'information element' in the spec anyway).
>
> Reported-by: Bernhard Gabler <[email protected]>
> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
> ---
> net/wireless/reg.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index 16c7e4ef5820..ecfee5f06c76 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -1652,7 +1652,7 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
> case NL80211_REGDOM_SET_BY_DRIVER:
> return "driver";
> case NL80211_REGDOM_SET_BY_COUNTRY_IE:
> - return "country IE";
> + return "country element";
> default:
> WARN_ON(1);
> return "bug";
> @@ -2618,7 +2618,7 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
> * This doesn't happen yet, not sure we
> * ever want to support it for this case.
> */
> - WARN_ONCE(1, "Unexpected intersection for country IEs");
> + WARN_ONCE(1, "Unexpected intersection for country elements");
> return REG_REQ_IGNORE;
> }
>

2018-04-20 06:55:59

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] reg: Rename confusing 'country IE' in log output

On Fri, 2018-04-20 at 00:32 +0200, Bernhard Gabler wrote:

> looking at the patch, I see more occurences of the outdated "IE" in a
> constant name and function name:
>
> case NL80211_REGDOM_SET_BY_COUNTRY_IE:
> or
> reg_process_hint_country_ie()


Right. I searched for further strings that would show up to the user,
but didn't see any, but the code still refers to it in many places.

> Would it make sense to likewise fix these (and possibly more)
> occurrences too, to match the spec from 2016?
> For the sake of readability and consistency of code ...

I dunno. Country IE isn't the only place where "IE" is used, it's even
used in struct names (struct ieee80211_tim_ie, and many others), so I
would think that people who actually work on the code will have to be
aware of the previous wording of "Information Element" (and the
abbreviation IE) anyway. Changing *everything* here to be just _element
instead of _ie or then probably the shorter _elem would IMHO be too much
code churn.

johannes

2018-04-23 19:10:54

by Sebastian Gottschall

[permalink] [raw]
Subject: mac80211: db3bdcb9c3ffc628c5284d7ed03a704295ba1214 breaks wds operation

just for your information. while testing your latest mac80211 tree i
found out that
db3bdcb9c3ffc628c5284d7ed03a704295ba1214
will break wds sta / ap support (tested in ath10k)
i understand what this patch does. but this patch should not find its
way into mac80211 without a patch to all affected drivers
i'm pretty sure that problem will not just belong to ath10k. basicly all
SW_CRYPTO_CONTROL enabled devices wont support wds anymore with that patch
beside this your patch description is wrong. ap_vlan which is used for
wds sta / ap operation works with encryption in ath10k and is not
related in any way to sw_crypto support

Sebastian



Am 20.04.2018 um 08:55 schrieb Johannes Berg:
> On Fri, 2018-04-20 at 00:32 +0200, Bernhard Gabler wrote:
>
>> looking at the patch, I see more occurences of the outdated "IE" in a
>> constant name and function name:
>>
>> case NL80211_REGDOM_SET_BY_COUNTRY_IE:
>> or
>> reg_process_hint_country_ie()
>
> Right. I searched for further strings that would show up to the user,
> but didn't see any, but the code still refers to it in many places.
>
>> Would it make sense to likewise fix these (and possibly more)
>> occurrences too, to match the spec from 2016?
>> For the sake of readability and consistency of code ...
> I dunno. Country IE isn't the only place where "IE" is used, it's even
> used in struct names (struct ieee80211_tim_ie, and many others), so I
> would think that people who actually work on the code will have to be
> aware of the previous wording of "Information Element" (and the
> abbreviation IE) anyway. Changing *everything* here to be just _element
> instead of _ie or then probably the shorter _elem would IMHO be too much
> code churn.
>
> johannes
>

--
Mit freundlichen Grüssen / Regards

Sebastian Gottschall / CTO

NewMedia-NET GmbH - DD-WRT
Firmensitz: Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: [email protected]
Tel.: +496251-582650 / Fax: +496251-5826565