Date: Sat, 17 Sep 2005 10:03:43 -0700 From: Sam Leffler <sam@errno.com> To: Damien Bergamini <damien@FreeBSD.org> Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/iwi if_iwi.c if_iwireg.h if_iwivar.h Message-ID: <432C4C6F.80906@errno.com> In-Reply-To: <200509171241.j8HCf5ov019561@repoman.freebsd.org> References: <200509171241.j8HCf5ov019561@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?432C4C6F.80906>