-------

>From Randy Dunlap:

3.  +static int __init ieee80211_crypto_init(void)
+    (void) ieee80211_register_crypto_ops(&ieee80211_crypt_null);

That register call can fail, so I'd rather see it checked, as in all
of the other similar register calls.


6.  +struct ieee80211_ccmp_data {

+    u32 dot11RSNAStatsCCMPFormatErrors;
+    u32 dot11RSNAStatsCCMPReplays;
+    u32 dot11RSNAStatsCCMPDecryptErrors;

Are these MIB-mandated names?  otherwise the studlyCaps should go.

7.  +static int ieee80211_ccmp_decrypt()

Does anything care about the various negative/error case return
values?

8.  What calls the .print_stats functions?
static char * ieee80211_ccmp_print_stats()

Is it possible to use seq_file there instead of sprintf(),
or is this in /sysfs (so seq_file is not possible)?
Are there any overflow possibilities?

Enough for tonight...
Here are a few more comments.

1.  No need to check ptr for NULL before kfree(ptr), so several
places like this can be simplified:

+fail:
+    if (priv) {
+        if (priv->tfm)
+            crypto_free_tfm(priv->tfm);
+        kfree(priv);
+    }


3.  +static inline int ieee80211_network_init()
is more than 200 lines.  Somebody likes huge inline functions.

-------------------------
>From Stephen Hemminger


1. You can get rid of the /proc interface altogether since now 
with sysfs module parameters can be made accessible via module_param

2. Include files need to be moved and factored better:
	include/linux/ieee80211.h for anything exposed to utilities and
	   general API related
	include/net/ieee80211.h for interfaces other drivers would use.
	drivers/net/wireless/iee80211/iee80211_xxx.h for stuff that
	   only gets used internally.


4. Don't put data in .h files like eap_types[]
   Maybe just move eap_get_type() to .c fle

5. Use C99 initializers for tags arrays like eap_types[]
 i.e static const char *eap_types[] = {
	[EAP_PACKET] = "EAP-Packet",
...
	
5. Move is_broadcast_addr and is_multicast_addr to etherdevice.h
   besides is_muliticast_addr(x) has extra uneeded test.

6. Don't put format type stuff in .h file MAC_FMT, MAC_ARG, etc...

8. Patch if_ether.h rather than stuff like:
 	#ifndef ETH_P_80211_RAW
 	#define ETH_P_80211_RAW (ETH_P_ECONET + 1)
	#endif

9. Isn't SNAP generic and not part of just wireless, LLC and other places
   like ATM have the similar stuff, maybe one set of routines and definitions?

10. Do we really need another set of statistics?  Already have ethtool, net_stats, wireless, ...?

11. Does ieee->scans need to be atomic?

12.  offset_in_page() shouldn't be here and is defined elsewhere.

The basic philosophy changes as this gets incorporated in mailine kernel.
You need to feel free to patch in several existing header files rather than
standing alone as required in an out of tree driver.

----

>From Jouni Malinen

>When you look through the patch you'll likely notice the #ifdef 
>> NOTYET/#endif sequences surrounding portions of code from the hostap 
>> project.  Portions of this subsystem were based on an earlier version of 
>> the hostap project.  Those areas that weren't directly supported by the 
>> ipw* projects weren't ported to be completely hardware independent 
>> (since I don't have the hardware to test it), and so are still wrapped 
>> in the ifdefs.  These sections mainly cover support for MASTER and WDS 
>> modes.


We will need to take care of these areas in order to be able to call
this "generic IEEE 802.11 networking stack".. I would like to be able to
test replacing the upper level code in Host AP driver with this and
verify that all functionality is still available. Another useful test
case would be to go through what needs to be done to make this work with
the madwifi driver (i.e., merge some parts of net80211 code with this
and make the low-level parts of the driver use the combination for IEEE
802.11 processing).
>+static void * ieee80211_ccmp_init(int key_idx)
>> +{
>> +	struct ieee80211_ccmp_data *priv;
>> +
>> +	priv = kmalloc(sizeof(*priv), GFP_ATOMIC);


You do not seem to include try_module_get(THIS_MODULE) here (and
matching module_put in deinit) like the version in Host AP driver is
using. What is protecting the CCMP/TKIP modules from being unloaded when
they are still used?