All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] mac80211_hwsim: fix duplicate beacons on TSF adjust
@ 2013-12-15 21:14 Thomas Pedersen
  2013-12-15 21:14 ` [PATCH 2/3] mac80211: reset TSF to 0 when joining a mesh Thomas Pedersen
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Thomas Pedersen @ 2013-12-15 21:14 UTC (permalink / raw)
  To: Johannes Berg; +Cc: open80211s, linux-wireless, Thomas Pedersen

From: Thomas Pedersen <thomas@cozybit.com>

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 <thomas@cozybit.com>
---
 drivers/net/wireless/mac80211_hwsim.c |   56 +++++++++++++++------------------
 1 file changed, 25 insertions(+), 31 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index ab87556..cd27851 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -434,17 +434,35 @@ 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_is_queued(&data->beacon_timer.timer))
+		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,
@@ -1067,8 +1085,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;
@@ -1077,15 +1093,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;
 }
@@ -1138,15 +1146,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 if (!hrtimer_is_queued(&data->beacon_timer.timer))
+		mac80211_hwsim_beacon_sched(hw);
 
 	return 0;
 }
@@ -1216,16 +1217,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(
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] mac80211: reset TSF to 0 when joining a mesh
  2013-12-15 21:14 [PATCH 1/3] mac80211_hwsim: fix duplicate beacons on TSF adjust Thomas Pedersen
@ 2013-12-15 21:14 ` Thomas Pedersen
  2013-12-16 12:33   ` Sergey Ryazanov
  2013-12-15 21:14 ` [PATCH 3/3] mac80211: update adjusting TBTT bit in beacon Thomas Pedersen
  2013-12-16 12:53 ` [PATCH 1/3] mac80211_hwsim: fix duplicate beacons on TSF adjust Johannes Berg
  2 siblings, 1 reply; 12+ messages in thread
From: Thomas Pedersen @ 2013-12-15 21:14 UTC (permalink / raw)
  To: Johannes Berg; +Cc: open80211s, linux-wireless, Thomas Pedersen

From: Thomas Pedersen <thomas@cozybit.com>

This should ensure the mac80211 DTIM count matches that of
the firmware beacon timer (DTIM 0 starts at TSF 0).

Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
---
 net/mac80211/mesh.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 330d1f7..1174157 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -802,6 +802,8 @@ int ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 		return -ENOMEM;
 	}
 
+	/* next beacon will be DTIM-1, so TSF=0 was DTIM=0 */
+	drv_set_tsf(local, sdata, 0);
 	ieee80211_bss_info_change_notify(sdata, changed);
 
 	netif_carrier_on(sdata->dev);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] mac80211: update adjusting TBTT bit in beacon
  2013-12-15 21:14 [PATCH 1/3] mac80211_hwsim: fix duplicate beacons on TSF adjust Thomas Pedersen
  2013-12-15 21:14 ` [PATCH 2/3] mac80211: reset TSF to 0 when joining a mesh Thomas Pedersen
