On Thursday 07 February 2008 08:52:35 Johannes Berg wrote: > Looks fine, one last question: > > + if (mactime <= timestamp) { > > + if (CONFIG_MAC80211_IBSS_DEBUG || net_ratelimit()) > > + printk(KERN_DEBUG "%s: beacon TSF higher than " > > + "local TSF - IBSS merge with BSSID %s\n", > > + dev->name, print_mac(mac, mgmt->bssid)); > > I'd rewrite that as timestamp >= mactime and rename the two variables > (maybe beacon_timestamp and phy_timestamp or so?), but that's not too > important. i'll do that, sure. > However, in any case "<=" seems wrong, shouldn't we only > merge when it's actually higher? If your hardware/firmware has synced > properly and the PHY delay is accounted for properly etc. then every > beacon from the own BSS will fulfil the == part of the condition, no? true. i was not aware that the beacon timestamp is taking PHY delays into account and wrongly asumed that the RX timestamp should always be a bit later. (which is also what i saw on my atheros hardware: we usually get the RX timestamp about 60 to 300 usec later than the beacon timestamp, but it's quite likely that we have not calibrated everything 100% correctly.) > Also, the beacon timestamp is defined as follows: > > A STA sending a beacon shall set the value of the beacon’s > timestamp so that it equals the value of the STA’s TSF timer at > the time that the data symbol containing the first bit of the > timestamp is transmitted to the PHY plus the transmitting STA’s > delays through its local PHY from the MAC-PHY interface to its > interface with the WM [e.g., antenna, light-emitting diode (LED) > emission surface]. > > (IEEE 802.11 11.1.2) > > Since we define the RX timestamp to be when the first data symbol of the > frame hits the PHY, we here have to take into account the offset between > the two, at 1 MBit that's 192 (=24 bytes * 8 usecs/byte) usecs earlier > than the beacon timestamp. > > On the other hand, you're unlikely to hit that window, but I suppose it > warrants at least a comment. thats a very good point! if everything was calibrated properly and all delays taken into account *exactly* that would cause us to go thru a merge all the time, on every received beacon. i think we have to take that into account, for different bitrates. what about something like: timestamp >= ( mactime + (24 * 8 / beacon_phy_rate_in_mbps )) i'll resend my patch adding that and the other proposed changes soon. bruno - To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html