DragonFlyBSD

On 15 Apr 2010, at 13:43, Johannes Hofmann wrote:

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?

Dillon had patches for this at http://apollo.backplane.com/DFlyMisc/wifi03.patch.
I didn't commit them on my branch because it didn't fix the issue that Dillion is having with ath.

• 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.

Regards,
--
Rui Paulo

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.

Cheers,
Alex Hornung

:* 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.

-Matt
                    Matthew Dillon 
                    <dillon@backplane.com>

::* 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.

-Matt
                    Matthew Dillon 
                    <dillon@backplane.com>

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.

The new patch can be found at:
http://leaf.dragonflybsd.org/~hofmann/iwi_update2.diff

Hi Johannes,

If I'm not mistaken, this iwi(4) update has been committed in
0e474d750d8cddd6ef1c9562c0858c338dab536d.

Shall we mark this ticket as resolved?

Cheers,
Antonio Huete

"Antonio Huete Jimenez \(via DragonFly issue tracker\)" <sinknull@leaf.dragonflybsd.org> wrote:

Antonio Huete Jimenez <tuxillo@quantumachine.net> added the comment:

Hi Johannes,

If I'm not mistaken, this iwi(4) update has been committed in
0e474d750d8cddd6ef1c9562c0858c338dab536d.

Shall we mark this ticket as resolved?

Yes, it's committed and works fine for me - posting this via iwi(4) :)

Cheers,
Johannes