cvs commit: src/sys/dev/iwi if_iwi.c if_iwireg.h if_iwivar.h

Sam Leffler sam at errno.com
Sat Sep 17 09:56:40 PDT 2005


Damien Bergamini wrote:
> damien      2005-09-17 12:41:05 UTC
> 
>   FreeBSD src repository
> 
>   Modified files:
>     sys/dev/iwi          if_iwi.c if_iwireg.h if_iwivar.h 
>   Log:
>   o Add initial bits for IBSS support.

You appear to have added a private sta table so that you could have an 
index for each node table entry.  However I don't see any place where 
you index into the table; only that you need the index to pass to the 
firmware.  This seems to say you only need a per-node index and that can 
be done differently (see below).

FWIW, the intended way to maange per-node driver-private data like this 
is for the driver to extend the node data structure with a private 
definition (override the class decl in c++ parlance) and store the index 
in the private data area.  This could eliminate the table and avoid the 
problems you've introduced by holding uncounted references to node data 
structures.  It also avoids likely race conditions that'll result from 
having an unlocked data structure that overlaps scope with the net80211 
node table (I did something similar in the ath driver only to find out 
the hard way it was a mistake).

>   o Allow association with APs that do not broadcast SSID (with hints from
>     Nick Hudson and Hajimu Umemoto).
>   o IFQ_DRV_PREPEND mbuf when h/w ring is full so it can be sent later.
>   o Increment if_oerrors when appropriate.
>   o Did some cleanup while I'm here.
>   
>   MFC after:      1 day

This commit msg didn't indicate that you also made changes for WME 
support.  You appear to disallow QoS encapsulation of EAPOL frames but 
this is already done in ieee80211_encap.  ieee80211_classify should 
assign an AC for all frames so use M_WME_GETAC(m) and don't make 
driver-local decisions unless they are required by the h/w (hard to tell 
from just looking at a diff).

I suggest that MFC'ing some of these changes so quickly are a bad idea.

	Sam


More information about the cvs-all mailing list