Re: [PATCHv2 3/3] mac80211_hwsim: Implement ieee80211_ops:tx_last_beacon

On Fri, Mar 2, 2012 at 10:06 AM, Johannes Berg
[off-list ref] wrote:

On Fri, 2012-03-02 at 09:51 -0800, Javier Cardona wrote:

quoted

On Thu, Mar 1, 2012 at 11:50 PM, Johannes Berg
[off-list ref] wrote:

quoted

On Thu, 2012-03-01 at 15:40 -0800, Javier Cardona wrote:

quoted

Without this you won't see probe responses in an IBSS/hwsim network,
which is convenient for testing.

Signed-off-by: Javier Cardona <redacted>
---
 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.

quoted

quoted

quoted

+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.

quoted

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 :-)

quoted

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