2008-01-20 06:15:27

by Bruno Randolf

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH 2/4] ath5k: always extend rx timestamp with tsf

On Sunday 20 January 2008 11:12:11 Derek Smithies wrote:
> On Sat, 19 Jan 2008, Luis R. Rodriguez wrote:
> > On Jan 18, 2008 7:50 AM, Bruno Randolf <[email protected]> wrote:
> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* always extend the mac timestam=
p, since this
> > > information is + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* also needed for=
proper IBSS merging.
> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*
> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* XXX: it might be too late to d=
o it here, since
> > > rs_tstamp is + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* 15bit only. that =
means TSF extension
> > > has to be done within + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* 32.768us=
ec =3D 32ms. it might be
> > > necessary to move this to the + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* =
interrupt handler,
> > > like it is done in madwifi. + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
> >
> > I'm trying to understand this a bit more and am confused. The TSF
> > timer is based on 1MHz clock so it has a resolution of 1 microsecon=
d
> > (us). For 15 bits that would mean 32768 us so don't you mean it sho=
uld
> > be done within 32.768 ms instead of usec (or us)?
>
> Hi,
> =A0it is a typo.

it's just a different way to use a "." to denote 1000. americans might
write 32,768usec, im not sure, different styles worldwide... what i mea=
n
is 32768us equals about 32ms. i'll remove that dot to make it unambigou=
s.

> You are correct. It should be done within 32.768 ms. On a high end la=
ptop,
> it is almost guaranteed that the bottom half will process the packet
> within 32 ms. However, on an embedded box (low spec cpu) that has a
> temporarily high load, 32 ms is a short time, and the timestamp may h=
ave
> wrapped around. this is unfortunate.

i know that this might happen, and that's why i included this comment. =
i
was just too lazy to make the same act as is done in madwifi (lopping t=
hru
all rx descriptors in the interrupt handler). the current code is
sufficient for IBSS testing right now.

also even if we move TSF extension into the interrupt handler this will
not help in all cases. there are situations in IBSS mode - when a HW me=
rge
(=3D automatic HW TSF update upon receipt of a beacon with a higherr TS=
=46 and
the same BSSID) happens - where the rx timestamp is apparently based on
the old TSF, before the HW TSF is updated. in that case we cannot exten=
d
the timestamp in any meaningful way. i'm not sure how we should handle
this.

> On Sat, 19 Jan 2008, Luis R. Rodriguez wrote:
> Right now we only use mactime and even RX_FLAG_TSFT within mac80211 i=
n
> rx.c on ieee80211_rx_monitor() for IEEE80211_RADIOTAP_TSFT so right
> now these changes would seem to do nothing. Should we be using this
> for something else -- if so what is it?

see my "[PATCH] mac80211: enable IBSS merging". it is used there to dec=
ide
wether we have to merge IBSS on receipt of a beacon. strictly speaking =
it
would be sufficient to extend the rxstamp only for beacons and in monit=
or
mode, but i thought checking for these cases is more overhead, so why n=
ot
extend TSF all the time...

bruno


2008-01-20 08:46:07

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH 2/4] ath5k: always extend rx timestamp with tsf

On Jan 20, 2008 12:37 AM, bruno randolf <[email protected]> wrote:
> On Sunday 20 January 2008 11:12:11 Derek Smithies wrote:
> > On Sat, 19 Jan 2008, Luis R. Rodriguez wrote:
> > > On Jan 18, 2008 7:50 AM, Bruno Randolf <[email protected]> wrote:
> > > > + * always extend the mac timestamp, since this
> > > > information is + * also needed for proper IBSS merging.
> > > > + *
> > > > + * XXX: it might be too late to do it here, since
> > > > rs_tstamp is + * 15bit only. that means TSF extension
> > > > has to be done within + * 32.768usec = 32ms. it might be
> > > > necessary to move this to the + * interrupt handler,
> > > > like it is done in madwifi. + */
> > >
> > > I'm trying to understand this a bit more and am confused. The TSF
> > > timer is based on 1MHz clock so it has a resolution of 1 microsecond
> > > (us). For 15 bits that would mean 32768 us so don't you mean it should
> > > be done within 32.768 ms instead of usec (or us)?
> >
> > Hi,
> > it is a typo.
>
> it's just a different way to use a "." to denote 1000. americans might
> write 32,768usec, im not sure, different styles worldwide...