@ 2013-12-15 21:14 ` Thomas Pedersen
  2013-12-16 12:55   ` Johannes Berg
  2013-12-16 12:53 ` [PATCH 1/3] mac80211_hwsim: fix duplicate beacons on TSF adjust Johannes Berg
  2 siblings, 1 reply; 12+ messages in thread
From: Thomas Pedersen @ 2013-12-15 21:14 UTC (permalink / raw)
  To: Johannes Berg; +Cc: open80211s, linux-wireless, Thomas Pedersen

From: Thomas Pedersen <thomas@cozybit.com>

This regression was introduced in "mac80211: cache mesh
beacon".

mesh_sync_offset_adjust_tbtt()  was assuming that the
beacon would be rebuilt in every single pre-tbtt
interrupt, but now the beacon update happens on the
workqueue, and it must be ready for immediate delivery to
the driver.

Save a pointer to the meshconf IE in the beacon_data (this
works because both the IE pointer and beacon buffer are
protected by the same rcu_{dereference,assign_pointer}())
for quick updates during pre-tbtt. This is faster and a
little prettier than iterating over the elements to find
the meshconf IE every time.

Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
---
 net/mac80211/ieee80211_i.h |    9 ++++++++-
 net/mac80211/mesh.c        |    5 +++++
 net/mac80211/mesh_sync.c   |    9 ++++++++-
 net/mac80211/tx.c          |    3 +--
 4 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 32bae21..ec1d4a6 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -232,6 +232,7 @@ struct ieee80211_rx_data {
 struct beacon_data {
 	u8 *head, *tail;
 	int head_len, tail_len;
+	struct ieee80211_meshconf_ie *meshconf;
 	struct rcu_head rcu_head;
 };
 
@@ -540,7 +541,10 @@ struct ieee80211_mesh_sync_ops {
 			     struct ieee80211_mgmt *mgmt,
 			     struct ieee802_11_elems *elems,
 			     struct ieee80211_rx_status *rx_status);
-	void (*adjust_tbtt)(struct ieee80211_sub_if_data *sdata);
+
+	/* should be called with beacon_data under RCU read lock */
+	void (*adjust_tbtt)(struct ieee80211_sub_if_data *sdata,
+			    struct beacon_data *beacon);
 	/* add other framework functions here */
 };
 
@@ -614,6 +618,9 @@ struct ieee80211_if_mesh {
 	bool chsw_init;
 	u8 chsw_ttl;
 	u16 pre_value;
+
+	/* offset from skb->data while building IE */
+	int meshconf_offset;
 };
 
 #ifdef CONFIG_MAC80211_MESH
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 1174157..86c2d41 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -259,6 +259,9 @@ int mesh_add_meshconf_ie(struct ieee80211_sub_if_data *sdata,
 	*pos++ = WLAN_EID_MESH_CONFIG;
 	*pos++ = meshconf_len;
 
+	/* save a pointer for quick updates in pre-tbtt */
+	ifmsh->meshconf_offset = pos - skb->data;
+
 	/* Active path selection protocol ID */
 	*pos++ = ifmsh->mesh_pp_id;
 	/* Active path selection metric ID   */
@@ -723,6 +726,8 @@ ieee80211_mesh_build_beacon(struct ieee80211_if_mesh *ifmsh)
 
 	bcn->tail_len = skb->len;
 	memcpy(bcn->tail, skb->data, bcn->tail_len);
+	bcn->meshconf = (struct ieee80211_meshconf_ie *)
+					(bcn->tail + ifmsh->meshconf_offset);
 
 	dev_kfree_skb(skb);
 	rcu_assign_pointer(ifmsh->beacon, bcn);
diff --git a/net/mac80211/mesh_sync.c b/net/mac80211/mesh_sync.c
index d1cf2d5..2bc5dc2 100644
--- a/net/mac80211/mesh_sync.c
+++ b/net/mac80211/mesh_sync.c
@@ -164,12 +164,15 @@ no_sync:
 	rcu_read_unlock();
 }
 
-static void mesh_sync_offset_adjust_tbtt(struct ieee80211_sub_if_data *sdata)
+static void mesh_sync_offset_adjust_tbtt(struct ieee80211_sub_if_data *sdata,
+					 struct beacon_data *beacon)
 {
 	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
+	u8 cap;
 
 	WARN_ON(ifmsh->mesh_sp_id != IEEE80211_SYNC_METHOD_NEIGHBOR_OFFSET);
 	BUG_ON(!rcu_read_lock_held());
+	cap = beacon->meshconf->meshconf_cap;
 
 	spin_lock_bh(&ifmsh->sync_offset_lock);
 
@@ -194,6 +197,10 @@ static void mesh_sync_offset_adjust_tbtt(struct ieee80211_sub_if_data *sdata)
 		ifmsh->adjusting_tbtt = false;
 	}
 	spin_unlock_bh(&ifmsh->sync_offset_lock);
+
+	beacon->meshconf->meshconf_cap = ifmsh->adjusting_tbtt ?
+			IEEE80211_MESHCONF_CAPAB_TBTT_ADJUSTING | cap :
+			~IEEE80211_MESHCONF_CAPAB_TBTT_ADJUSTING & cap;
 }
 
 static const struct sync_method sync_methods[] = {
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 6d59e21..d758509 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2602,8 +2602,7 @@ struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
 			ieee80211_update_csa(sdata, bcn);
 
 		if (ifmsh->sync_ops)
-			ifmsh->sync_ops->adjust_tbtt(
-						sdata);
+			ifmsh->sync_ops->adjust_tbtt(sdata, bcn);
 
 		skb = dev_alloc_skb(local->tx_headroom +
 				    bcn->head_len +
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] mac80211: reset TSF to 0 when joining a mesh
  2013-12-15 21:14 ` [PATCH 2/3] mac80211: reset TSF to 0 when joining a mesh Thomas Pedersen
