On Fri, Mar 2, 2012 at 10:06 AM, Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote: > On Fri, 2012-03-02 at 09:51 -0800, Javier Cardona wrote: >> On Thu, Mar 1, 2012 at 11:50 PM, Johannes Berg >> <johannes@xxxxxxxxxxxxxxxx> wrote: >> > On Thu, 2012-03-01 at 15:40 -0800, Javier Cardona wrote: >> >> Without this you won't see probe responses in an IBSS/hwsim network, >> >> which is convenient for testing. >> >> >> >> Signed-off-by: Javier Cardona <javier@xxxxxxxxxxx> >> >> --- >> >> drivers/net/wireless/mac80211_hwsim.c | 9 +++++++++ >> >> 1 files changed, 9 insertions(+), 0 deletions(-) >> >> >> >> diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c >> >> index deed95f..e4b6ca6 100644 >> >> --- a/drivers/net/wireless/mac80211_hwsim.c >> >> +++ b/drivers/net/wireless/mac80211_hwsim.c >> >> @@ -47,6 +47,8 @@ static bool fake_hw_scan; >> >> module_param(fake_hw_scan, bool, 0444); >> >> MODULE_PARM_DESC(fake_hw_scan, "Install fake (no-op) hw-scan handler"); >> >> >> >> +static struct ieee80211_hw *txed_last_beacon; >> > >> > That's definitely not right. >> >> I am using that module global to save the last hw that transmitted a >> beacon. The pointer is only tested for equality, never dereferenced. >> And even if a read access happens during a non-atomic write, there are >> no adverse consquences other than mac80211_hwsim_tx_last_beacon() >> returning the wrong value. Would you rather see it protected with a >> mutex? > > No, no, it's just the logic. > >> >> +static int mac80211_hwsim_tx_last_beacon(struct ieee80211_hw *hw) >> >> +{ >> >> + return (hw == txed_last_beacon); >> >> +} >> > >> > And this is mostly a fancy way of saying "return 1" -- there's no beacon >> > backoff protocol in here, so why not just hardcode "return 1"? >> >> This only returns true if 'hw' is the last hw that transmitted a >> beacon. Worked for me, as I was seeing 0's and 1's. > > Yeah, but it's nowhere near how IBSS works. You're doing this across > different channels, no matter whether it's even in IBSS mode, and > there's no backoff (all of them will TX anyway) I thought that function was called only in IBSS mode, but still, you are right. There are many things i did not consider. >> My (maybe flawed) thinking was: I need this function to return true to >> test my previous patch. Hardcoding a 1 would do it me, but someone >> might complain that makes no sense. So I tried to simulate a behavior >> that's closer to reality. In my IBSS/hwsim network I now see one >> probe response at a time even though there are multiple phy's. > > It's so far from reality that I don't think we should do it :-) > >> What would you want me to do with this patch? I'm not particulary >> attached to it... > > I don't really care -- do you want to return 1 or just leave this patch > out? Hmmm, I think I'll leave it out. If anyone is using hwsim to test IBSS they'll probably know how to code this better. Thanks for your help! Javier -- 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