Date: Sat, 17 Sep 2005 21:43:46 +0200 From: "Damien Bergamini" <damien.bergamini@free.fr> To: "Sam Leffler" <sam@errno.com> 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: <00d501c5bbc0$1e8a9240$0300a8c0@COMETE> References: <200509171241.j8HCf5ov019561@repoman.freebsd.org> <432C4C6F.80906@errno.com>
next in thread | previous in thread | raw e-mail | index | archive | help
| 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). No, you missed the point. This is not a table of ieee80211_node's, but just a table of the neighbours mac addresses. Thus there is no problem with reference counting, locking and such. The iwi_find_txnode() function just looks for a mac address in the table and returns its index (an entry is created if it was not already existing). | 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). No. I need to decide on which Tx ring the packet will be sent. And I must decide this before calling ieee80211_encap() because in case the ring is full, I want to put the packet back to the if_snd queue. Unfortunately, the check against EAPOL frames is done in ieee80211_encap(), not in ieee80211_classify(). If I use M_WME_GETAC(m) and the frame is an EAPOL frame, I'll end up checking for free space in the wrong ring. | I suggest that MFC'ing some of these changes so quickly are a bad idea. I don't think so. The WME changes have been in 7-CURRENT for a while now and the new changes are not likely to break anything. Most of the new code will not be triggered if IBSS mode is not active. I'd like this diff to make its way in BETA5 so it can be tested by many. Moreover it fixes the association problems with APs hiding their SSID. Damien
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?00d501c5bbc0$1e8a9240$0300a8c0>