@ 2013-12-16 12:33   ` Sergey Ryazanov
  2013-12-16 18:34     ` Thomas Pedersen
  0 siblings, 1 reply; 12+ messages in thread
From: Sergey Ryazanov @ 2013-12-16 12:33 UTC (permalink / raw)
  To: Thomas Pedersen
  Cc: Johannes Berg, open80211s, linux-wireless, Thomas Pedersen

Hello Thomas,

2013/12/16 Thomas Pedersen <twpedersen@gmail.com>:
> diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
> index 330d1f7..1174157 100644
> --- a/net/mac80211/mesh.c
> +++ b/net/mac80211/mesh.c
> @@ -802,6 +802,8 @@ int ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
>                 return -ENOMEM;
>         }
>
> +       /* next beacon will be DTIM-1, so TSF=0 was DTIM=0 */
> +       drv_set_tsf(local, sdata, 0);
>         ieee80211_bss_info_change_notify(sdata, changed);
>
>         netif_carrier_on(sdata->dev);

What happen with AP interface on the same radio if we configure mesh
portal? Clients could be confused by such TSF jump.

-- 
BR,
Sergey

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] mac80211_hwsim: fix duplicate beacons on TSF adjust
  2013-12-15 21:14 [PATCH 1/3] mac80211_hwsim: fix duplicate beacons on TSF adjust Thomas Pedersen
  2013-12-15 21:14 ` [PATCH 2/3] mac80211: reset TSF to 0 when joining a mesh Thomas Pedersen
  2013-12-15 21:14 ` [PATCH 3/3] mac80211: update adjusting TBTT bit in beacon Thomas Pedersen
@ 2013-12-16 12:53 ` Johannes Berg
  2013-12-16 13:24   ` Johannes Berg
  2 siblings, 1 reply; 12+ messages in thread
From: Johannes Berg @ 2013-12-16 12:53 UTC (permalink / raw)
  To: Thomas Pedersen; +Cc: open80211s, linux-wireless, Thomas Pedersen

On Sun, 2013-12-15 at 13:14 -0800, Thomas Pedersen wrote:
> From: Thomas Pedersen <thomas@cozybit.com>
> 
> 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.

Applied.

johannes


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] mac80211: update adjusting TBTT bit in beacon
  2013-12-15 21:14 ` [PATCH 3/3] mac80211: update adjusting TBTT bit in beacon Thomas Pedersen
@ 2013-12-16 12:55   ` Johannes Berg
  0 siblings, 0 replies; 12+ messages in thread
From: Johannes Berg @ 2013-12-16 12:55 UTC (permalink / raw)
  To: Thomas Pedersen; +Cc: open80211s, linux-wireless, Thomas Pedersen

On Sun, 2013-12-15 at 13:14 -0800, Thomas Pedersen wrote:
> From: Thomas Pedersen <thomas@cozybit.com>
> 
> This regression was introduced in "mac80211: cache mesh
> beacon".
> 
> mesh_sync_offset_adjust_tbtt()  was assuming that the
> beacon would be rebuilt in every single pre-tbtt
> interrupt, but now the beacon update happens on the
> workqueue, and it must be ready for immediate delivery to
> the driver.
> 
> Save a pointer to the meshconf IE in the beacon_data (this
> works because both the IE pointer and beacon buffer are
> protected by the same rcu_{dereference,assign_pointer}())
> for quick updates during pre-tbtt. This is faster and a
> little prettier than iterating over the elements to find
> the meshconf IE every time.

