[PATCH] ieee80211-crypt: Make some TKIP and CCMP error logging conditional on IEEE80211_DEBUG_DROP

[PATCH] ieee80211-crypt: Make some TKIP and CCMP error logging conditional on IEEE80211_DEBUG_DROP

[Larry Finger]

There are messages arising from ieee80211_crypt that spam the logs of
casual users. These are changed to be logged only if the user specifically
requests the IEEE80211_DEBUG_DROP messages. In either case, the error/drop count
is incremented.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---

Index: wireless-2.6/net/ieee80211/ieee80211_crypt_tkip.c
===================================================================
--- wireless-2.6.orig/net/ieee80211/ieee80211_crypt_tkip.c
+++ wireless-2.6/net/ieee80211/ieee80211_crypt_tkip.c
@@ -465,7 +465,7 @@ static int ieee80211_tkip_decrypt(struct
 
 	if (tkip_replay_check(iv32, iv16, tkey->rx_iv32, tkey->rx_iv16)) {
 		if (net_ratelimit()) {
-			printk(KERN_DEBUG "TKIP: replay detected: STA=" MAC_FMT
+			IEEE80211_DEBUG_DROP("TKIP: replay detected: STA=" MAC_FMT
 			       " previous TSC %08x%04x received TSC "
 			       "%08x%04x\n", MAC_ARG(hdr->addr2),
 			       tkey->rx_iv32, tkey->rx_iv16, iv32, iv16);
@@ -507,7 +507,7 @@ static int ieee80211_tkip_decrypt(struct
 			tkey->rx_phase1_done = 0;
 		}
 		if (net_ratelimit()) {
-			printk(KERN_DEBUG "TKIP: ICV error detected: STA="
+			IEEE80211_DEBUG_DROP("TKIP: ICV error detected: STA="
 			       MAC_FMT "\n", MAC_ARG(hdr->addr2));
 		}
 		tkey->dot11RSNAStatsTKIPICVErrors++;
Index: wireless-2.6/net/ieee80211/ieee80211_crypt_ccmp.c
===================================================================
--- wireless-2.6.orig/net/ieee80211/ieee80211_crypt_ccmp.c
+++ wireless-2.6/net/ieee80211/ieee80211_crypt_ccmp.c
@@ -338,7 +338,7 @@ static int ieee80211_ccmp_decrypt(struct
 
 	if (ccmp_replay_check(pn, key->rx_pn)) {
 		if (net_ratelimit()) {
-			printk(KERN_DEBUG "CCMP: replay detected: STA=" MAC_FMT
+			IEEE80211_DEBUG_DROP("CCMP: replay detected: STA=" MAC_FMT
 			       " previous PN %02x%02x%02x%02x%02x%02x "
 			       "received PN %02x%02x%02x%02x%02x%02x\n",
 			       MAC_ARG(hdr->addr2), MAC_ARG(key->rx_pn),

Re: [PATCH] ieee80211-crypt: Make some TKIP and CCMP error logging conditional on IEEE80211_DEBUG_DROP

[Andreas Schwab]

Larry Finger <Larry.Finger@lwfinger.net> writes:

> There are messages arising from ieee80211_crypt that spam the logs of
> casual users. These are changed to be logged only if the user specifi=
cally
> requests the IEEE80211_DEBUG_DROP messages. In either case, the error=
/drop count
> is incremented.
>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
>
> Index: wireless-2.6/net/ieee80211/ieee80211_crypt_tkip.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- wireless-2.6.orig/net/ieee80211/ieee80211_crypt_tkip.c
> +++ wireless-2.6/net/ieee80211/ieee80211_crypt_tkip.c
> @@ -465,7 +465,7 @@ static int ieee80211_tkip_decrypt(struct
> =20
>  	if (tkip_replay_check(iv32, iv16, tkey->rx_iv32, tkey->rx_iv16)) {
>  		if (net_ratelimit()) {
> -			printk(KERN_DEBUG "TKIP: replay detected: STA=3D" MAC_FMT
> +			IEEE80211_DEBUG_DROP("TKIP: replay detected: STA=3D" MAC_FMT
>  			       " previous TSC %08x%04x received TSC "
>  			       "%08x%04x\n", MAC_ARG(hdr->addr2),
>  			       tkey->rx_iv32, tkey->rx_iv16, iv32, iv16);
> @@ -507,7 +507,7 @@ static int ieee80211_tkip_decrypt(struct
>  			tkey->rx_phase1_done =3D 0;
>  		}
>  		if (net_ratelimit()) {
> -			printk(KERN_DEBUG "TKIP: ICV error detected: STA=3D"
> +			IEEE80211_DEBUG_DROP("TKIP: ICV error detected: STA=3D"
>  			       MAC_FMT "\n", MAC_ARG(hdr->addr2));
>  		}
>  		tkey->dot11RSNAStatsTKIPICVErrors++;
> Index: wireless-2.6/net/ieee80211/ieee80211_crypt_ccmp.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- wireless-2.6.orig/net/ieee80211/ieee80211_crypt_ccmp.c
> +++ wireless-2.6/net/ieee80211/ieee80211_crypt_ccmp.c
> @@ -338,7 +338,7 @@ static int ieee80211_ccmp_decrypt(struct
> =20
>  	if (ccmp_replay_check(pn, key->rx_pn)) {
>  		if (net_ratelimit()) {
> -			printk(KERN_DEBUG "CCMP: replay detected: STA=3D" MAC_FMT
> +			IEEE80211_DEBUG_DROP("CCMP: replay detected: STA=3D" MAC_FMT
>  			       " previous PN %02x%02x%02x%02x%02x%02x "
>  			       "received PN %02x%02x%02x%02x%02x%02x\n",
>  			       MAC_ARG(hdr->addr2), MAC_ARG(key->rx_pn),
>
>

This is causing an undefined refrence to ieee80211_debug_level with
CONFIG_IEEE80211_DEBUG=3Dy and CONFIG_IEEE80211_CRYPT_CCMP=3Dm and/or
CONFIG_IEEE80211_CRYPT_TKIP=3Dm.

WARNING: "ieee80211_debug_level" [net/ieee80211/ieee80211_crypt_tkip.ko=
] undefined!
WARNING: "ieee80211_debug_level" [net/ieee80211/ieee80211_crypt_ccmp.ko=
] undefined!

Andreas.

--=20
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstra=DFe 5, 90409 N=FCrnberg, Germany
PGP key fingerprint =3D 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4=
ED5
"And now for something completely different."
-
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ieee80211-crypt: Make some TKIP and CCMP error logging conditional on IEEE80211_DEBUG_DROP

[Larry Finger]

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

Andreas Schwab wrote:
> 
> This is causing an undefined refrence to ieee80211_debug_level with
> CONFIG_IEEE80211_DEBUG=y and CONFIG_IEEE80211_CRYPT_CCMP=m and/or
> CONFIG_IEEE80211_CRYPT_TKIP=m.
> 
> WARNING: "ieee80211_debug_level" [net/ieee80211/ieee80211_crypt_tkip.ko] undefined!
> WARNING: "ieee80211_debug_level" [net/ieee80211/ieee80211_crypt_ccmp.ko] undefined!
> 
> Andreas.

The missing symbol was not exported by ieee80211_module.c. Sorry for not testing enough. The fix is 
as follows:

Index: wireless-2.6/net/ieee80211/ieee80211_module.c
===================================================================
--- wireless-2.6.orig/net/ieee80211/ieee80211_module.c
+++ wireless-2.6/net/ieee80211/ieee80211_module.c
@@ -229,6 +229,7 @@ void free_ieee80211(struct net_device *d

  static int debug = 0;
  u32 ieee80211_debug_level = 0;
+EXPORT_SYMBOL_GPL(ieee80211_debug_level);
  static struct proc_dir_entry *ieee80211_proc = NULL;

  static int show_debug_level(char *page, char **start, off_t offset,



[-- Attachment #2: missing_debug --]
[-- Type: text/plain, Size: 513 bytes --]

Index: wireless-2.6/net/ieee80211/ieee80211_module.c
===================================================================
--- wireless-2.6.orig/net/ieee80211/ieee80211_module.c
+++ wireless-2.6/net/ieee80211/ieee80211_module.c
@@ -229,6 +229,7 @@ void free_ieee80211(struct net_device *d
 
 static int debug = 0;
 u32 ieee80211_debug_level = 0;
+EXPORT_SYMBOL_GPL(ieee80211_debug_level);
 static struct proc_dir_entry *ieee80211_proc = NULL;
 
 static int show_debug_level(char *page, char **start, off_t offset,

Re: [PATCH] ieee80211-crypt: Make some TKIP and CCMP error logging conditional on IEEE80211_DEBUG_DROP

[Michael Buesch]

On Monday 16 April 2007 20:50, Larry Finger wrote:
> Andreas Schwab wrote:
> > 
> > This is causing an undefined refrence to ieee80211_debug_level with
> > CONFIG_IEEE80211_DEBUG=y and CONFIG_IEEE80211_CRYPT_CCMP=m and/or
> > CONFIG_IEEE80211_CRYPT_TKIP=m.
> > 
> > WARNING: "ieee80211_debug_level" [net/ieee80211/ieee80211_crypt_tkip.ko] undefined!
> > WARNING: "ieee80211_debug_level" [net/ieee80211/ieee80211_crypt_ccmp.ko] undefined!
> > 
> > Andreas.
> 
> The missing symbol was not exported by ieee80211_module.c. Sorry for not testing enough. The fix is 
> as follows:
> 
> Index: wireless-2.6/net/ieee80211/ieee80211_module.c
> ===================================================================
> --- wireless-2.6.orig/net/ieee80211/ieee80211_module.c
> +++ wireless-2.6/net/ieee80211/ieee80211_module.c
> @@ -229,6 +229,7 @@ void free_ieee80211(struct net_device *d
> 
>   static int debug = 0;
>   u32 ieee80211_debug_level = 0;
> +EXPORT_SYMBOL_GPL(ieee80211_debug_level);

We don't use the _GPL suffix in mac80211.

>   static struct proc_dir_entry *ieee80211_proc = NULL;
> 
>   static int show_debug_level(char *page, char **start, off_t offset,
> 
> 
> 

-- 
Greetings Michael.

Re: [PATCH] ieee80211-crypt: Make some TKIP and CCMP error logging conditional on IEEE80211_DEBUG_DROP

[Larry Finger]

Michael Buesch wrote:
> On Monday 16 April 2007 20:50, Larry Finger wrote:
>> Andreas Schwab wrote:
>>> This is causing an undefined refrence to ieee80211_debug_level with
>>> CONFIG_IEEE80211_DEBUG=y and CONFIG_IEEE80211_CRYPT_CCMP=m and/or
>>> CONFIG_IEEE80211_CRYPT_TKIP=m.
>>>
>>> WARNING: "ieee80211_debug_level" [net/ieee80211/ieee80211_crypt_tkip.ko] undefined!
>>> WARNING: "ieee80211_debug_level" [net/ieee80211/ieee80211_crypt_ccmp.ko] undefined!
>>>
>>> Andreas.
>> The missing symbol was not exported by ieee80211_module.c. Sorry for not testing enough. The fix is 
>> as follows:
>>
>> Index: wireless-2.6/net/ieee80211/ieee80211_module.c
>> ===================================================================
>> --- wireless-2.6.orig/net/ieee80211/ieee80211_module.c
>> +++ wireless-2.6/net/ieee80211/ieee80211_module.c
>> @@ -229,6 +229,7 @@ void free_ieee80211(struct net_device *d
>>
>>   static int debug = 0;
>>   u32 ieee80211_debug_level = 0;
>> +EXPORT_SYMBOL_GPL(ieee80211_debug_level);
> 
> We don't use the _GPL suffix in mac80211.
> 

Upon inspection, neither does most of ieee80211. It is now changed.

Larry

Re: [PATCH] ieee80211-crypt: Make some TKIP and CCMP error logging conditional on IEEE80211_DEBUG_DROP

[John W. Linville]

On Mon, Apr 16, 2007 at 07:24:14PM -0500, Larry Finger wrote:
> Michael Buesch wrote:
> > On Monday 16 April 2007 20:50, Larry Finger wrote:

> >> @@ -229,6 +229,7 @@ void free_ieee80211(struct net_device *d
> >>
> >>   static int debug = 0;
> >>   u32 ieee80211_debug_level = 0;
> >> +EXPORT_SYMBOL_GPL(ieee80211_debug_level);
> > 
> > We don't use the _GPL suffix in mac80211.
> 
> Upon inspection, neither does most of ieee80211. It is now changed.

You are strongly encouraged to use the _GPL version for new symbol
exports, especially those which are fundamentally internal to
in-kernel subsystems and/or have no reasonable usage by drivers.
FWIW, this symbol would seem to fulfill both of those criteria.

If you do not object, I would prefer the _GPL version of the patch.

John
-- 
John W. Linville
linville@tuxdriver.com

Re: [PATCH] ieee80211-crypt: Make some TKIP and CCMP error logging conditional on IEEE80211_DEBUG_DROP

[Dan Williams]

On Tue, 2007-04-17 at 09:12 -0400, John W. Linville wrote:
> On Mon, Apr 16, 2007 at 07:24:14PM -0500, Larry Finger wrote:
> > Michael Buesch wrote:
> > > On Monday 16 April 2007 20:50, Larry Finger wrote:
> 
> > >> @@ -229,6 +229,7 @@ void free_ieee80211(struct net_device *d
> > >>
> > >>   static int debug = 0;
> > >>   u32 ieee80211_debug_level = 0;
> > >> +EXPORT_SYMBOL_GPL(ieee80211_debug_level);
> > > 
> > > We don't use the _GPL suffix in mac80211.
> > 
> > Upon inspection, neither does most of ieee80211. It is now changed.
> 
> You are strongly encouraged to use the _GPL version for new symbol
> exports, especially those which are fundamentally internal to
> in-kernel subsystems and/or have no reasonable usage by drivers.
> FWIW, this symbol would seem to fulfill both of those criteria.
> 
> If you do not object, I would prefer the _GPL version of the patch.

What's the rationale for mac80211 _not_ using _GPL exports? I thought
most new exports were pretty much required to be _GPL (otherwise
somebody would NAK it) unless it was really, really necessary that they
weren't.

Dan

Re: [PATCH] ieee80211-crypt: Make some TKIP and CCMP error logging conditional on IEEE80211_DEBUG_DROP

[John W. Linville]

On Tue, Apr 17, 2007 at 10:25:08AM -0400, Dan Williams wrote:
> On Tue, 2007-04-17 at 09:12 -0400, John W. Linville wrote:
> > On Mon, Apr 16, 2007 at 07:24:14PM -0500, Larry Finger wrote:
> > > Michael Buesch wrote:
> > > > On Monday 16 April 2007 20:50, Larry Finger wrote:
> > 
> > > >> @@ -229,6 +229,7 @@ void free_ieee80211(struct net_device *d
> > > >>
> > > >>   static int debug = 0;
> > > >>   u32 ieee80211_debug_level = 0;
> > > >> +EXPORT_SYMBOL_GPL(ieee80211_debug_level);
> > > > 
> > > > We don't use the _GPL suffix in mac80211.
> > > 
> > > Upon inspection, neither does most of ieee80211. It is now changed.
> > 
> > You are strongly encouraged to use the _GPL version for new symbol
> > exports, especially those which are fundamentally internal to
> > in-kernel subsystems and/or have no reasonable usage by drivers.
> > FWIW, this symbol would seem to fulfill both of those criteria.
> > 
> > If you do not object, I would prefer the _GPL version of the patch.
> 
> What's the rationale for mac80211 _not_ using _GPL exports? I thought
> most new exports were pretty much required to be _GPL (otherwise
> somebody would NAK it) unless it was really, really necessary that they
> weren't.

An argument against _GPL exports for mac80211 might be leaving the
exports alone as a token of gratitude or respect towards Devicescape
for having seeded the development of mac80211 with a big chunk of code.
While I do thank Devicescape for their support, I'm not sure that
this argument would be truly compelling.

A more presuasive argument in favor of this pragmatism is that
it would be counter-productive to discourage driver availability.
At this point regulatory issues are still enough of a spectre that
some vendors will want the option of offering non-GPL drivers.
Such drivers would clearly not be redistributable, but there are
arguments that allow for such drivers (i.e. "the user installed
the driver -- not us", etc like Nvidia video drivers).  Of course,
no one likes enabling this kind of "bad behaviour".

Probably the best reason in favor of leaving them as-is is that they
were written that way by their original author(s).

Should I ask for opinionated discussion on the matter? :-)

John
-- 
John W. Linville
linville@tuxdriver.com

Re: [PATCH] ieee80211-crypt: Make some TKIP and CCMP error loggingconditional on IEEE80211_DEBUG_DROP

[Jouni Malinen]

On Tue, Apr 17, 2007 at 10:25:08AM -0400, Dan Williams wrote:

> What's the rationale for mac80211 _not_ using _GPL exports? I thought
> most new exports were pretty much required to be _GPL (otherwise
> somebody would NAK it) unless it was really, really necessary that they
> weren't.

Where is this kind of "requirement" documented? I was under the
impression that in general, the authors could decide how the symbols
gets exported as far as using _GPL or not is concerned.

My personal view on this _GPL in exports is that it is just unnecessary
extra complexity in the implementation and there should not be any place
for license enforcement in the kernel implementation; this is better
left for the license text and things completely outside the source code.
As such, I have a preference of not seeing _GPL added to code that is
derived from anything I've written.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH] ieee80211-crypt: Make some TKIP and CCMP error logging conditional on IEEE80211_DEBUG_DROP

[Dan Williams]

On Tue, 2007-04-17 at 11:21 -0400, John W. Linville wrote:
> On Tue, Apr 17, 2007 at 10:25:08AM -0400, Dan Williams wrote:
> > On Tue, 2007-04-17 at 09:12 -0400, John W. Linville wrote:
> > > On Mon, Apr 16, 2007 at 07:24:14PM -0500, Larry Finger wrote:
> > > > Michael Buesch wrote:
> > > > > On Monday 16 April 2007 20:50, Larry Finger wrote:
> > > 
> > > > >> @@ -229,6 +229,7 @@ void free_ieee80211(struct net_device *d
> > > > >>
> > > > >>   static int debug = 0;
> > > > >>   u32 ieee80211_debug_level = 0;
> > > > >> +EXPORT_SYMBOL_GPL(ieee80211_debug_level);
> > > > > 
> > > > > We don't use the _GPL suffix in mac80211.
> > > > 
> > > > Upon inspection, neither does most of ieee80211. It is now changed.
> > > 
> > > You are strongly encouraged to use the _GPL version for new symbol
> > > exports, especially those which are fundamentally internal to
> > > in-kernel subsystems and/or have no reasonable usage by drivers.
> > > FWIW, this symbol would seem to fulfill both of those criteria.
> > > 
> > > If you do not object, I would prefer the _GPL version of the patch.
> > 
> > What's the rationale for mac80211 _not_ using _GPL exports? I thought
> > most new exports were pretty much required to be _GPL (otherwise
> > somebody would NAK it) unless it was really, really necessary that they
> > weren't.
> 
> An argument against _GPL exports for mac80211 might be leaving the
> exports alone as a token of gratitude or respect towards Devicescape
> for having seeded the development of mac80211 with a big chunk of code.
> While I do thank Devicescape for their support, I'm not sure that
> this argument would be truly compelling.
> 
> A more presuasive argument in favor of this pragmatism is that
> it would be counter-productive to discourage driver availability.
> At this point regulatory issues are still enough of a spectre that
> some vendors will want the option of offering non-GPL drivers.
> Such drivers would clearly not be redistributable, but there are
> arguments that allow for such drivers (i.e. "the user installed
> the driver -- not us", etc like Nvidia video drivers).  Of course,
> no one likes enabling this kind of "bad behaviour".

Completely agree; my observations are based on mails from people like
gregkh and christoph h (who seem to be most vocal in this area), and
they alone are certainly not representative of kernel policy.  I've seen
more than a few things NAK-ed by various people due to symbol exports,
or at least serious questions raised about _why_ they are non-GPL.  So
we'd better at least be able to come up with reasons why one was chosen
over the other.

I don't particularly care one way or the other.

I guess the only way we find out is if somebody speaks up when a merge
happens :)

Dan

> Probably the best reason in favor of leaving them as-is is that they
> were written that way by their original author(s).
> 
> Should I ask for opinionated discussion on the matter? :-)
> 
> John

Re: [PATCH] ieee80211-crypt: Make some TKIP and CCMP error loggingconditional on IEEE80211_DEBUG_DROP

[Michael Buesch]

On Tuesday 17 April 2007 17:41, Jouni Malinen wrote:
> My personal view on this _GPL in exports is that it is just unnecessary
> extra complexity in the implementation and there should not be any place
> for license enforcement in the kernel implementation; this is better
> left for the license text and things completely outside the source code.
> As such, I have a preference of not seeing _GPL added to code that is
> derived from anything I've written.

I clearly second that opinion and I thought we (the main people working
on wireless) had decided to _not_ use GPL syms in mac80211 (was d80211).
In my opinion license policy should not be represented in sourcecode.
Source is source, license is license. That are two absolutely seperate
things to me. For example, if I want to relicense some code (as
a copyright holder), I clearly don't want to change sourcecode for this.
In fact, I think the GPL suffix is pretty much useless, legal wise.
And everybody knows that the kernel is GPLed. So _every_ single symbol
is actually a _GPL() symbol. the _GPL() annotation is just redundant
and bloats code.

Using kernel syms from a non-GPL compatible module is always walking
a borderline. Be it for _GPL() or normal syms; Exactly the same issue
for both.

All IMO, of course. I'm perfectly aware of different people having
different opinions on this. ;)

-- 
Greetings Michael.