There was some bug when rescheduling the next beacon from
the beacon tasklet after adjusting TSF which would cause
the beacon timer to trigger twice. Beaconing at "old" TBT
(previously scheduled interface TBTT) with new timestamp
was incorrect anyway.
Instead, reschedule the beacon straight away when
adjusting TSF.
Signed-off-by: Thomas Pedersen <[email protected]>
---
v2:
don't kill hrtimer tasklet if currently running to
avoid deadlock (Johannes)
drivers/net/wireless/mac80211_hwsim.c | 57 +++++++++++++++------------------
1 file changed, 26 insertions(+), 31 deletions(-)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 9c0cc8d..cf3b9d3 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -442,17 +442,36 @@ static u64 mac80211_hwsim_get_tsf(struct ieee80211_hw *hw,
return le64_to_cpu(__mac80211_hwsim_get_tsf(data));
}
+static void mac80211_hwsim_beacon_sched(struct ieee80211_hw *hw)
+{
+ struct mac80211_hwsim_data *data = hw->priv;
+ u64 tsf = mac80211_hwsim_get_tsf(hw, NULL);
+ u32 bcn_int = data->beacon_int;
+ u64 until_tbtt;
+
+ if (!bcn_int)
+ return;
+
+ until_tbtt = bcn_int - do_div(tsf, bcn_int);
+ if (!hrtimer_callback_running(&data->beacon_timer.timer) &&
+ !test_bit(TASKLET_STATE_RUN, &data->beacon_timer.tasklet.state))
+ tasklet_hrtimer_cancel(&data->beacon_timer);
+ tasklet_hrtimer_start(&data->beacon_timer,
+ ns_to_ktime(until_tbtt * 1000),
+ HRTIMER_MODE_REL);
+}
+
static void mac80211_hwsim_set_tsf(struct ieee80211_hw *hw,
struct ieee80211_vif *vif, u64 tsf)
{
struct mac80211_hwsim_data *data = hw->priv;
u64 now = mac80211_hwsim_get_tsf(hw, vif);
- u32 bcn_int = data->beacon_int;
s64 delta = tsf - now;
data->tsf_offset += delta;
- /* adjust after beaconing with new timestamp at old TBTT */
- data->bcn_delta = do_div(delta, bcn_int);
+
+ /* reschedule next beacon to happen at new TBTT */
+ mac80211_hwsim_beacon_sched(hw);
}
static void mac80211_hwsim_monitor_rx(struct ieee80211_hw *hw,
@@ -1075,8 +1094,6 @@ mac80211_hwsim_beacon(struct hrtimer *timer)
container_of(timer, struct mac80211_hwsim_data,
beacon_timer.timer);
struct ieee80211_hw *hw = data->hw;
- u64 bcn_int = data->beacon_int;
- ktime_t next_bcn;
if (!data->started)
goto out;
@@ -1085,15 +1102,7 @@ mac80211_hwsim_beacon(struct hrtimer *timer)
hw, IEEE80211_IFACE_ITER_NORMAL,
mac80211_hwsim_beacon_tx, data);
- /* beacon at new TBTT + beacon interval */
- if (data->bcn_delta) {
- bcn_int -= data->bcn_delta;
- data->bcn_delta = 0;
- }
-
- next_bcn = ktime_add(hrtimer_get_expires(timer),
- ns_to_ktime(bcn_int * 1000));
- tasklet_hrtimer_start(&data->beacon_timer, next_bcn, HRTIMER_MODE_ABS);
+ mac80211_hwsim_beacon_sched(hw);
out:
return HRTIMER_NORESTART;
}
@@ -1146,15 +1155,8 @@ static int mac80211_hwsim_config(struct ieee80211_hw *hw, u32 changed)
data->power_level = conf->power_level;
if (!data->started || !data->beacon_int)
tasklet_hrtimer_cancel(&data->beacon_timer);
- else if (!hrtimer_is_queued(&data->beacon_timer.timer)) {
- u64 tsf = mac80211_hwsim_get_tsf(hw, NULL);
- u32 bcn_int = data->beacon_int;
- u64 until_tbtt = bcn_int - do_div(tsf, bcn_int);
-
- tasklet_hrtimer_start(&data->beacon_timer,
- ns_to_ktime(until_tbtt * 1000),
- HRTIMER_MODE_REL);
- }
+ else
+ mac80211_hwsim_beacon_sched(hw);
return 0;
}
@@ -1224,16 +1226,9 @@ static void mac80211_hwsim_bss_info_changed(struct ieee80211_hw *hw,
if (data->started &&
!hrtimer_is_queued(&data->beacon_timer.timer) &&
info->enable_beacon) {
- u64 tsf, until_tbtt;
- u32 bcn_int;
if (WARN_ON(!data->beacon_int))
data->beacon_int = 1000 * 1024;
- tsf = mac80211_hwsim_get_tsf(hw, vif);
- bcn_int = data->beacon_int;
- until_tbtt = bcn_int - do_div(tsf, bcn_int);
- tasklet_hrtimer_start(&data->beacon_timer,
- ns_to_ktime(until_tbtt * 1000),
- HRTIMER_MODE_REL);
+ mac80211_hwsim_beacon_sched(hw);
} else if (!info->enable_beacon) {
unsigned int count = 0;
ieee80211_iterate_active_interfaces_atomic(
--
1.7.10.4
On starting a mesh or AP BSS, the interface dtim_count
countdown should match that of the driver TSF.
Signed-off-by: Thomas Pedersen <[email protected]>
---
v2:
adjust dtim_count instead of setting TSF=0
(Sergey)
v3:
handle interface types other than mesh, and drop
IBSS dtim calculation since mac80211 doesn't
support ad-hoc PS.
net/mac80211/cfg.c | 2 ++
net/mac80211/debugfs_netdev.c | 1 +
net/mac80211/ieee80211_i.h | 2 ++
net/mac80211/mesh.c | 1 +
net/mac80211/util.c | 40 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 46 insertions(+)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index ac18528..8211d30 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -951,6 +951,7 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
struct cfg80211_ap_settings *params)
{
struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ struct ieee80211_local *local = sdata->local;
struct beacon_data *old;
struct ieee80211_sub_if_data *vlan;
u32 changed = BSS_CHANGED_BEACON_INT |
@@ -1030,6 +1031,7 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
return err;
}
+ ieee80211_recalc_dtim(local, sdata);
ieee80211_bss_info_change_notify(sdata, changed);
netif_carrier_on(dev);
diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index 04b5a14..89160f7 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -468,6 +468,7 @@ static ssize_t ieee80211_if_parse_tsf(
}
}
+ ieee80211_recalc_dtim(local, sdata);
return buflen;
}
__IEEE80211_IF_FILE_W(tsf);
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 33ebe08..337fcad 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1794,6 +1794,8 @@ ieee80211_cs_get(struct ieee80211_local *local, u32 cipher,
int ieee80211_cs_headroom(struct ieee80211_local *local,
struct cfg80211_crypto_settings *crypto,
enum nl80211_iftype iftype);
+void ieee80211_recalc_dtim(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata);
#ifdef CONFIG_MAC80211_NOINLINE
#define debug_noinline noinline
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 5a74b24..5b919ca 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -807,6 +807,7 @@ int ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
return -ENOMEM;
}
+ ieee80211_recalc_dtim(local, sdata);
ieee80211_bss_info_change_notify(sdata, changed);
netif_carrier_on(sdata->dev);
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 656648b..d51d6c1 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2588,3 +2588,43 @@ int ieee80211_cs_headroom(struct ieee80211_local *local,
return headroom;
}
+
+void ieee80211_recalc_dtim(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata)
+{
+ u64 tsf = drv_get_tsf(local, sdata);
+ u64 dtim_count = 0;
+ u16 beacon_int = sdata->vif.bss_conf.beacon_int * 1024;
+ u8 dtim_period = sdata->vif.bss_conf.dtim_period;
+ struct ps_data *ps;
+ u8 bcns_from_dtim;
+
+ if (tsf == -1ULL || !beacon_int || !dtim_period)
+ return;
+
+ if (sdata->vif.type == NL80211_IFTYPE_AP ||
+ sdata->vif.type == NL80211_IFTYPE_AP_VLAN) {
+ if (!sdata->bss)
+ return;
+
+ ps = &sdata->bss->ps;
+ } else if (ieee80211_vif_is_mesh(&sdata->vif))
+ ps = &sdata->u.mesh.ps;
+ else
+ return;
+
+ /*
+ * actually finds last dtim_count, mac80211 will update in
+ * __beacon_add_tim().
+ * dtim_count = dtim_period - (tsf / bcn_int) % dtim_period
+ */
+ do_div(tsf, beacon_int);
+ bcns_from_dtim = do_div(tsf, dtim_period);
+ /* just had a DTIM */
+ if (!bcns_from_dtim)
+ dtim_count = 0;
+ else
+ dtim_count = dtim_period - bcns_from_dtim;
+
+ ps->dtim_count = dtim_count;
+}
--
1.7.10.4
On Wed, 2014-01-15 at 09:55 -0800, Thomas Pedersen wrote:
> >> + if (!hrtimer_callback_running(&data->beacon_timer.timer) &&
> >> + !test_bit(TASKLET_STATE_RUN, &data->beacon_timer.tasklet.state))
> >> + tasklet_hrtimer_cancel(&data->beacon_timer);
> >
> > That test_bit() really seems suspicious - there are no other users in
> > the tree except for the internal tasklet code... What are you trying to
> > do?
>
> I'm trying to avoid calling tasklet_hrtimer_cancel() recursively, that
> is, when mac80211_hwsim_beacon_sched() is called from within the
> hrtimer tasklet itself. Looking at it again this does seem ugly. Would
> it be acceptable to pass a flag to _beacon_sched() indicating whether
> a reschedule is taking place from within the tasklet?
That seems fine to me.
> We could also
> have a mac80211_hwsim_beacon_sched() (which would cance the hrtimer
> tasklet) and __mac80211_hwsim_beacon_sched() pair, but I'm not sure if
> that nomenclature applies here.
Whichever causes less code duplication? Could one call the other maybe?
Dunno, up to you. I was really just thinking that the test_bit() was odd
since nobody else uses it.
johannes
On Thu, 2013-12-19 at 10:25 -0800, Thomas Pedersen wrote:
> There was some bug when rescheduling the next beacon from
> the beacon tasklet after adjusting TSF which would cause
> the beacon timer to trigger twice. Beaconing at "old" TBT
> (previously scheduled interface TBTT) with new timestamp
> was incorrect anyway.
>
> Instead, reschedule the beacon straight away when
> adjusting TSF.
>
> Signed-off-by: Thomas Pedersen <[email protected]>
>
> ---
> v2:
>
> don't kill hrtimer tasklet if currently running to
> avoid deadlock (Johannes)
>
> drivers/net/wireless/mac80211_hwsim.c | 57 +++++++++++++++------------------
> 1 file changed, 26 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
> index 9c0cc8d..cf3b9d3 100644
> --- a/drivers/net/wireless/mac80211_hwsim.c
> +++ b/drivers/net/wireless/mac80211_hwsim.c
> @@ -442,17 +442,36 @@ static u64 mac80211_hwsim_get_tsf(struct ieee80211_hw *hw,
> return le64_to_cpu(__mac80211_hwsim_get_tsf(data));
> }
>
> +static void mac80211_hwsim_beacon_sched(struct ieee80211_hw *hw)
> +{
> + struct mac80211_hwsim_data *data = hw->priv;
> + u64 tsf = mac80211_hwsim_get_tsf(hw, NULL);
> + u32 bcn_int = data->beacon_int;
> + u64 until_tbtt;
> +
> + if (!bcn_int)
> + return;
> +
> + until_tbtt = bcn_int - do_div(tsf, bcn_int);
> + if (!hrtimer_callback_running(&data->beacon_timer.timer) &&
> + !test_bit(TASKLET_STATE_RUN, &data->beacon_timer.tasklet.state))
> + tasklet_hrtimer_cancel(&data->beacon_timer);
That test_bit() really seems suspicious - there are no other users in
the tree except for the internal tasklet code... What are you trying to
do?
johannes
On Mon, Jan 6, 2014 at 11:09 AM, Johannes Berg
<[email protected]> wrote:
> On Thu, 2013-12-19 at 10:25 -0800, Thomas Pedersen wrote:
>> There was some bug when rescheduling the next beacon from
>> the beacon tasklet after adjusting TSF which would cause
>> the beacon timer to trigger twice. Beaconing at "old" TBT
>> (previously scheduled interface TBTT) with new timestamp
>> was incorrect anyway.
>>
>> Instead, reschedule the beacon straight away when
>> adjusting TSF.
>>
>> Signed-off-by: Thomas Pedersen <[email protected]>
>>
>> ---
>> v2:
>>
>> don't kill hrtimer tasklet if currently running to
>> avoid deadlock (Johannes)
>>
>> drivers/net/wireless/mac80211_hwsim.c | 57 +++++++++++++++------------------
>> 1 file changed, 26 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
>> index 9c0cc8d..cf3b9d3 100644
>> --- a/drivers/net/wireless/mac80211_hwsim.c
>> +++ b/drivers/net/wireless/mac80211_hwsim.c
>> @@ -442,17 +442,36 @@ static u64 mac80211_hwsim_get_tsf(struct ieee80211_hw *hw,
>> return le64_to_cpu(__mac80211_hwsim_get_tsf(data));
>> }
>>
>> +static void mac80211_hwsim_beacon_sched(struct ieee80211_hw *hw)
>> +{
>> + struct mac80211_hwsim_data *data = hw->priv;
>> + u64 tsf = mac80211_hwsim_get_tsf(hw, NULL);
>> + u32 bcn_int = data->beacon_int;
>> + u64 until_tbtt;
>> +
>> + if (!bcn_int)
>> + return;
>> +
>> + until_tbtt = bcn_int - do_div(tsf, bcn_int);
>> + if (!hrtimer_callback_running(&data->beacon_timer.timer) &&
>> + !test_bit(TASKLET_STATE_RUN, &data->beacon_timer.tasklet.state))
>> + tasklet_hrtimer_cancel(&data->beacon_timer);
>
> That test_bit() really seems suspicious - there are no other users in
> the tree except for the internal tasklet code... What are you trying to
> do?
I'm trying to avoid calling tasklet_hrtimer_cancel() recursively, that
is, when mac80211_hwsim_beacon_sched() is called from within the
hrtimer tasklet itself. Looking at it again this does seem ugly. Would
it be acceptable to pass a flag to _beacon_sched() indicating whether
a reschedule is taking place from within the tasklet? We could also
have a mac80211_hwsim_beacon_sched() (which would cance the hrtimer
tasklet) and __mac80211_hwsim_beacon_sched() pair, but I'm not sure if
that nomenclature applies here.
Thanks for reviewing.
Thomas
On Thu, 2013-12-19 at 10:25 -0800, Thomas Pedersen wrote:
> On starting a mesh or AP BSS, the interface dtim_count
> countdown should match that of the driver TSF.
Applied, though I think it's futile to really try to do this without
supporting multiple TSF timers :)
johannes