Yes, that is the 'american' way, not that it is correct.

> what i mean
> is 32768us equals about 32ms. i'll remove that dot to make it unambigous.

Yeah thanks, might be more clear for those of us not used to that
convention, I didn't even know it existed. Where is this "." for 1000
notation convention used, just so I know :) ?

> > You are correct. It should be done within 32.768 ms. On a high end laptop,
> > it is almost guaranteed that the bottom half will process the packet
> > within 32 ms. However, on an embedded box (low spec cpu) that has a
> > temporarily high load, 32 ms is a short time, and the timestamp may have
> > wrapped around. this is unfortunate.
>
> i know that this might happen, and that's why i included this comment. i
> was just too lazy to make the same act as is done in madwifi (lopping thru
> all rx descriptors in the interrupt handler). the current code is
> sufficient for IBSS testing right now.

Thanks for the explanation.

BTW while on the topic of TSF extension, anyone get why we do the tsf
-= 0x8000 if the hw's tsf's 15 bits value is < the rx descriptor's
rstamp? This is what ath5k_extend_tsf() does right before the (tsf &
~0x7fff) | rstamp.

> also even if we move TSF extension into the interrupt handler this will
> not help in all cases. there are situations in IBSS mode - when a HW merge
> (= automatic HW TSF update upon receipt of a beacon with a higherr TSF and
> the same BSSID) happens - where the rx timestamp is apparently based on
> the old TSF, before the HW TSF is updated. in that case we cannot extend
> the timestamp in any meaningful way. i'm not sure how we should handle
> this.

Hmm.. have you seen this with madwifi driver? What do you think is the
cause? If we don't do proper locking I can see this being an issue on
the driver side unless this is specifically a hardware issue.

> > On Sat, 19 Jan 2008, Luis R. Rodriguez wrote:
> > Right now we only use mactime and even RX_FLAG_TSFT within mac80211 in
> > rx.c on ieee80211_rx_monitor() for IEEE80211_RADIOTAP_TSFT so right
> > now these changes would seem to do nothing. Should we be using this
> > for something else -- if so what is it?
>
> see my "[PATCH] mac80211: enable IBSS merging". it is used there to decide
> wether we have to merge IBSS on receipt of a beacon.

Thanks, I have started to review it.

> strictly speaking it
> would be sufficient to extend the rxstamp only for beacons and in monitor
> mode, but i thought checking for these cases is more overhead, so why not
> extend TSF all the time...

Well sure, and also what's the point in updating mactime with 15-bit
value? So yes, sure, lets just keep it. Monitor mode and promiscuous
mode could use it as well. The only real penalty is in STA for
non-beacon frames and that is a matter of how expensive
ath5k_hw_get_tsf64() is and that's just two register reads.

Luis

2008-01-21 06:33:28

by Kalle Valo

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH 2/4] ath5k: always extend rx timestamp with tsf

Luis R. Rodriguez <[email protected]> writes:

>> what i mean
>> is 32768us equals about 32ms. i'll remove that dot to make it unambigous.
>
> Yeah thanks, might be more clear for those of us not used to that
> convention, I didn't even know it existed. Where is this "." for 1000
> notation convention used, just so I know :) ?

It seems most of Europe uses it, even though we Finns don't:

"In much of Europe, however, a comma is used as a decimal separator,
while a full stop or a space is used for the presentation of large
numbers:

* 1.000.000 (One million)
* 1.000,000 or 1 000,000 (One thousand)"

http://en.wikipedia.org/wiki/Full_stop#Mathematical_usage

--
Kalle Valo