Intel would like to announce the initial release of a Linux WiMAX

common stack and drivers for its WiMAX devices (eg: Intel(r)

WiMAX/WiFi Link 5050, with estimated availability around third

quarter of 2008).
The project follows a standard layered approach: a generic WiMAX

kernel stack provides a common interface to control WiMAX devices.

A user space daemon and libraries provide a high level API for

control (network scanning, connection, disconnection, roaming

management, etc) and access to the drivers. 
Currently much of this higher level code is tailored to Intel's

device, but we wanted to get it out as soon as possible.
You can find the code and additional information at

http://linuxwimax.org.
--

Looking at the kernel/user interface: wimax-tools-1.1/lib/op-open.c
This API is repeating the mistake of the old Linux Wireless API. It is putting version

checks between kernel and library and this is a maintenance nightmare. Linux

API's are not COM. Versioning is a mistake.  Use a TLA api like netlink so it

can be extensible without version handshake.

--

This is the short (not in depth) code review of kernel component of Wimax.
Generic stack: drivers/net/wimax
1. Why spread those over 8 files of 100 lines each. Better to have a single

   file with 1000 lines and get reduced namespace and better compiler inlining

   etc.
2. The debug infrastructure is a mess of ugly macros that are unlikely

   to accepted in the current form, rework or delete it.
3. Use of sysfs for family and version ok, but why bother?

   Please don't build sysfs version checks into the API.
4. __wimax_flush_queue: is a nop, just remove
5. Stack is using generic netlink instead use newer netlink interface

   for management of devices: newlink/dellink instead see macvlan
i2400m hardware driver
1. sysfs

  the inflight file is being used in a /proc style. Either change to

  one value per file or move to /proc/net/i2400m/ethX or better yet use debugfs

  /debugfs/i2400m/ethX/xxx
2. Use internal dev->stats for network stats (may not need get_stats at all)
3. No ioctl stub if not implemented
4. Use netdev_alloc_skb rather than alloc_skb for new code.
5. Use skb_copy_to_linear_data in i2400m_net_rx
6. Since hardware has to copy every received frame. Don't bother with

   checksum offload, the copy does the checksum for you and is safer.
7. Fine grain file organization in i2400m is bogus, put in one file and use

   proper name scope. Having 100 line files is unneeded
8. Fix the FIXME's?
9. Anyway to reuse existing usbnet infrastructure?
10. Since device is USB, Rx is in workqueue, so no need to go through

    netif_rx() softirq, should be able to go through netif_receive_skb
11. net_device and private data are zero on allocation, so no need

   for memset.
12. Since this is new code elminate all legacy ifdef's
--

I might have missed something in the latest code, but how it is using

generic netlink for device management. The current code is using

generic netlink for communication with the user space component. This

includes task like scanning and association. The current kernel/user

space API might need improvement, but I am not getting your point  
I don't think so. WiMAX is not emulating an Ethernet device. It is an  
Don't worry. We will do this before submitting it for mainline

inclusion. It is a temporary solutions to help people that wanna test

it with older kernels.
Regards
Marcel
--

Style difference, simplifies maintenance. Each file is logically grouped.

Everything for doing X is in file X. I know you are a fan of big files,

I am not :)
There is almost no inlining because each X only does stuff from itself. If

there is a need for inlined stuff (I missed it) it should go to one of the
Ok, I'll figure a way to clean it up. Need to move most to inlines,
family ID: it makes it easy to map device<->family-id without

expensive look ups the kernel like an attribute with the device

name would be. Rationale is most systems will have a single wimax

device in the kernel. include/linux/wimax.h and drivers/net/wimax/id-table.c

should have this documented...ouch, just remembered. The code drop

out there shouldn't be as complete (documentation wise) as what

I need to publish. Pls hold for it, will be available.
version: I anticipate the wimax API exported to user space is

going to undergo a lot of changes while we all agree on what

is the best interface. Because things might break, I want to

make sure user space stuff can detect that and fail cleanly.

Hence the versioning. 
It is designed to be flexible so that adding new APIs allows

old code to work (however, changing existing APIs is where it

breaks). From the docs:
 * Each WiMAX device exports two sysfs files declaring the generic

 * netlink family ID associated to the interface and another one which

 * version it supports. The version code has to fit in one byte

 * (restrictions imposed by generic netlink); we use version / 10 for

 * the major version and version % 10 for the minor. This gives 9

 * minors for each major and 25 majors.

 *

 * The inexistence of any of this means the device does not support

 * the WiMAX extensions.

 *

 * The version change protocol is as follow:

 *

 * Your user space code should not try to work if the major version it

 * was compiled for differs from what the kernel offers. As well, it

 * should not work if the minor version of the kernel interface is

 * lower...
[ message continues ]

Inlining gets more and more discouraged. Also longer term I would

expect that gcc will do inlining over files anyways (it already supports

that, but needs special support in the Makefiles which the kernel

doesn't have). So even with inlining you wouldn't need to merge files
It's still a bad way to do that (I agree with Stephen on that).

Was also always a mess on wireless.
If you don't want expandable TLAs another better alternative to

versions is ext2 style compatible/incompatible feature bitmaps.
-Andi
--

Ain't that another way of saying versions? Sorry guys, but I am

having a hard time understanding how the alternatives are better.
Expandable TLAs are fine, until we need to change the meaning of

an existing field/value. Then there is no way to change it without

breaking existing code other than create a new message. And then

we have a mess like wireless again. 
With simple versioning, we just bump up the major number and break.

In user space, the libwimax shim library would take care of  hiding

this for the user if so is needed.
--

Inaky
--

It's a related way, but an actually sane way.
With compat/incompat bitmaps user space can actually make a educated

decision if it should fail or not
-Andi
--

I notice that there is a binary supplicant required. After the experience with the

ipw3945 driver lack of acceptance, I would hope that Intel would have learned

that dependence on binary userspace components would limit acceptance especially

with other more hostile projects (OpenBSD).

--

On Tue, 1 Apr 2008 11:07:37 -0700
Code review occurs on patch submittable not by pointing developers off

to an external project. Most likely there will be feedback that requires

changes to API and other choices so you want to do this soon.

--

I know. Just wanted to get out the bits ASAP for whoever is interested to

start looking at it.
I am pondering how to do this because there is a lot of code and

I still don't have it ready for merging to mainline. 
Eventually I'll have a git tree branching off netdev and will have to break

up the kernel part in patchbombs for review & submission and fix stuff as

we go.
Thanks,

--