* Locking is done completely with lockmgr locks as it is done for
ath now. What is the plan about if_serializer?
I noticed that e.g. parent_updown() in ieee80211_proto.c calls
if_ioctl without if_serializer held. Does this mean that
if_serializer use is deprecated?

* It still uses our old firmware API via wrapper functions, as I
didn't know how to create the firmware modules needed with the new API.
Therefore the patch brings kern_firmware.c back into the kernel
build, but all this can easily be switched to the new API.

* sysctl's are not removed on module unload, so when unloading/loading
I get warnings about reusing sysctl leafs. I didn't find the
relevent code in the other drivers, so maybe I'm missing something
here.

* The alloc_unr()/free_unr() stuff is just commented out. Are there
any plans to bring in this API from FreeBSD?

History

> Hi,
>
> http://leaf.dragonflybsd.org/~hofmann/iwi_update.diff> is a patch to bring in the newest iwi(4) from FreeBSD.
> It basically works and I can associate via wpa2.
> There are some issues though that need to be resolved:
>
> * Locking is done completely with lockmgr locks as it is done for
> ath now. What is the plan about if_serializer?
> I noticed that e.g. parent_updown() in ieee80211_proto.c calls
> if_ioctl without if_serializer held. Does this mean that
> if_serializer use is deprecated?

> * It still uses our old firmware API via wrapper functions, as I
> didn't know how to create the firmware modules needed with the new API.
> Therefore the patch brings kern_firmware.c back into the kernel
> build, but all this can easily be switched to the new API.

See sys/tools/fw_stub.awk in FreeBSD.

> * sysctl's are not removed on module unload, so when unloading/loading
> I get warnings about reusing sysctl leafs. I didn't find the
> relevent code in the other drivers, so maybe I'm missing something
> here.

ath needs this too, IIRC, so you're not missing anything.

>
> * The alloc_unr()/free_unr() stuff is just commented out. Are there
> any plans to bring in this API from FreeBSD?

This facility seems to be pretty straightforward to port. I'll let someone else comment on the desirability.

fwiw, for such a basic use of alloc_unr, devfs_clone_bitmap functions will work
fine (see man devfs_clone_bitmap_get). Just note that they always start the
numbering at 0, so you might need to set a few as allocated if you don't want
them. An upper limit is provided.

:* Locking is done completely with lockmgr locks as it is done for
: ath now. What is the plan about if_serializer?
: I noticed that e.g. parent_updown() in ieee80211_proto.c calls
: if_ioctl without if_serializer held. Does this mean that
: if_serializer use is deprecated?

The serializer is already held at that point. The serializer is
not optional, it must be held properly when messing with the ifnet
(such as when dequeueing packets). I still have some work to do
on the ath driver but as Rui mentioned I'm having problems stabilizing
it on SMP.

The kernel will call into the driver with the serializer held for
entry points via ifnet, but things such as callouts and the device
function call API need to acquire the serializer.

The 80211 (wlan) code does acquire the serializer when making most
cross-ifnet calls which simplifies the other drivers but it's still
a real mess.

:* It still uses our old firmware API via wrapper functions, as I
: didn't know how to create the firmware modules needed with the new API.
: Therefore the patch brings kern_firmware.c back into the kernel
: build, but all this can easily be switched to the new API.
:
:* sysctl's are not removed on module unload, so when unloading/loading
: I get warnings about reusing sysctl leafs. I didn't find the
: relevent code in the other drivers, so maybe I'm missing something
: here.

Any SYSCTL declarations should be automatically added and removed.
Any manually added sysctl subtrees (made with procedure calls within
the driver) have to be explicitly removed by the driver.

An example can be found in /usr/src/sys/dev/coretemp/coretemp.c,
coretemp_attach() and coretemp_detach().

:* The alloc_unr()/free_unr() stuff is just commented out. Are there
: any plans to bring in this API from FreeBSD?
:

That doesn't look optional. Sigh. I suppose you could port subr_unit.c
but as is typical in FreeBSDland they have completely overengineered
the API. It would be a whole lot easier if they had just used the
subr_blist.c code for that.

::* The alloc_unr()/free_unr() stuff is just commented out. Are there
:: any plans to bring in this API from FreeBSD?
::
:
: That doesn't look optional. Sigh. I suppose you could port subr_unit.c
: but as is typical in FreeBSDland they have completely overengineered
: the API. It would be a whole lot easier if they had just used the
: subr_blist.c code for that.

Rui also mentioned that AlexH thought the clone_bitmap code would
work just fine too.

Matthew Dillon <dillon@apollo.backplane.com> wrote:
> ::* The alloc_unr()/free_unr() stuff is just commented out. Are there
> :: any plans to bring in this API from FreeBSD?
> ::
> :
> : That doesn't look optional. Sigh. I suppose you could port subr_unit.c
> : but as is typical in FreeBSDland they have completely overengineered
> : the API. It would be a whole lot easier if they had just used the
> : subr_blist.c code for that.
>
> Rui also mentioned that AlexH thought the clone_bitmap code would
> work just fine too.

Thanks for the feedback!
I've cleaned up the patch so that

* sysctl nodes are removed on detach.

* The new firmware infrastructure from FreeBSD is used.
This also needs a small bugfix in subr_firmware.c to avoid a panic
(see my mail on @bugs).
Note, that you now need to build and load a firmware kernel module
for iwi to work.
I'm uncertain how this should be automated. I think there might be
licensing issues if we would ship the Intel firmware.

* devfs_clone_bitmap_*() functions are used for unique number
allocation.