Applied, since it doesn't look like this depends on patch 2. I'll let
you discuss patch 2 with Sergey first, though I'm of the opinion that if
there's no per-vif TSF in your hardware then you're SOL anyway and what
you did is fine.

johannes


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] mac80211_hwsim: fix duplicate beacons on TSF adjust
  2013-12-16 12:53 ` [PATCH 1/3] mac80211_hwsim: fix duplicate beacons on TSF adjust Johannes Berg
@ 2013-12-16 13:24   ` Johannes Berg
  2013-12-16 18:35     ` Thomas Pedersen
  0 siblings, 1 reply; 12+ messages in thread
From: Johannes Berg @ 2013-12-16 13:24 UTC (permalink / raw)
  To: Thomas Pedersen; +Cc: open80211s, linux-wireless, Thomas Pedersen

On Mon, 2013-12-16 at 13:53 +0100, Johannes Berg wrote:
> On Sun, 2013-12-15 at 13:14 -0800, Thomas Pedersen wrote:
> > From: Thomas Pedersen <thomas@cozybit.com>
> > 
> > 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.
> 
> Applied.

Nope, never mind - this caused the wpa_s test cases to crash the kernel.

johannes


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] mac80211: reset TSF to 0 when joining a mesh
  2013-12-16 12:33   ` Sergey Ryazanov
@ 2013-12-16 18:34     ` Thomas Pedersen
  2013-12-16 21:55       ` Sergey Ryazanov
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Pedersen @ 2013-12-16 18:34 UTC (permalink / raw)
  To: Sergey Ryazanov
  Cc: Johannes Berg, open80211s, linux-wireless, Thomas Pedersen

On Mon, Dec 16, 2013 at 4:33 AM, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
> Hello Thomas,
>
> 2013/12/16 Thomas Pedersen <twpedersen@gmail.com>:
>> diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
>> index 330d1f7..1174157 100644
>> --- a/net/mac80211/mesh.c
>> +++ b/net/mac80211/mesh.c
>> @@ -802,6 +802,8 @@ int ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
>>                 return -ENOMEM;
>>         }
>>
>> +       /* next beacon will be DTIM-1, so TSF=0 was DTIM=0 */
>> +       drv_set_tsf(local, sdata, 0);
>>         ieee80211_bss_info_change_notify(sdata, changed);
>>
>>         netif_carrier_on(sdata->dev);
>
> What happen with AP interface on the same radio if we configure mesh
> portal? Clients could be confused by such TSF jump.

Like Johannes said; either the driver supports per-vif TSF, or well,
it doesn't. Anyway wouldn't clients reset their own TSF when the new
beacon is received?

Thomas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] mac80211_hwsim: fix duplicate beacons on TSF adjust
  2013-12-16 13:24   ` Johannes Berg
@ 2013-12-16 18:35     ` Thomas Pedersen
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Pedersen @ 2013-12-16 18:35 UTC (permalink / raw)
  To: Johannes Berg; +Cc: open80211s, linux-wireless, Thomas Pedersen

On Mon, Dec 16, 2013 at 5:24 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2013-12-16 at 13:53 +0100, Johannes Berg wrote:
>> On Sun, 2013-12-15 at 13:14 -0800, Thomas Pedersen wrote:
>> > From: Thomas Pedersen <thomas@cozybit.com>
>> >
>> > 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.
>>
>> Applied.
>
> Nope, never mind - this caused the wpa_s test cases to crash the kernel.

OK sorry about that, I'll give it a run through those.

