[PATCH v2 1/3] mac80211_hwsim: use hrtimer for beacon timers

[PATCH v2 1/3] mac80211_hwsim: use hrtimer for beacon timers

[Thomas Pedersen]

For testing various timing-sensitive protocols (power
save, etc.), a beacon accuracy of jiffies is not
sufficient.

Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
---

v2:
	Use tasklet_kill() on interface stop.

 drivers/net/wireless/mac80211_hwsim.c |   63 ++++++++++++++++++++-------------
 1 file changed, 39 insertions(+), 24 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index ff90855..84dbfe8 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -333,11 +333,11 @@ struct mac80211_hwsim_data {
 	int scan_chan_idx;
 
 	struct ieee80211_channel *channel;
-	unsigned long beacon_int; /* in jiffies unit */
+	ktime_t beacon_int;
 	unsigned int rx_filter;
 	bool started, idle, scanning;
 	struct mutex mutex;
-	struct timer_list beacon_timer;
+	struct hrtimer beacon_timer;
 	enum ps_mode {
 		PS_DISABLED, PS_ENABLED, PS_AUTO_POLL, PS_MANUAL_POLL
 	} ps;
@@ -358,6 +358,7 @@ struct mac80211_hwsim_data {
 
 	/* difference between this hw's clock and the real clock, in usecs */
 	u64 tsf_offset;
+	struct tasklet_struct bcn_tasklet;
 };
 
 
@@ -896,7 +897,8 @@ static void mac80211_hwsim_stop(struct ieee80211_hw *hw)
 {
 	struct mac80211_hwsim_data *data = hw->priv;
 	data->started = false;
-	del_timer(&data->beacon_timer);
+	hrtimer_cancel(&data->beacon_timer);
+	tasklet_kill(&data->bcn_tasklet);
 	wiphy_debug(hw->wiphy, "%s\n", __func__);
 }
 
@@ -980,21 +982,30 @@ static void mac80211_hwsim_beacon_tx(void *arg, u8 *mac,
 				rcu_dereference(vif->chanctx_conf)->def.chan);
 }
 
-
-static void mac80211_hwsim_beacon(unsigned long arg)
+static void mac80211_hwsim_bcn_tasklet(unsigned long d)
 {
-	struct ieee80211_hw *hw = (struct ieee80211_hw *) arg;
-	struct mac80211_hwsim_data *data = hw->priv;
-
-	if (!data->started)
-		return;
+	struct mac80211_hwsim_data *data = (struct mac80211_hwsim_data *) d;
+	struct ieee80211_hw *hw = data->hw;
 
 	ieee80211_iterate_active_interfaces_atomic(
 		hw, IEEE80211_IFACE_ITER_NORMAL,
 		mac80211_hwsim_beacon_tx, hw);
+}
 
-	data->beacon_timer.expires = jiffies + data->beacon_int;
-	add_timer(&data->beacon_timer);
+static enum hrtimer_restart
+mac80211_hwsim_beacon(struct hrtimer *timer)
+{
+	struct mac80211_hwsim_data *data =
+		container_of(timer, struct mac80211_hwsim_data, beacon_timer);
+
+	if (!data->started)
+		return HRTIMER_NORESTART;
+
+	/* must defer here since hrtimers are run in hard-IRQ */
+	tasklet_schedule(&data->bcn_tasklet);
+
+	hrtimer_forward(timer, hrtimer_get_expires(timer), data->beacon_int);
+	return HRTIMER_RESTART;
 }
 
 static const char *hwsim_chantypes[] = {
@@ -1031,10 +1042,11 @@ static int mac80211_hwsim_config(struct ieee80211_hw *hw, u32 changed)
 	WARN_ON(data->channel && channels > 1);
 
 	data->power_level = conf->power_level;
-	if (!data->started || !data->beacon_int)
-		del_timer(&data->beacon_timer);
-	else
-		mod_timer(&data->beacon_timer, jiffies + data->beacon_int);
+	if (!data->started || !ktime_to_ns(data->beacon_int))
+		hrtimer_cancel(&data->beacon_timer);
+	else if (!hrtimer_is_queued(&data->beacon_timer))
+		hrtimer_start(&data->beacon_timer, data->beacon_int,
+			      HRTIMER_MODE_REL);
 
 	return 0;
 }
