Return-path: Received: from mail-vb0-f46.google.com ([209.85.212.46]:58261 "EHLO mail-vb0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759728Ab2FAN00 convert rfc822-to-8bit (ORCPT ); Fri, 1 Jun 2012 09:26:26 -0400 Received: by vbbff1 with SMTP id ff1so1274098vbb.19 for ; Fri, 01 Jun 2012 06:26:25 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1336554519.4323.7.camel@jlt3.sipsolutions.net> Date: Fri, 1 Jun 2012 15:26:24 +0200 Message-ID: (sfid-20120601_152631_460084_734FDBFC) Subject: Re: [RFC] mac80211: add time synchronisation with BSS for assoc From: Ivo Van Doorn To: Helmut Schaa Cc: Johannes Berg , linux-wireless@vger.kernel.org, Gertjan van Wingerde Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-wireless-owner@vger.kernel.org List-ID: On Fri, Jun 1, 2012 at 9:41 AM, Helmut Schaa wrote: > On Fri, Jun 1, 2012 at 9:37 AM, Ivo Van Doorn wrote: >> Hi, >> >>> On Wed, May 9, 2012 at 11:08 AM, Johannes Berg >>> wrote: >>>> From: Johannes Berg >>>> >>>> Some drivers (iwlegacy, iwlwifi and rt2x00) today use the >>>> bss_conf.last_tsf value. By itself though that value is >>>> completely worthless since it may be ancient. What really >>>> is needed is synchronisation between some device time and >>>> the TSF. >>>> >>>> To clarify this, rename bss_conf.last_tsf to sync_tsf and >>>> add sync_device_ts which is obtained from rx_status which >>>> gets a new field device_timestamp for this purpose. This >>>> is intentionally not using the mactime field since that >>>> is used for other things and in IBSS is expected to sync >>>> with the IBSS's TSF which isn't necessarily true for the >>>> device timestamp. >>>> >>>> Also, since we have the information and it's useful even >>>> before the connection has been established, give all the >>>> timing details to the driver before authenticating. >>>> >>>> Signed-off-by: Johannes Berg >>>> --- >>>> I note that rt2x00 is completely broken since it mixes last_tsf (now >>>> sync_tsf) with jiffies ... what?? >>> >>> Thanks for coming up with this Johannes, ?that looks indeed strange. >>> >>> Ivo, that was introduced with your "rt2x00: Add autowake support for >>> USB hardware" patch as far as I can tell. >>> >>> Since the last_beacon value is updated within rt2x00 already we should >>> be able to just remove the assignment in rt2x00lib_config_erp. Ivo, could >>> you please recheck? >>> >>> This would also remove rt2x00's dependency on the last_tsf >>> field as provided by mac80211. >>> >>> Thanks, >>> Helmut >>> >>> diff --git a/drivers/net/wireless/rt2x00/rt2x00config.c >>> b/drivers/net/wireless/rt2x00/rt2x00config.c >>> index 96db933..b2d2ad3 100644 >>> --- a/drivers/net/wireless/rt2x00/rt2x00config.c >>> +++ b/drivers/net/wireless/rt2x00/rt2x00config.c >>> @@ -102,7 +102,6 @@ void rt2x00lib_config_erp(struct rt2x00_dev *rt2x00dev, >>> >>> ? ? ? ?/* Update the AID, this is needed for dynamic PS support */ >>> ? ? ? ?rt2x00dev->aid = bss_conf->assoc ? bss_conf->aid : 0; >>> - ? ? ? rt2x00dev->last_beacon = bss_conf->last_tsf; >> >> Can't access the code right now, is the last_beacon initialized somewhere else? >> because I do assume we need the value somewhere? > > Yup, it's set in two code paths. One is within rt2x00 where last_beacon is > set to jiffies. The other one is here where it is set to last_tsf which is not > derived from jiffies at all :D Hmm ok, well then removing this line should be safe :)