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

[Sam Leffler]

Damien Bergamini wrote:
> | > 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).
> | 
> | Sorry, you're correct, these are mac addresses and not node references. 
> |  But the suggestion still holds.  You've got a separate piece of 
> | per-node  information that logically belongs in a driver-private area of 
> | the node.  Having it there would eliminate the table lookup; you just 
> | take the node pointer and deref to get the index.  The intent of 
> | driver-private node storage is to eliminate add-on tables like this.
> 
> Yeah, I already used something similar in ral for per-node rate adaptation.
> But I thought it would be an overkill here for such a simple task.
> Moreover, I must maintain exactly the same table in h/w, so keeping the h/w
> table in sync with net80211 nodes would be a nightmare.

Not sure about the nightmare of syncing state or things being overkill. 
  Doing per-driver state is very simple and if you need one piece of 
data you''re likely going to need more.

The main advantage I can see to doing what I suggested is you eliminate 
a fixed-size table in your driver.  I don't think you do lookups often 
so replacing the linear search with the O(1) deref probably doesn't matter.

> 
> | Note that when I MFC'd changes in this driver recently that I did not 
> | MFC any of your WME mods.  My suggestion was that you not MFC _some_ of 
> | the changes; not things like fixing hidden ap handling.  re is the final 
> | arbiter of what can be MFC'd.
> 
> Looks like it's already too late for BETA5 anyway ...

How are you testing your WME support?

	Sam