Re: [Madwifi-devel] struct ieee80211_node * ni

[Benoit P. <ben...@fr...>]

Benoit PAPILLAULT a écrit :
> Hello there,
>
> I am currently working on IBSS merge and it appears that it is not 
> properly working due to the usage of struct ieee80211_node. I'd like to 
> sum up what I learned reading the madwifi source code and would like 
> suggestions for improving the current code.
>
> Whenever a packet is received, ath_rx_tasklet() is called. At this 
> point, there are 3 choices:
>
> - rs_keyidx is used to locate "ni" (struct ieee80211_node *) and 
> ieee80211_input is called
>
> - ieee80211_find_rxnode is used with either  i_addr1 or i_addr2 to 
> locate an EXISTING node and ieee80211_input is called
>
> - if both failed, ieee80211_input_all is called and then ieee80211_input 
> is called for every vap with  ni = vap->iv_bss
>
> Now, let's say  we have two boxes in IBSS mode and the first one is 
> started first (and as such is the IBSS master with a bigger TSF). One 
> the second box, the following should happen:
>
> - A beacon is received from the first box with the same SSID and IBSS 
> flag is set and the TSF is higher, then the IBSSID should be used and HW 
> TSF synchronized.
>
> In fact, the following happen:
>
> - A beacon is received and since it's a new "node", ieee80211_input is 
> called with ni = vap->iv_bss. In ieee80211_recv_mgmt, a new 
> "ieee80211_node" is created but NOT added to the global table. This new 
> node contains the TSF from the new beacon. Whenever another beacon is 
> received, it will not be located in the "node" table as it should. If it 
> were located, then the test done in ath_recv_mgmt will be done on this 
> node and ieee80211_ibss_merge() is called. A third beacon seems to be 
> needed to update the HW TSF as well.
>
> I think a first improvement would be to require only ONE beacon (instead 
> of THREE) to properly handle IBSS merge. Moreover, I have the following 
> questions:
>
> /proc/net/madwifi/ath0/associated_sta is used to display this node table 
> per vap. However, it is not clear if :
> a. all packets received should have an entry recorded here (whatever the 
> frequency is). This could be usefull to know the signal strengh of 
> neighboring SSID for instances.
> b. all packets belonging to the current BSS or IBSS should be recorded
> c. all packets matching the current BSSID (SSID and BSSID are not used 
> the same way in BSS and IBSS).
> d. only really associated sta (ie, after auth?)
>
> I would prefer a/ since it's the less restricted (or b). However, i'm 
> not sure all fields of the ieee80211_node can be filled in....
>
> Next, I think it's quite ridiculious and prone to bugs to REQUIRE the 
> use ieee80211_node for different purposes. And i don't like the way to 
> pass parameters between ieee80211_recv_mgmt and ath_recv_mgmt through 
> those structures. It makes the code hard to understand & debug. My 
> suggestions here would to fully handle IBSS merge in ieee80211_recv_mgmt 
> and just set sc->syncbeacon (for instance).
>
> Comments?
> Benoit
>   
I have tried IBSS merge test scenarios, as described on 
http://madwifi.org/wiki/DevDocs/AdhocTestScenario to test my patch and 
test 5 is failing.

A more thorought debug show that, for a very mysterious and unknown 
reason, the first beacon received by A from B is the only beacon where 
TSF(A) < TSF(B) even if TSF(A) is nearly synchronized (the hardware has 
probably started the synchronisation procedure since the IBSSID is 
correct from start). Here is my log of the first three beacon received 
by A from B:

Jul 14 22:12:38 192.168.1.2 kernel: ath_beacon_config: ni=c6cc4000 tsf=0 
hw_tsf=10143459047 tsftu=0 hw_tsftu=9905721
Jul 14 22:12:38 192.168.1.2 kernel: ath_beacon_config: first beacon
Jul 14 22:12:38 192.168.1.2 kernel: ath_beacon_config: nexttbtt=100 
intval=100
Jul 14 22:12:38 192.168.1.2 kernel: wifi0: ath_recv_mgmt ni=c6cc4000 
subtype=0x80
Jul 14 22:12:38 192.168.1.2 kernel: ni=c6cc4000 vap=c58c8280 
i_addr2=06:0b:6b:37:1f:d2
Jul 14 22:12:38 192.168.1.2 kernel: ieee80211_add_neighbor: ni=c5f77000 
ni_tstamp=10143539468
Jul 14 22:12:38 192.168.1.2 kernel: check for ibss merge for ni=c6cc4000 
TSF1(t4)=10143513234 TSF2(t3)=         1
=> beacon 1, TSF(A) = 10143513234, TSF(B) = 10143539468, merge condition 
met!

Jul 14 22:12:38 192.168.1.2 kernel: wifi0: ath_recv_mgmt ni=c5f77000 
subtype=0x80
Jul 14 22:12:38 192.168.1.2 kernel: ni=c5f77000 vap=c58c8280 
i_addr2=06:0b:6b:37:1f:d2
Jul 14 22:12:38 192.168.1.2 kernel: ieee80211_recv_mgmt: ni=c5f77000 
ni_tstamp=10143641729
Jul 14 22:12:38 192.168.1.2 kernel: check for ibss merge for ni=c5f77000 
TSF1(t4)=10143641851 TSF2(t3)=10143641729
=> beacon 2, TSF(A) = 10143641851, TSF(B) = 10143641729, merge condition 
not met!

Jul 14 22:12:38 192.168.1.2 kernel: wifi0: ath_recv_mgmt ni=c5f77000 
subtype=0x80
Jul 14 22:12:38 192.168.1.2 kernel: ni=c5f77000 vap=c58c8280 
i_addr2=06:0b:6b:37:1f:d2
Jul 14 22:12:38 192.168.1.2 kernel: ieee80211_recv_mgmt: ni=c5f77000 
ni_tstamp=10143744242
Jul 14 22:12:38 192.168.1.2 kernel: check for ibss merge for ni=c5f77000 
TSF1(t4)=10143744365 TSF2(t3)=10143744242
=> beacon 3, TSF(A) = 10143744365, TSF(B) = 10143744242, merge condition 
not met!

That's one reason to handle IBSS merge on the very first beacon. 
Remaining question is why the TSF(A) < TSF(B) is valid on the first beacon!

Best regards,
Benoit