Thanks,
Thomas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] mac80211: reset TSF to 0 when joining a mesh
  2013-12-16 18:34     ` Thomas Pedersen
@ 2013-12-16 21:55       ` Sergey Ryazanov
  2013-12-17 17:59         ` Thomas Pedersen
  0 siblings, 1 reply; 12+ messages in thread
From: Sergey Ryazanov @ 2013-12-16 21:55 UTC (permalink / raw)
  To: Thomas Pedersen
  Cc: Johannes Berg, open80211s, linux-wireless, Thomas Pedersen

2013/12/16 Thomas Pedersen <twpedersen@gmail.com>:
> On Mon, Dec 16, 2013 at 4:33 AM, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
>> Hello Thomas,
>>
>> 2013/12/16 Thomas Pedersen <twpedersen@gmail.com>:
>>> diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
>>> index 330d1f7..1174157 100644
>>> --- a/net/mac80211/mesh.c
>>> +++ b/net/mac80211/mesh.c
>>> @@ -802,6 +802,8 @@ int ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
>>>                 return -ENOMEM;
>>>         }
>>>
>>> +       /* next beacon will be DTIM-1, so TSF=0 was DTIM=0 */
>>> +       drv_set_tsf(local, sdata, 0);
>>>         ieee80211_bss_info_change_notify(sdata, changed);
>>>
>>>         netif_carrier_on(sdata->dev);
>>
>> What happen with AP interface on the same radio if we configure mesh
>> portal? Clients could be confused by such TSF jump.
>
> Like Johannes said; either the driver supports per-vif TSF, or well,
> it doesn't. Anyway wouldn't clients reset their own TSF when the new
> beacon is received?
>
Yeah, IEEE 802.11 says that stations must blindly use AP time, but it
says nothing about the situation when time is accidentally reset to
zero. I can't predict reaction of each chip/firmware/driver to such
situation.

Another one unclear situation is the reaction of peers of mesh STA,
which share the same radio (and same TSF). If we silently reset its
time, what happens to the time synchronization with neighbors?

Why could not we calculate DTIM counter value on basis of known TSF
and Beacon/DTIM interval, instead of primitive time reset?

-- 
BR,
Sergey

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] mac80211: reset TSF to 0 when joining a mesh
  2013-12-16 21:55       ` Sergey Ryazanov
@ 2013-12-17 17:59         ` Thomas Pedersen
  2013-12-18  0:17           ` Sergey Ryazanov
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Pedersen @ 2013-12-17 17:59 UTC (permalink / raw)
  To: Sergey Ryazanov
  Cc: Johannes Berg, open80211s, linux-wireless, Thomas Pedersen

On Mon, Dec 16, 2013 at 1:55 PM, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
> 2013/12/16 Thomas Pedersen <twpedersen@gmail.com>:
>> On Mon, Dec 16, 2013 at 4:33 AM, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
>>> Hello Thomas,
>>>
>>> 2013/12/16 Thomas Pedersen <twpedersen@gmail.com>:
>>>> diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
>>>> index 330d1f7..1174157 100644
>>>> --- a/net/mac80211/mesh.c
>>>> +++ b/net/mac80211/mesh.c
>>>> @@ -802,6 +802,8 @@ int ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
>>>>                 return -ENOMEM;
>>>>         }
>>>>
>>>> +       /* next beacon will be DTIM-1, so TSF=0 was DTIM=0 */
>>>> +       drv_set_tsf(local, sdata, 0);
>>>>         ieee80211_bss_info_change_notify(sdata, changed);
>>>>
>>>>         netif_carrier_on(sdata->dev);
>>>
>>> What happen with AP interface on the same radio if we configure mesh
>>> portal? Clients could be confused by such TSF jump.
>>
>> Like Johannes said; either the driver supports per-vif TSF, or well,
>> it doesn't. Anyway wouldn't clients reset their own TSF when the new
>> beacon is received?
>>
> Yeah, IEEE 802.11 says that stations must blindly use AP time, but it
> says nothing about the situation when time is accidentally reset to
> zero. I can't predict reaction of each chip/firmware/driver to such
> situation.
>
> Another one unclear situation is the reaction of peers of mesh STA,
> which share the same radio (and same TSF). If we silently reset its
> time, what happens to the time synchronization with neighbors?

Peer mesh STAs will notice an overly large TSF difference since last
measurement, and reset Toffset to this new value.

> Why could not we calculate DTIM counter value on basis of known TSF
> and Beacon/DTIM interval, instead of primitive time reset?

Yeah this sounds like  a nicer solution. So drv_get_tsf() on mesh
join, then calculate the right dtim_count? I'll give this a try.