@@ -1084,12 +1096,12 @@ static void mac80211_hwsim_bss_info_changed(struct ieee80211_hw *hw,
 
 	if (changed & BSS_CHANGED_BEACON_INT) {
 		wiphy_debug(hw->wiphy, "  BCNINT: %d\n", info->beacon_int);
-		data->beacon_int = 1024 * info->beacon_int / 1000 * HZ / 1000;
-		if (WARN_ON(!data->beacon_int))
-			data->beacon_int = 1;
-		if (data->started)
-			mod_timer(&data->beacon_timer,
-				  jiffies + data->beacon_int);
+		data->beacon_int = ns_to_ktime(info->beacon_int * 1024 * 1000);
+		if (WARN_ON(!ktime_to_ns(data->beacon_int)))
+			data->beacon_int = ns_to_ktime(1000 * 1000);
+		if (data->started && !hrtimer_is_queued(&data->beacon_timer))
+			hrtimer_start(&data->beacon_timer,
+				      data->beacon_int, HRTIMER_MODE_REL);
 	}
 
 	if (changed & BSS_CHANGED_ERP_CTS_PROT) {
@@ -2370,8 +2382,11 @@ static int __init init_mac80211_hwsim(void)
 							data->debugfs, data,
 							&hwsim_fops_group);
 
-		setup_timer(&data->beacon_timer, mac80211_hwsim_beacon,
-			    (unsigned long) hw);
+		tasklet_init(&data->bcn_tasklet,
+			     mac80211_hwsim_bcn_tasklet, (unsigned long) data);
+		hrtimer_init(&data->beacon_timer,
+			     CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+		data->beacon_timer.function = mac80211_hwsim_beacon;
 
 		list_add_tail(&data->list, &hwsim_radios);
 	}
-- 
1.7.10.4

[PATCH v2 2/3] mac80211_hwsim: beacon at beacon interval

[Thomas Pedersen]

A beacon period starts at TSF time 0. Spoof this by
rounding the starting beacon time to a multiple of the
beacon interval.

Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
---
 drivers/net/wireless/mac80211_hwsim.c |   21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 84dbfe8..0b38600 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -1044,9 +1044,14 @@ static int mac80211_hwsim_config(struct ieee80211_hw *hw, u32 changed)
 	data->power_level = conf->power_level;
 	if (!data->started || !ktime_to_ns(data->beacon_int))
 		hrtimer_cancel(&data->beacon_timer);
-	else if (!hrtimer_is_queued(&data->beacon_timer))
-		hrtimer_start(&data->beacon_timer, data->beacon_int,
-			      HRTIMER_MODE_REL);
+	else if (!hrtimer_is_queued(&data->beacon_timer)) {
+		u64 tsf = le64_to_cpu(__mac80211_hwsim_get_tsf(data));
+		u64 bcn_int = ktime_to_ns(data->beacon_int) / 1000;
+		u64 next_tbtt = bcn_int - do_div(tsf, bcn_int);
+
+		hrtimer_start(&data->beacon_timer,
+			      ns_to_ktime(next_tbtt * 1000), HRTIMER_MODE_REL);
+	}
 
 	return 0;
 }
@@ -1099,9 +1104,15 @@ static void mac80211_hwsim_bss_info_changed(struct ieee80211_hw *hw,
 		data->beacon_int = ns_to_ktime(info->beacon_int * 1024 * 1000);
 		if (WARN_ON(!ktime_to_ns(data->beacon_int)))
 			data->beacon_int = ns_to_ktime(1000 * 1000);
-		if (data->started && !hrtimer_is_queued(&data->beacon_timer))
+		if (data->started && !hrtimer_is_queued(&data->beacon_timer)) {
+			u64 tsf = le64_to_cpu(__mac80211_hwsim_get_tsf(data));
+			u64 bcn_int = ktime_to_ns(data->beacon_int) / 1000;
+			u64 next_tbtt = bcn_int - do_div(tsf, bcn_int);
+
 			hrtimer_start(&data->beacon_timer,
-				      data->beacon_int, HRTIMER_MODE_REL);
+				      ns_to_ktime(next_tbtt * 1000),
+				      HRTIMER_MODE_REL);
+		}
 	}
 
 	if (changed & BSS_CHANGED_ERP_CTS_PROT) {
-- 
1.7.10.4

[PATCH v2 3/3] mac80211_hwsim: emulate proper beaconing

[Thomas Pedersen]

What this means:

1) Fill in beacon timestamp for the monitor interface, and
   don't make the timestamp adjustment so contrived.
2) Change beacon time in response to TSF adjustment. This
   means hwsim PHYs can now be told to beacon offset
   (Toffset) from each other in time by adjusting the TSF.
3) PHY TSF offset adjustments are cumulative. i.e. +1000,
   then -1000 should not result in a TSF with offset -1000.

Per-station Toffset tracking has been tested, and beacons
are transmitted shortly after TBTT as shown in the
timestamp.

Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
---
 drivers/net/wireless/mac80211_hwsim.c |   50 ++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 0b38600..94257f9 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -357,7 +357,8 @@ struct mac80211_hwsim_data {
 	int power_level;
 
 	/* difference between this hw's clock and the real clock, in usecs */
-	u64 tsf_offset;
+	s64 tsf_offset;
+	s64 bcn_delta;
 	struct tasklet_struct bcn_tasklet;
 };
 
@@ -408,8 +409,7 @@ static netdev_tx_t hwsim_mon_xmit(struct sk_buff *skb,
 
 static __le64 __mac80211_hwsim_get_tsf(struct mac80211_hwsim_data *data)
 {
-	struct timeval tv = ktime_to_timeval(ktime_get_real());
-	u64 now = tv.tv_sec * USEC_PER_SEC + tv.tv_usec;
+	u64 now = ktime_to_ns(hrtimer_cb_get_time(&data->beacon_timer)) / 1000;
 	return cpu_to_le64(now + data->tsf_offset);
 }
 
@@ -424,9 +424,12 @@ static void mac80211_hwsim_set_tsf(struct ieee80211_hw *hw,
 		struct ieee80211_vif *vif, u64 tsf)
 {
 	struct mac80211_hwsim_data *data = hw->priv;
-	struct timeval tv = ktime_to_timeval(ktime_get_real());
-	u64 now = tv.tv_sec * USEC_PER_SEC + tv.tv_usec;
-	data->tsf_offset = tsf - now;
+	u64 now = __mac80211_hwsim_get_tsf(data);
+	s32 delta = tsf - now;
+
+	data->tsf_offset += delta;
+	/* adjust after beaconing with new timestamp at old TBTT */
+	data->bcn_delta = delta;
 }
 
 static void mac80211_hwsim_monitor_rx(struct ieee80211_hw *hw,
@@ -697,7 +700,6 @@ static bool mac80211_hwsim_tx_frame_no_nl(struct ieee80211_hw *hw,
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	struct ieee80211_rx_status rx_status;
-	struct ieee80211_rate *txrate = ieee80211_get_tx_rate(hw, info);
 
 	memset(&rx_status, 0, sizeof(rx_status));
 	rx_status.flag |= RX_FLAG_MACTIME_START;
@@ -727,7 +729,6 @@ static bool mac80211_hwsim_tx_frame_no_nl(struct ieee80211_hw *hw,
 	spin_lock(&hwsim_radio_lock);
 	list_for_each_entry(data2, &hwsim_radios, list) {
 		struct sk_buff *nskb;
-		struct ieee80211_mgmt *mgmt;
 		struct tx_iter_data tx_iter_data = {
 			.receive = false,
 			.channel = chan,
@@ -763,17 +764,8 @@ static bool mac80211_hwsim_tx_frame_no_nl(struct ieee80211_hw *hw,
 		if (mac80211_hwsim_addr_match(data2, hdr->addr1))
 			ack = true;
 
-		/* set bcn timestamp relative to receiver mactime */
 		rx_status.mactime =
 				le64_to_cpu(__mac80211_hwsim_get_tsf(data2));
-		mgmt = (struct ieee80211_mgmt *) nskb->data;
-		if (ieee80211_is_beacon(mgmt->frame_control) ||
-		    ieee80211_is_probe_resp(mgmt->frame_control))
-			mgmt->u.beacon.timestamp = cpu_to_le64(
-				rx_status.mactime +
-				(data->tsf_offset - data2->tsf_offset) +
-				24 * 8 * 10 / txrate->bitrate);
-
 #if 0
 		/*
 		 * Don't enable this code by default as the OUI 00:00:00
@@ -964,7 +956,11 @@ static void mac80211_hwsim_tx_frame(struct ieee80211_hw *hw,
 static void mac80211_hwsim_beacon_tx(void *arg, u8 *mac,
 				     struct ieee80211_vif *vif)
 {
-	struct ieee80211_hw *hw = arg;
+	struct mac80211_hwsim_data *data = arg;
+	struct ieee80211_hw *hw = data->hw;
+	struct ieee80211_tx_info *info;
+	struct ieee80211_rate *txrate;
+	struct ieee80211_mgmt *mgmt;
 	struct sk_buff *skb;
 
 	hwsim_check_magic(vif);
@@ -977,6 +973,13 @@ static void mac80211_hwsim_beacon_tx(void *arg, u8 *mac,
 	skb = ieee80211_beacon_get(hw, vif);
 	if (skb == NULL)
 		return;
+	info = IEEE80211_SKB_CB(skb);
+	txrate = ieee80211_get_tx_rate(hw, info);
+
+	mgmt = (struct ieee80211_mgmt *) skb->data;
+	/* fake header transmission time */
+	mgmt->u.beacon.timestamp = cpu_to_le64(__mac80211_hwsim_get_tsf(data) +
+					       24 * 8 * 10 / txrate->bitrate);
 
 	mac80211_hwsim_tx_frame(hw, skb,
 				rcu_dereference(vif->chanctx_conf)->def.chan);
@@ -989,7 +992,7 @@ static void mac80211_hwsim_bcn_tasklet(unsigned long d)
 
 	ieee80211_iterate_active_interfaces_atomic(
 		hw, IEEE80211_IFACE_ITER_NORMAL,
-		mac80211_hwsim_beacon_tx, hw);
+		mac80211_hwsim_beacon_tx, data);
 }
 
 static enum hrtimer_restart
@@ -997,6 +1000,7 @@ mac80211_hwsim_beacon(struct hrtimer *timer)
 {
 	struct mac80211_hwsim_data *data =
 		container_of(timer, struct mac80211_hwsim_data, beacon_timer);
+	u64 bcn_int = ktime_to_ns(data->beacon_int) / 1000;
 
 	if (!data->started)
 		return HRTIMER_NORESTART;
@@ -1004,7 +1008,13 @@ mac80211_hwsim_beacon(struct hrtimer *timer)
 	/* must defer here since hrtimers are run in hard-IRQ */
 	tasklet_schedule(&data->bcn_tasklet);
 
-	hrtimer_forward(timer, hrtimer_get_expires(timer), data->beacon_int);
+	/* beacon at new TBTT + beacon interval */
+	if (data->bcn_delta) {
+		bcn_int -= data->bcn_delta;
+		data->bcn_delta = 0;
+	}
+	hrtimer_forward(timer, hrtimer_get_expires(timer),
+			ns_to_ktime(bcn_int * 1000));
 	return HRTIMER_RESTART;
 }
 
-- 
1.7.10.4

Re: [PATCH v2 1/3] mac80211_hwsim: use hrtimer for beacon timers

[Johannes Berg]

On Thu, 2012-12-20 at 10:57 -0800, Thomas Pedersen wrote:

> @@ -896,7 +897,8 @@ static void mac80211_hwsim_stop(struct ieee80211_hw *hw)
>  {
>  	struct mac80211_hwsim_data *data = hw->priv;
>  	data->started = false;
> -	del_timer(&data->beacon_timer);
> +	hrtimer_cancel(&data->beacon_timer);
> +	tasklet_kill(&data->bcn_tasklet);
>  	wiphy_debug(hw->wiphy, "%s\n", __func__);

Hm this seems odd, why is it stopped here rather than when beaconing is
stopped? Or does it do both?

> @@ -1084,12 +1096,12 @@ static void mac80211_hwsim_bss_info_changed(struct ieee80211_hw *hw,
>  
>  	if (changed & BSS_CHANGED_BEACON_INT) {
>  		wiphy_debug(hw->wiphy, "  BCNINT: %d\n", info->beacon_int);
> -		data->beacon_int = 1024 * info->beacon_int / 1000 * HZ / 1000;
> -		if (WARN_ON(!data->beacon_int))
> -			data->beacon_int = 1;
> -		if (data->started)
> -			mod_timer(&data->beacon_timer,
> -				  jiffies + data->beacon_int);
> +		data->beacon_int = ns_to_ktime(info->beacon_int * 1024 * 1000);
> +		if (WARN_ON(!ktime_to_ns(data->beacon_int)))
> +			data->beacon_int = ns_to_ktime(1000 * 1000);

Can that warning really happen? It seems it should be checking
info->beacon_int rather than data->beacon_int? Why convert back and
forth?

Also the default here seems very very small, that's like less than 1ms
beacon interval (not even in TU)?

johannes

Re: [PATCH v2 2/3] mac80211_hwsim: beacon at beacon interval

[Johannes Berg]

On Thu, 2012-12-20 at 10:57 -0800, Thomas Pedersen wrote:

> +	else if (!hrtimer_is_queued(&data->beacon_timer)) {
> +		u64 tsf = le64_to_cpu(__mac80211_hwsim_get_tsf(data));

This is odd, you haven't even modified __mac80211_hwsim_get_tsf() yet?
Also, there's mac80211_hwsim_get_tsf() (although I'd argue that they
should be the other way around wrt. endianness)

johannes

Re: [PATCH v2 3/3] mac80211_hwsim: emulate proper beaconing

[Johannes Berg]

On Thu, 2012-12-20 at 10:57 -0800, Thomas Pedersen wrote:

> +	u64 now = __mac80211_hwsim_get_tsf(data);

That looks like an endian bug.

johannes

Re: [PATCH v2 1/3] mac80211_hwsim: use hrtimer for beacon timers

[Thomas Pedersen]

On Fri, Dec 21, 2012 at 03:33:16PM +0100, Johannes Berg wrote:
> On Thu, 2012-12-20 at 10:57 -0800, Thomas Pedersen wrote:
> 
> > @@ -896,7 +897,8 @@ static void mac80211_hwsim_stop(struct ieee80211_hw *hw)
> >  {
> >  	struct mac80211_hwsim_data *data = hw->priv;
> >  	data->started = false;
> > -	del_timer(&data->beacon_timer);
> > +	hrtimer_cancel(&data->beacon_timer);
> > +	tasklet_kill(&data->bcn_tasklet);
> >  	wiphy_debug(hw->wiphy, "%s\n", __func__);
> 
> Hm this seems odd, why is it stopped here rather than when beaconing is
> stopped? Or does it do both?

It seems mac80211_hwsim doesn't even handle the case of
BSS_CHANGED_BEACON_ENABLED, I can add this.

> > @@ -1084,12 +1096,12 @@ static void mac80211_hwsim_bss_info_changed(struct ieee80211_hw *hw,
> >  
> >  	if (changed & BSS_CHANGED_BEACON_INT) {
> >  		wiphy_debug(hw->wiphy, "  BCNINT: %d\n", info->beacon_int);
> > -		data->beacon_int = 1024 * info->beacon_int / 1000 * HZ / 1000;
> > -		if (WARN_ON(!data->beacon_int))
> > -			data->beacon_int = 1;
> > -		if (data->started)
> > -			mod_timer(&data->beacon_timer,
> > -				  jiffies + data->beacon_int);
> > +		data->beacon_int = ns_to_ktime(info->beacon_int * 1024 * 1000);
> > +		if (WARN_ON(!ktime_to_ns(data->beacon_int)))
> > +			data->beacon_int = ns_to_ktime(1000 * 1000);
> 
> Can that warning really happen? It seems it should be checking
> info->beacon_int rather than data->beacon_int? Why convert back and
> forth?

No, it looks like nl80211 will filter out beacon intervals of 0 anyway
so let's kill that.

> Also the default here seems very very small, that's like less than 1ms
> beacon interval (not even in TU)?

Yes, I was just trying to mirror the existing code. How is 1000 (TU) for
a sane default?

Thomas

Re: [PATCH v2 2/3] mac80211_hwsim: beacon at beacon interval

[Thomas Pedersen]

On Fri, Dec 21, 2012 at 03:36:24PM +0100, Johannes Berg wrote:
> On Thu, 2012-12-20 at 10:57 -0800, Thomas Pedersen wrote:
> 
> > +	else if (!hrtimer_is_queued(&data->beacon_timer)) {
> > +		u64 tsf = le64_to_cpu(__mac80211_hwsim_get_tsf(data));
> 
> This is odd, you haven't even modified __mac80211_hwsim_get_tsf() yet?
> Also, there's mac80211_hwsim_get_tsf() (although I'd argue that they
> should be the other way around wrt. endianness)

You're right, mac80211_hwsim_get_tsf() is less redundant. Also I guess
the timebase changes should go into the first patch.

Thomas

Re: [PATCH v2 3/3] mac80211_hwsim: emulate proper beaconing

[Thomas Pedersen]

On Fri, Dec 21, 2012 at 03:37:18PM +0100, Johannes Berg wrote:
> On Thu, 2012-12-20 at 10:57 -0800, Thomas Pedersen wrote:
> 
> > +	u64 now = __mac80211_hwsim_get_tsf(data);
> 
> That looks like an endian bug.

Yep :)

Thomas