Thomas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] mac80211: reset TSF to 0 when joining a mesh
  2013-12-17 17:59         ` Thomas Pedersen
@ 2013-12-18  0:17           ` Sergey Ryazanov
  0 siblings, 0 replies; 12+ messages in thread
From: Sergey Ryazanov @ 2013-12-18  0:17 UTC (permalink / raw)
  To: Thomas Pedersen
  Cc: Johannes Berg, open80211s, linux-wireless, Thomas Pedersen,
	Felix Fietkau

2013/12/17 Thomas Pedersen <twpedersen@gmail.com>:
> On Mon, Dec 16, 2013 at 1:55 PM, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
>> 2013/12/16 Thomas Pedersen <twpedersen@gmail.com>:
>>> On Mon, Dec 16, 2013 at 4:33 AM, Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
>>>> Hello Thomas,
>>>>
>>>> 2013/12/16 Thomas Pedersen <twpedersen@gmail.com>:
>>>>> diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
>>>>> index 330d1f7..1174157 100644
>>>>> --- a/net/mac80211/mesh.c
>>>>> +++ b/net/mac80211/mesh.c
>>>>> @@ -802,6 +802,8 @@ int ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
>>>>>                 return -ENOMEM;
>>>>>         }
>>>>>
>>>>> +       /* next beacon will be DTIM-1, so TSF=0 was DTIM=0 */
>>>>> +       drv_set_tsf(local, sdata, 0);
>>>>>         ieee80211_bss_info_change_notify(sdata, changed);
>>>>>
>>>>>         netif_carrier_on(sdata->dev);
>>>>
>>>> What happen with AP interface on the same radio if we configure mesh
>>>> portal? Clients could be confused by such TSF jump.
>>>
>>> Like Johannes said; either the driver supports per-vif TSF, or well,
>>> it doesn't. Anyway wouldn't clients reset their own TSF when the new
>>> beacon is received?
>>>
>> Yeah, IEEE 802.11 says that stations must blindly use AP time, but it
>> says nothing about the situation when time is accidentally reset to
>> zero. I can't predict reaction of each chip/firmware/driver to such
>> situation.
>>
>> Another one unclear situation is the reaction of peers of mesh STA,
>> which share the same radio (and same TSF). If we silently reset its
>> time, what happens to the time synchronization with neighbors?
>
> Peer mesh STAs will notice an overly large TSF difference since last
> measurement, and reset Toffset to this new value.
>
Hm, I missed that. Thanks.

>> Why could not we calculate DTIM counter value on basis of known TSF
>> and Beacon/DTIM interval, instead of primitive time reset?
>
> Yeah this sounds like  a nicer solution. So drv_get_tsf() on mesh
> join, then calculate the right dtim_count? I'll give this a try.
>
I must confess that my question was inspired by Felix patch for ath9k
[1], which removes the TSF reset at each reconfiguration. Maybe we
could add the necessary functions to mac80211 or cfg80211 and all
drivers could benefit from that?

1. http://www.spinics.net/lists/linux-wireless/msg116087.html

-- 
BR,
Sergey

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-12-18  0:18 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-15 21:14 [PATCH 1/3] mac80211_hwsim: fix duplicate beacons on TSF adjust Thomas Pedersen
2013-12-15 21:14 ` [PATCH 2/3] mac80211: reset TSF to 0 when joining a mesh Thomas Pedersen
2013-12-16 12:33   ` Sergey Ryazanov
2013-12-16 18:34     ` Thomas Pedersen
2013-12-16 21:55       ` Sergey Ryazanov
2013-12-17 17:59         ` Thomas Pedersen
2013-12-18  0:17           ` Sergey Ryazanov
2013-12-15 21:14 ` [PATCH 3/3] mac80211: update adjusting TBTT bit in beacon Thomas Pedersen
2013-12-16 12:55   ` Johannes Berg
2013-12-16 12:53 ` [PATCH 1/3] mac80211_hwsim: fix duplicate beacons on TSF adjust Johannes Berg
2013-12-16 13:24   ` Johannes Berg
2013-12-16 18:35     ` Thomas Pedersen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.